From 6319b9fc6c708f6536ac66ad7b6f636a3d4336f6 Mon Sep 17 00:00:00 2001 From: Bryan Bednarski Date: Mon, 6 Jul 2026 15:55:35 -0600 Subject: [PATCH 01/11] feat(ffi): activate dynamic plugins Signed-off-by: Bryan Bednarski --- Cargo.lock | 1 + crates/ffi/Cargo.toml | 5 +- crates/ffi/nemo_relay.h | 55 ++ crates/ffi/src/api/mod.rs | 5 +- crates/ffi/src/api/plugin.rs | 160 ++++- crates/ffi/src/types/mod.rs | 28 + crates/ffi/tests/integration/api_tests.rs | 32 +- crates/ffi/tests/integration/main.rs | 5 +- .../integration/plugin_activation_tests.rs | 387 ++++++++++++ crates/ffi/tests/unit/api/plugin_tests.rs | 232 ++++++++ crates/ffi/tests/unit/api_tests.rs | 28 +- go/nemo_relay/plugin.go | 144 +++++ go/nemo_relay/plugin_activation_test.go | 557 ++++++++++++++++++ 13 files changed, 1594 insertions(+), 45 deletions(-) create mode 100644 crates/ffi/tests/integration/plugin_activation_tests.rs create mode 100644 go/nemo_relay/plugin_activation_test.go diff --git a/Cargo.lock b/Cargo.lock index c1e6dadaa..3a2fd4dbc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1581,6 +1581,7 @@ dependencies = [ "nemo-relay-pii-redaction", "serde", "serde_json", + "tempfile", "tokio", "tokio-stream", "uuid", diff --git a/crates/ffi/Cargo.toml b/crates/ffi/Cargo.toml index f5628dfad..aad8a3c20 100644 --- a/crates/ffi/Cargo.toml +++ b/crates/ffi/Cargo.toml @@ -17,7 +17,7 @@ workspace = true crate-type = ["cdylib", "staticlib", "rlib"] [dependencies] -nemo-relay = { workspace = true, features = ["atof-streaming", "otel", "openinference"] } +nemo-relay = { workspace = true, features = ["atof-streaming", "otel", "openinference", "worker-grpc"] } nemo-relay-adaptive = { workspace = true, features = ["redis-backend"] } nemo-relay-pii-redaction.workspace = true chrono = "0.4" @@ -30,3 +30,6 @@ tokio-stream = "0.1" [build-dependencies] cbindgen = "0.29.2" + +[dev-dependencies] +tempfile = "3" diff --git a/crates/ffi/nemo_relay.h b/crates/ffi/nemo_relay.h index 6355d579c..35763c40c 100644 --- a/crates/ffi/nemo_relay.h +++ b/crates/ffi/nemo_relay.h @@ -159,6 +159,14 @@ typedef struct FfiOpenInferenceSubscriber FfiOpenInferenceSubscriber; */ typedef struct FfiOpenTelemetrySubscriber FfiOpenTelemetrySubscriber; +/** + * Opaque owned dynamic plugin host activation. + * + * The inner option allows explicit activation cleanup to be idempotent while + * retaining a stable allocation until the foreign caller frees the handle. + */ +typedef struct FfiPluginActivation FfiPluginActivation; + /** * Opaque plugin registration context. * @@ -1408,6 +1416,38 @@ NemoRelayStatus nemo_relay_openinference_subscriber_force_flush(const struct Ffi */ NemoRelayStatus nemo_relay_openinference_subscriber_shutdown(const struct FfiOpenInferenceSubscriber *subscriber); +/** + * Load and activate dynamic plugins as one owned transaction. + * + * `config_json` is the base [`PluginConfig`] document and + * `dynamic_plugins_json` is an array of explicit dynamic-plugin activation + * specifications. On success, the caller owns `out_activation` and must clear + * and free it with `nemo_relay_plugin_activation_clear` and + * `nemo_relay_plugin_activation_free`. `out_report_json` is a library-owned C + * string and must be released with `nemo_relay_string_free`. + * + * # Safety + * Both input pointers must reference valid, null-terminated C strings. + * `out_activation` and `out_report_json` must be valid, non-null output pointers. + */ +NemoRelayStatus nemo_relay_activate_dynamic_plugins(const char *config_json, + const char *dynamic_plugins_json, + struct FfiPluginActivation **out_activation, + char **out_report_json); + +/** + * Clear one owned dynamic plugin activation. + * + * This operation is idempotent. A null handle is treated as already cleared. + * The handle allocation remains owned by the caller and must still be passed + * to `nemo_relay_plugin_activation_free`. + * + * # Safety + * `activation` must be a valid activation handle returned by + * `nemo_relay_activate_dynamic_plugins`, or null. + */ +NemoRelayStatus nemo_relay_plugin_activation_clear(struct FfiPluginActivation *activation); + /** * Validate a generic plugin config document and return the diagnostics report as JSON. * @@ -2451,6 +2491,21 @@ void nemo_relay_openinference_subscriber_free(struct FfiOpenInferenceSubscriber */ void nemo_relay_adaptive_runtime_free(struct FfiAdaptiveRuntime *ptr); +/** + * Free a dynamic plugin activation handle previously returned by + * `nemo_relay_activate_dynamic_plugins`. + * + * Any activation that has not already been explicitly cleared is cleaned up + * best-effort by its Rust destructor before the allocation is released. The + * caller's handle is set to null before cleanup, making repeated calls through + * the same handle variable safe. + * + * # Safety + * `ptr` must be null or point to a writable activation-handle variable whose + * value is null or was returned by `nemo_relay_activate_dynamic_plugins`. + */ +void nemo_relay_plugin_activation_free(struct FfiPluginActivation **ptr); + /** * Free a codec handle previously returned by one of the codec constructor * functions (`nemo_relay_openai_chat_codec_new`, etc.). diff --git a/crates/ffi/src/api/mod.rs b/crates/ffi/src/api/mod.rs index 1d62d9e69..27817a82b 100644 --- a/crates/ffi/src/api/mod.rs +++ b/crates/ffi/src/api/mod.rs @@ -36,8 +36,8 @@ use crate::error::{ }; use crate::types::{ FfiAtifExporter, FfiAtofExporter, FfiCodecHandle, FfiLLMHandle, FfiLLMRequest, - FfiOpenInferenceSubscriber, FfiOpenTelemetrySubscriber, FfiPluginContext, FfiScopeHandle, - FfiScopeStack, FfiThreadScopeStackBinding, FfiToolHandle, NemoRelayScopeType, + FfiOpenInferenceSubscriber, FfiOpenTelemetrySubscriber, FfiPluginActivation, FfiPluginContext, + FfiScopeHandle, FfiScopeStack, FfiThreadScopeStackBinding, FfiToolHandle, NemoRelayScopeType, }; pub use crate::types::{nemo_relay_openinference_subscriber_free, nemo_relay_otel_subscriber_free}; use libc::c_char; @@ -57,6 +57,7 @@ use nemo_relay::api::tool as core_tool_api; use nemo_relay::api::tool::ToolAttributes; use nemo_relay::codec::optimization::LlmOptimizationContribution; use nemo_relay::error::Result as FlowResult; +use nemo_relay::plugin::dynamic::{DynamicPluginActivationSpec, PluginHostActivation}; use nemo_relay::plugin::{ ConfigDiagnostic, DiagnosticLevel, Plugin, PluginConfig, PluginError, PluginRegistrationContext, active_plugin_report, clear_plugin_configuration, deregister_plugin, diff --git a/crates/ffi/src/api/plugin.rs b/crates/ffi/src/api/plugin.rs index 51021e09e..ea0e5fadd 100644 --- a/crates/ffi/src/api/plugin.rs +++ b/crates/ffi/src/api/plugin.rs @@ -2,17 +2,18 @@ // SPDX-License-Identifier: Apache-2.0 use super::{ - Arc, CStr, ConfigDiagnostic, DiagnosticLevel, FfiPluginContext, Future, - NemoRelayEventSanitizeCb, NemoRelayEventSubscriberCb, NemoRelayFreeFn, NemoRelayJsonCb, - NemoRelayLlmConditionalCb, NemoRelayLlmExecInterceptCb, NemoRelayLlmRequestCb, - NemoRelayLlmRequestInterceptCb, NemoRelayPluginRegisterCb, NemoRelayPluginValidateCb, - NemoRelayStatus, NemoRelayToolConditionalCb, NemoRelayToolExecInterceptCb, - NemoRelayToolSanitizeCb, Pin, Plugin, PluginConfig, PluginError, PluginRegistrationContext, - active_plugin_report, c_char, c_str_to_json, c_str_to_string, clear_last_error, - clear_plugin_configuration, deregister_plugin, initialize_plugins, json_to_c_string, - last_error_message, list_plugin_kinds, nemo_relay_string_free, register_adaptive_component, - register_plugin, set_last_error, status_from_plugin_error, tokio_runtime, - validate_plugin_config, wrap_event_sanitize_fn, wrap_event_subscriber, wrap_llm_conditional_fn, + Arc, CStr, ConfigDiagnostic, DiagnosticLevel, DynamicPluginActivationSpec, FfiPluginActivation, + FfiPluginContext, Future, NemoRelayEventSanitizeCb, NemoRelayEventSubscriberCb, + NemoRelayFreeFn, NemoRelayJsonCb, NemoRelayLlmConditionalCb, NemoRelayLlmExecInterceptCb, + NemoRelayLlmRequestCb, NemoRelayLlmRequestInterceptCb, NemoRelayPluginRegisterCb, + NemoRelayPluginValidateCb, NemoRelayStatus, NemoRelayToolConditionalCb, + NemoRelayToolExecInterceptCb, NemoRelayToolSanitizeCb, Pin, Plugin, PluginConfig, PluginError, + PluginHostActivation, PluginRegistrationContext, active_plugin_report, c_char, c_str_to_json, + c_str_to_string, clear_last_error, clear_plugin_configuration, deregister_plugin, + initialize_plugins, json_to_c_string, last_error_message, list_plugin_kinds, + nemo_relay_string_free, register_adaptive_component, register_plugin, set_last_error, + status_from_plugin_error, tokio_runtime, validate_plugin_config, wrap_event_sanitize_fn, + wrap_event_subscriber, wrap_llm_conditional_fn, wrap_llm_exec_intercept_fn, wrap_llm_request_intercept_fn, wrap_llm_response_fn, wrap_llm_sanitize_request_fn, wrap_llm_stream_exec_intercept_fn, wrap_tool_conditional_fn, wrap_tool_exec_intercept_fn, wrap_tool_request_intercept_fn, wrap_tool_sanitize_fn, @@ -132,6 +133,143 @@ fn ensure_pii_redaction_component_registered() -> std::result::Result<(), NemoRe register_pii_redaction_component().map_err(|err| status_from_plugin_error(&err)) } +fn parse_plugin_config( + config_json: *const c_char, +) -> std::result::Result { + let value = c_str_to_json(config_json).ok_or(NemoRelayStatus::InvalidJson)?; + serde_json::from_value(value).map_err(|error| { + set_last_error(&format!("invalid plugin config: {error}")); + NemoRelayStatus::InvalidJson + }) +} + +fn parse_dynamic_plugin_specs( + dynamic_plugins_json: *const c_char, +) -> std::result::Result, NemoRelayStatus> { + let value = c_str_to_json(dynamic_plugins_json).ok_or(NemoRelayStatus::InvalidJson)?; + serde_json::from_value(value).map_err(|error| { + set_last_error(&format!("invalid dynamic plugin specifications: {error}")); + NemoRelayStatus::InvalidJson + }) +} + +fn lock_plugin_activation( + activation: *mut FfiPluginActivation, +) -> std::result::Result< + std::sync::MutexGuard<'static, Option>, + NemoRelayStatus, +> { + if activation.is_null() { + return Err(NemoRelayStatus::NullPointer); + } + unsafe { &*activation }.0.lock().map_err(|error| { + set_last_error(&format!("plugin activation lock poisoned: {error}")); + NemoRelayStatus::Internal + }) +} + +/// Load and activate dynamic plugins as one owned transaction. +/// +/// `config_json` is the base [`PluginConfig`] document and +/// `dynamic_plugins_json` is an array of explicit dynamic-plugin activation +/// specifications. On success, the caller owns `out_activation` and must clear +/// and free it with `nemo_relay_plugin_activation_clear` and +/// `nemo_relay_plugin_activation_free`. `out_report_json` is a library-owned C +/// string and must be released with `nemo_relay_string_free`. +/// +/// # Safety +/// Both input pointers must reference valid, null-terminated C strings. +/// `out_activation` and `out_report_json` must be valid, non-null output pointers. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn nemo_relay_activate_dynamic_plugins( + config_json: *const c_char, + dynamic_plugins_json: *const c_char, + out_activation: *mut *mut FfiPluginActivation, + out_report_json: *mut *mut c_char, +) -> NemoRelayStatus { + clear_last_error(); + if !out_activation.is_null() { + unsafe { *out_activation = std::ptr::null_mut() }; + } + if !out_report_json.is_null() { + unsafe { *out_report_json = std::ptr::null_mut() }; + } + if out_activation.is_null() { + set_last_error("out_activation pointer is null"); + return NemoRelayStatus::NullPointer; + } + if out_report_json.is_null() { + set_last_error("out_report_json pointer is null"); + return NemoRelayStatus::NullPointer; + } + + if let Err(status) = ensure_adaptive_component_registered() { + return status; + } + if let Err(status) = ensure_pii_redaction_component_registered() { + return status; + } + let config = match parse_plugin_config(config_json) { + Ok(config) => config, + Err(status) => return status, + }; + let dynamic_plugins = match parse_dynamic_plugin_specs(dynamic_plugins_json) { + Ok(dynamic_plugins) => dynamic_plugins, + Err(status) => return status, + }; + let (activation, report) = + match tokio_runtime().block_on(PluginHostActivation::activate(config, dynamic_plugins)) { + Ok(result) => result, + Err(error) => return status_from_plugin_error(&error), + }; + let report_json = match serde_json::to_value(report) { + Ok(value) => value, + Err(error) => { + let _ = activation.clear(); + set_last_error(&error.to_string()); + return NemoRelayStatus::Internal; + } + }; + + unsafe { + *out_activation = Box::into_raw(Box::new(FfiPluginActivation(std::sync::Mutex::new( + Some(activation), + )))); + *out_report_json = json_to_c_string(&report_json); + } + NemoRelayStatus::Ok +} + +/// Clear one owned dynamic plugin activation. +/// +/// This operation is idempotent. A null handle is treated as already cleared. +/// The handle allocation remains owned by the caller and must still be passed +/// to `nemo_relay_plugin_activation_free`. +/// +/// # Safety +/// `activation` must be a valid activation handle returned by +/// `nemo_relay_activate_dynamic_plugins`, or null. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn nemo_relay_plugin_activation_clear( + activation: *mut FfiPluginActivation, +) -> NemoRelayStatus { + clear_last_error(); + if activation.is_null() { + return NemoRelayStatus::Ok; + } + let mut guard = match lock_plugin_activation(activation) { + Ok(guard) => guard, + Err(status) => return status, + }; + let Some(activation) = guard.take() else { + return NemoRelayStatus::Ok; + }; + match activation.clear() { + Ok(()) => NemoRelayStatus::Ok, + Err(error) => status_from_plugin_error(&error), + } +} + /// Validate a generic plugin config document and return the diagnostics report as JSON. /// /// # Safety diff --git a/crates/ffi/src/types/mod.rs b/crates/ffi/src/types/mod.rs index 20cf03cd1..871811ec9 100644 --- a/crates/ffi/src/types/mod.rs +++ b/crates/ffi/src/types/mod.rs @@ -26,6 +26,7 @@ use nemo_relay::api::scope::{ScopeHandle, ScopeType}; use nemo_relay::api::tool::ToolAttributes; use nemo_relay::api::tool::ToolHandle; use nemo_relay::codec::traits::{LlmCodec, LlmResponseCodec}; +use nemo_relay::plugin::dynamic::PluginHostActivation; use nemo_relay_adaptive::AdaptiveRuntime; use crate::convert::{json_to_c_string, str_to_c_string}; @@ -64,6 +65,11 @@ pub struct FfiOpenInferenceSubscriber( ); /// Opaque owned adaptive runtime handle. pub struct FfiAdaptiveRuntime(pub std::sync::Mutex>); +/// Opaque owned dynamic plugin host activation. +/// +/// The inner option allows explicit activation cleanup to be idempotent while +/// retaining a stable allocation until the foreign caller frees the handle. +pub struct FfiPluginActivation(pub std::sync::Mutex>); /// Opaque plugin registration context. /// /// This wrapper contains a borrowed raw pointer to an @@ -287,6 +293,28 @@ pub unsafe extern "C" fn nemo_relay_adaptive_runtime_free(ptr: *mut FfiAdaptiveR } } +/// Free a dynamic plugin activation handle previously returned by +/// `nemo_relay_activate_dynamic_plugins`. +/// +/// Any activation that has not already been explicitly cleared is cleaned up +/// best-effort by its Rust destructor before the allocation is released. The +/// caller's handle is set to null before cleanup, making repeated calls through +/// the same handle variable safe. +/// +/// # Safety +/// `ptr` must be null or point to a writable activation-handle variable whose +/// value is null or was returned by `nemo_relay_activate_dynamic_plugins`. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn nemo_relay_plugin_activation_free(ptr: *mut *mut FfiPluginActivation) { + if ptr.is_null() { + return; + } + let activation = unsafe { ptr.replace(std::ptr::null_mut()) }; + if !activation.is_null() { + drop(unsafe { Box::from_raw(activation) }); + } +} + /// Free a codec handle previously returned by one of the codec constructor /// functions (`nemo_relay_openai_chat_codec_new`, etc.). /// diff --git a/crates/ffi/tests/integration/api_tests.rs b/crates/ffi/tests/integration/api_tests.rs index 8d9e11b8d..fd934d67b 100644 --- a/crates/ffi/tests/integration/api_tests.rs +++ b/crates/ffi/tests/integration/api_tests.rs @@ -7,7 +7,7 @@ use super::*; use std::ffi::{CStr, CString}; use std::fs; use std::ptr; -use std::sync::{Mutex, OnceLock}; +use std::sync::OnceLock; use std::time::{SystemTime, UNIX_EPOCH}; use nemo_relay::plugin::PluginRegistrationContext; @@ -19,24 +19,24 @@ use nemo_relay_ffi::convert::nemo_relay_string_free; use nemo_relay_ffi::error::{NemoRelayStatus, nemo_relay_last_error, set_last_error}; use nemo_relay_ffi::types::{ FfiAtifExporter, FfiAtofExporter, FfiEvent, FfiLLMHandle, FfiLLMRequest, - FfiOpenTelemetrySubscriber, FfiScopeStack, FfiToolHandle, nemo_relay_atif_exporter_free, - nemo_relay_atof_exporter_free, nemo_relay_event_data, nemo_relay_event_input, - nemo_relay_event_metadata, nemo_relay_event_model_name, nemo_relay_event_name, - nemo_relay_event_output, nemo_relay_event_parent_uuid, nemo_relay_event_scope_type, - nemo_relay_event_timestamp, nemo_relay_event_tool_call_id, nemo_relay_event_uuid, - nemo_relay_llm_handle_attributes, nemo_relay_llm_handle_free, nemo_relay_llm_handle_name, - nemo_relay_llm_handle_parent_uuid, nemo_relay_llm_handle_uuid, nemo_relay_llm_request_content, - nemo_relay_llm_request_free, nemo_relay_llm_request_headers, nemo_relay_llm_request_new, - nemo_relay_otel_subscriber_free, nemo_relay_scope_handle_attributes, - nemo_relay_scope_handle_data, nemo_relay_scope_handle_free, nemo_relay_scope_handle_metadata, - nemo_relay_scope_handle_name, nemo_relay_scope_handle_parent_uuid, - nemo_relay_scope_handle_scope_type, nemo_relay_scope_handle_uuid, nemo_relay_scope_stack_free, - nemo_relay_tool_handle_attributes, nemo_relay_tool_handle_free, nemo_relay_tool_handle_name, - nemo_relay_tool_handle_parent_uuid, nemo_relay_tool_handle_uuid, + FfiOpenTelemetrySubscriber, FfiPluginActivation, FfiScopeStack, FfiToolHandle, + nemo_relay_atif_exporter_free, nemo_relay_atof_exporter_free, nemo_relay_event_data, + nemo_relay_event_input, nemo_relay_event_metadata, nemo_relay_event_model_name, + nemo_relay_event_name, nemo_relay_event_output, nemo_relay_event_parent_uuid, + nemo_relay_event_scope_type, nemo_relay_event_timestamp, nemo_relay_event_tool_call_id, + nemo_relay_event_uuid, nemo_relay_llm_handle_attributes, nemo_relay_llm_handle_free, + nemo_relay_llm_handle_name, nemo_relay_llm_handle_parent_uuid, nemo_relay_llm_handle_uuid, + nemo_relay_llm_request_content, nemo_relay_llm_request_free, nemo_relay_llm_request_headers, + nemo_relay_llm_request_new, nemo_relay_otel_subscriber_free, nemo_relay_plugin_activation_free, + nemo_relay_scope_handle_attributes, nemo_relay_scope_handle_data, nemo_relay_scope_handle_free, + nemo_relay_scope_handle_metadata, nemo_relay_scope_handle_name, + nemo_relay_scope_handle_parent_uuid, nemo_relay_scope_handle_scope_type, + nemo_relay_scope_handle_uuid, nemo_relay_scope_stack_free, nemo_relay_tool_handle_attributes, + nemo_relay_tool_handle_free, nemo_relay_tool_handle_name, nemo_relay_tool_handle_parent_uuid, + nemo_relay_tool_handle_uuid, }; use nemo_relay_ffi::{api, callable, types}; -static TEST_MUTEX: Mutex<()> = Mutex::new(()); static EVENT_LOG: OnceLock>> = OnceLock::new(); static COLLECTED_CHUNKS: OnceLock>> = OnceLock::new(); static FINALIZER_CALLS: OnceLock> = OnceLock::new(); diff --git a/crates/ffi/tests/integration/main.rs b/crates/ffi/tests/integration/main.rs index 04e80d2d4..1aacee267 100644 --- a/crates/ffi/tests/integration/main.rs +++ b/crates/ffi/tests/integration/main.rs @@ -20,9 +20,11 @@ use nemo_relay_ffi::{api, convert, error}; use serde_json::{Value as Json, json}; use std::ffi::{CStr, CString}; use std::pin::Pin; -use std::sync::Arc; +use std::sync::{Arc, Mutex}; use tokio_stream::Stream; +static TEST_MUTEX: Mutex<()> = Mutex::new(()); + unsafe fn nemo_relay_string_free_internal(ptr: *mut c_char) { unsafe { nemo_relay_string_free(ptr) }; } @@ -35,5 +37,6 @@ mod callable_tests; mod convert_coverage_tests; #[path = "../coverage/error_tests.rs"] mod error_coverage_tests; +mod plugin_activation_tests; #[path = "../unit/types_tests.rs"] mod types_tests; diff --git a/crates/ffi/tests/integration/plugin_activation_tests.rs b/crates/ffi/tests/integration/plugin_activation_tests.rs new file mode 100644 index 000000000..f3a9d30d0 --- /dev/null +++ b/crates/ffi/tests/integration/plugin_activation_tests.rs @@ -0,0 +1,387 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +use super::*; +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::ptr; +use std::sync::OnceLock; + +use nemo_relay_ffi::types::{FfiPluginActivation, nemo_relay_plugin_activation_free}; +use tempfile::TempDir; + +#[test] +fn ffi_activation_loads_native_callbacks_and_removes_them_before_free() { + let _guard = TEST_MUTEX.lock().unwrap(); + let _ = nemo_relay_clear_plugin_configuration(); + + let manifest_dir = TempDir::new().expect("native manifest tempdir"); + let manifest = write_native_manifest(manifest_dir.path(), build_native_fixture()); + let (mut activation, report) = activate_plugins(json!([{ + "plugin_id": "fixture_native", + "kind": "rust_dynamic", + "manifest_ref": manifest, + "config": {} + }])); + assert_eq!(report["diagnostics"], json!([])); + assert!(plugin_kinds().iter().any(|kind| kind == "fixture_native")); + + assert_eq!( + tool_request_intercepts("ffi-native-tool", json!({"input": true}))["native_plugin"], + true + ); + + unsafe { + assert_eq!( + api::nemo_relay_plugin_activation_clear(activation), + NemoRelayStatus::Ok + ); + assert_eq!( + api::nemo_relay_plugin_activation_clear(activation), + NemoRelayStatus::Ok + ); + nemo_relay_plugin_activation_free(&mut activation); + } + assert!(!plugin_kinds().iter().any(|kind| kind == "fixture_native")); + assert_eq!( + tool_request_intercepts("ffi-native-tool", json!({"input": true})), + json!({"input": true}) + ); + + let (mut drop_activation, _) = activate_plugins(json!([{ + "plugin_id": "fixture_native", + "kind": "rust_dynamic", + "manifest_ref": manifest, + "config": {} + }])); + assert_eq!( + tool_request_intercepts("ffi-native-tool", json!({"input": true}))["native_plugin"], + true + ); + unsafe { nemo_relay_plugin_activation_free(&mut drop_activation) }; + assert_eq!( + tool_request_intercepts("ffi-native-tool", json!({"input": true})), + json!({"input": true}) + ); +} + +#[test] +fn ffi_activation_loads_worker_callbacks_and_stops_worker_on_clear() { + let _guard = TEST_MUTEX.lock().unwrap(); + let _ = nemo_relay_clear_plugin_configuration(); + + let manifest_dir = TempDir::new().expect("worker manifest tempdir"); + let manifest = write_worker_manifest(manifest_dir.path(), build_worker_fixture()); + let (mut activation, report) = activate_plugins(json!([{ + "plugin_id": "fixture_worker", + "kind": "worker", + "manifest_ref": manifest, + "config": {} + }])); + assert_eq!(report["diagnostics"], json!([])); + assert!(plugin_kinds().iter().any(|kind| kind == "fixture_worker")); + assert_eq!( + tool_request_intercepts("ffi-worker-tool", json!({"input": true}))["worker_plugin"], + true + ); + + unsafe { + assert_eq!( + api::nemo_relay_plugin_activation_clear(activation), + NemoRelayStatus::Ok + ); + nemo_relay_plugin_activation_free(&mut activation); + } + assert!(!plugin_kinds().iter().any(|kind| kind == "fixture_worker")); + assert_eq!( + tool_request_intercepts("ffi-worker-tool", json!({"input": true})), + json!({"input": true}) + ); +} + +#[test] +fn ffi_activation_rolls_back_an_earlier_native_load_when_a_later_load_fails() { + let _guard = TEST_MUTEX.lock().unwrap(); + let _ = nemo_relay_clear_plugin_configuration(); + + let manifest_dir = TempDir::new().expect("native manifest tempdir"); + let manifest = write_native_manifest(manifest_dir.path(), build_native_fixture()); + let missing_manifest = manifest_dir.path().join("missing-relay-plugin.toml"); + let config = cstring(r#"{"version":1,"components":[]}"#); + let specs = cstring( + &json!([ + { + "plugin_id": "fixture_native", + "kind": "rust_dynamic", + "manifest_ref": manifest, + "config": {} + }, + { + "plugin_id": "fixture_missing", + "kind": "rust_dynamic", + "manifest_ref": missing_manifest, + "config": {} + } + ]) + .to_string(), + ); + let mut activation = ptr::null_mut(); + let mut report = ptr::null_mut(); + let status = unsafe { + api::nemo_relay_activate_dynamic_plugins( + config.as_ptr(), + specs.as_ptr(), + &mut activation, + &mut report, + ) + }; + assert_eq!(status, NemoRelayStatus::NotFound); + assert!(activation.is_null()); + assert!(report.is_null()); + assert!(!plugin_kinds().iter().any(|kind| kind == "fixture_native")); + assert_eq!( + tool_request_intercepts("ffi-native-tool", json!({"input": true})), + json!({"input": true}) + ); + + let (mut activation, _) = activate_plugins(json!([{ + "plugin_id": "fixture_native", + "kind": "rust_dynamic", + "manifest_ref": manifest, + "config": {} + }])); + unsafe { + assert_eq!( + api::nemo_relay_plugin_activation_clear(activation), + NemoRelayStatus::Ok + ); + nemo_relay_plugin_activation_free(&mut activation); + } +} + +fn activate_plugins(specs: Json) -> (*mut FfiPluginActivation, Json) { + let config = cstring(r#"{"version":1,"components":[]}"#); + let specs = cstring(&specs.to_string()); + let mut activation = ptr::null_mut(); + let mut report = ptr::null_mut(); + let status = unsafe { + api::nemo_relay_activate_dynamic_plugins( + config.as_ptr(), + specs.as_ptr(), + &mut activation, + &mut report, + ) + }; + assert_eq!( + status, + NemoRelayStatus::Ok, + "activation failed: {:?}", + unsafe { read_last_error() } + ); + assert!(!activation.is_null()); + (activation, unsafe { returned_json(report) }) +} + +fn cstring(value: &str) -> CString { + CString::new(value).expect("C string") +} + +unsafe fn read_last_error() -> Option { + let pointer = nemo_relay_last_error(); + (!pointer.is_null()).then(|| { + unsafe { CStr::from_ptr(pointer) } + .to_string_lossy() + .into_owned() + }) +} + +unsafe fn returned_json(pointer: *mut c_char) -> Json { + assert!(!pointer.is_null(), "expected returned JSON string"); + let json = unsafe { CStr::from_ptr(pointer) } + .to_string_lossy() + .into_owned(); + unsafe { nemo_relay_string_free(pointer) }; + serde_json::from_str(&json).expect("returned JSON") +} + +fn tool_request_intercepts(name: &str, args: Json) -> Json { + let name = cstring(name); + let args = cstring(&args.to_string()); + let mut output = ptr::null_mut(); + let status = unsafe { + api::nemo_relay_tool_request_intercepts(name.as_ptr(), args.as_ptr(), &mut output) + }; + assert_eq!( + status, + NemoRelayStatus::Ok, + "tool request intercept failed: {:?}", + unsafe { read_last_error() } + ); + unsafe { returned_json(output) } +} + +fn plugin_kinds() -> Vec { + let mut output = ptr::null_mut(); + assert_eq!( + unsafe { api::nemo_relay_list_plugin_kinds_json(&mut output) }, + NemoRelayStatus::Ok + ); + serde_json::from_value(unsafe { returned_json(output) }).expect("plugin kinds JSON") +} + +fn build_native_fixture() -> &'static Path { + static FIXTURE: OnceLock = OnceLock::new(); + FIXTURE.get_or_init(|| { + let source_dir = TempDir::new().expect("native fixture source tempdir"); + let fixture_dir = source_dir.path().join("native_plugin"); + let source = fixture_dir.join("src"); + std::fs::create_dir_all(&source).expect("native fixture src dir"); + let plugin_path = Path::new(env!("CARGO_MANIFEST_DIR")).join("../plugin"); + let manifest_template = std::fs::read_to_string( + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../core/tests/fixtures/native_plugin/Cargo.toml"), + ) + .expect("native fixture Cargo.toml"); + let manifest = manifest_template.replace( + r#"nemo-relay-plugin = { path = "../../../../plugin" }"#, + &format!("nemo-relay-plugin = {{ path = {plugin_path:?} }}"), + ); + std::fs::write(fixture_dir.join("Cargo.toml"), manifest) + .expect("write native fixture Cargo.toml"); + std::fs::copy( + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../core/tests/fixtures/native_plugin/src/lib.rs"), + source.join("lib.rs"), + ) + .expect("copy native fixture source"); + + let target = + Path::new(env!("CARGO_MANIFEST_DIR")).join("../../target/ffi-native-plugin-fixture"); + let status = Command::new(std::env::var("CARGO").unwrap_or_else(|_| "cargo".into())) + .arg("build") + .arg("--quiet") + .arg("--manifest-path") + .arg(fixture_dir.join("Cargo.toml")) + .arg("--target-dir") + .arg(&target) + .status() + .expect("native fixture build should start"); + assert!(status.success(), "native fixture build failed: {status}"); + let library = target.join("debug").join(native_library_name()); + assert!( + library.exists(), + "missing native fixture: {}", + library.display() + ); + library + }) +} + +fn build_worker_fixture() -> &'static Path { + static FIXTURE: OnceLock = OnceLock::new(); + FIXTURE.get_or_init(|| { + let manifest = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../core/tests/fixtures/worker_plugin/Cargo.toml"); + let target = + Path::new(env!("CARGO_MANIFEST_DIR")).join("../../target/ffi-worker-plugin-fixture"); + let status = Command::new(std::env::var("CARGO").unwrap_or_else(|_| "cargo".into())) + .arg("build") + .arg("--quiet") + .arg("--locked") + .arg("--manifest-path") + .arg(manifest) + .arg("--target-dir") + .arg(&target) + .status() + .expect("worker fixture build should start"); + assert!(status.success(), "worker fixture build failed: {status}"); + let binary = target.join("debug").join(format!( + "nemo-relay-worker-plugin-fixture{}", + std::env::consts::EXE_SUFFIX + )); + assert!( + binary.exists(), + "missing worker fixture: {}", + binary.display() + ); + binary + }) +} + +fn write_native_manifest(directory: &Path, library: &Path) -> PathBuf { + let manifest = directory.join("relay-plugin.toml"); + std::fs::write( + &manifest, + format!( + r#" +manifest_version = 1 + +[plugin] +id = "fixture_native" +kind = "rust_dynamic" + +[compat] +relay = "={version}" +native_api = "1" + +[defaults] +enabled = false + +[capabilities] +items = ["plugin_native"] + +[load] +library = {library:?} +symbol = "nemo_relay_fixture_native_plugin" +"#, + version = env!("CARGO_PKG_VERSION"), + library = library.to_string_lossy(), + ), + ) + .expect("write native fixture manifest"); + manifest +} + +fn write_worker_manifest(directory: &Path, binary: &Path) -> PathBuf { + let manifest = directory.join("relay-plugin.toml"); + std::fs::write( + &manifest, + format!( + r#" +manifest_version = 1 + +[plugin] +id = "fixture_worker" +kind = "worker" + +[compat] +relay = "={version}" +worker_protocol = "grpc-v1" + +[defaults] +enabled = false + +[capabilities] +items = ["plugin_worker"] + +[load] +runtime = "rust" +entrypoint = {entrypoint:?} +"#, + version = env!("CARGO_PKG_VERSION"), + entrypoint = binary.to_string_lossy(), + ), + ) + .expect("write worker fixture manifest"); + manifest +} + +fn native_library_name() -> &'static str { + if cfg!(target_os = "windows") { + "nemo_relay_plugin_fixture.dll" + } else if cfg!(target_os = "macos") { + "libnemo_relay_plugin_fixture.dylib" + } else { + "libnemo_relay_plugin_fixture.so" + } +} diff --git a/crates/ffi/tests/unit/api/plugin_tests.rs b/crates/ffi/tests/unit/api/plugin_tests.rs index 2d37db847..d151451f8 100644 --- a/crates/ffi/tests/unit/api/plugin_tests.rs +++ b/crates/ffi/tests/unit/api/plugin_tests.rs @@ -5,6 +5,238 @@ use super::*; +#[test] +fn test_ffi_dynamic_plugin_activation_owns_an_idempotent_lifecycle() { + let _guard = TEST_MUTEX.lock().unwrap(); + reset_globals(); + let _ = nemo_relay_clear_plugin_configuration(); + + let config = cstring(r#"{"version":1,"components":[]}"#); + let specs = cstring("[]"); + let mut activation = ptr::null_mut(); + let mut report_json = ptr::null_mut(); + unsafe { + assert_eq!( + nemo_relay_activate_dynamic_plugins( + config.as_ptr(), + specs.as_ptr(), + &mut activation, + &mut report_json, + ), + NemoRelayStatus::Ok + ); + assert!(!activation.is_null()); + assert_eq!(returned_json(report_json)["diagnostics"], json!([])); + + let mut conflicting_activation = ptr::null_mut(); + let mut conflicting_report = ptr::null_mut(); + assert_eq!( + nemo_relay_activate_dynamic_plugins( + config.as_ptr(), + specs.as_ptr(), + &mut conflicting_activation, + &mut conflicting_report, + ), + NemoRelayStatus::AlreadyExists + ); + assert!(conflicting_activation.is_null()); + assert!(conflicting_report.is_null()); + assert!( + read_last_error() + .unwrap_or_default() + .contains("active dynamic plugin host") + ); + + assert_eq!( + nemo_relay_plugin_activation_clear(activation), + NemoRelayStatus::Ok + ); + assert_eq!( + nemo_relay_plugin_activation_clear(activation), + NemoRelayStatus::Ok + ); + assert_eq!( + nemo_relay_plugin_activation_clear(ptr::null_mut()), + NemoRelayStatus::Ok + ); + nemo_relay_plugin_activation_free(&mut activation); + assert!(activation.is_null()); + nemo_relay_plugin_activation_free(&mut activation); + nemo_relay_plugin_activation_free(ptr::null_mut()); + + let mut dropped_activation = ptr::null_mut(); + let mut dropped_report = ptr::null_mut(); + assert_eq!( + nemo_relay_activate_dynamic_plugins( + config.as_ptr(), + specs.as_ptr(), + &mut dropped_activation, + &mut dropped_report, + ), + NemoRelayStatus::Ok + ); + returned_json(dropped_report); + nemo_relay_plugin_activation_free(&mut dropped_activation); + assert!(dropped_activation.is_null()); + + let mut after_drop_activation = ptr::null_mut(); + let mut after_drop_report = ptr::null_mut(); + assert_eq!( + nemo_relay_activate_dynamic_plugins( + config.as_ptr(), + specs.as_ptr(), + &mut after_drop_activation, + &mut after_drop_report, + ), + NemoRelayStatus::Ok + ); + returned_json(after_drop_report); + assert_eq!( + nemo_relay_plugin_activation_clear(after_drop_activation), + NemoRelayStatus::Ok + ); + nemo_relay_plugin_activation_free(&mut after_drop_activation); + } +} + +#[test] +fn test_ffi_dynamic_plugin_activation_rejects_invalid_inputs_without_outputs() { + let _guard = TEST_MUTEX.lock().unwrap(); + reset_globals(); + let _ = nemo_relay_clear_plugin_configuration(); + + let config = cstring(r#"{"version":1,"components":[]}"#); + let specs = cstring("[]"); + let invalid = cstring("not-json"); + unsafe { + let mut report = std::ptr::dangling_mut::(); + assert_eq!( + nemo_relay_activate_dynamic_plugins( + config.as_ptr(), + specs.as_ptr(), + ptr::null_mut(), + &mut report, + ), + NemoRelayStatus::NullPointer + ); + assert!(report.is_null()); + + let mut activation = std::ptr::dangling_mut::(); + assert_eq!( + nemo_relay_activate_dynamic_plugins( + config.as_ptr(), + specs.as_ptr(), + &mut activation, + ptr::null_mut(), + ), + NemoRelayStatus::NullPointer + ); + assert!(activation.is_null()); + + for (config_json, specs_json, expected_error) in [ + (invalid.as_ptr(), specs.as_ptr(), "invalid JSON"), + (config.as_ptr(), invalid.as_ptr(), "invalid JSON"), + ] { + let mut activation = std::ptr::dangling_mut::(); + let mut report = std::ptr::dangling_mut::(); + assert_eq!( + nemo_relay_activate_dynamic_plugins( + config_json, + specs_json, + &mut activation, + &mut report, + ), + NemoRelayStatus::InvalidJson + ); + assert!(activation.is_null()); + assert!(report.is_null()); + assert!( + read_last_error() + .unwrap_or_default() + .contains(expected_error) + ); + } + + let invalid_shape = cstring(r#"{"plugin_id":"not-an-array"}"#); + let mut activation = ptr::null_mut(); + let mut report = ptr::null_mut(); + assert_eq!( + nemo_relay_activate_dynamic_plugins( + config.as_ptr(), + invalid_shape.as_ptr(), + &mut activation, + &mut report, + ), + NemoRelayStatus::InvalidJson + ); + assert!( + read_last_error() + .unwrap_or_default() + .contains("invalid dynamic plugin specifications") + ); + } +} + +#[test] +fn test_ffi_dynamic_plugin_activation_surfaces_load_failures_and_releases_owner() { + let _guard = TEST_MUTEX.lock().unwrap(); + reset_globals(); + let _ = nemo_relay_clear_plugin_configuration(); + + let config = cstring(r#"{"version":1,"components":[]}"#); + let missing_manifest = std::env::temp_dir() + .join(format!("missing-relay-plugin-{}.toml", Uuid::now_v7())) + .to_string_lossy() + .into_owned(); + let specs = cstring( + &json!([{ + "plugin_id": "fixture_missing", + "kind": "rust_dynamic", + "manifest_ref": missing_manifest, + "config": {} + }]) + .to_string(), + ); + + unsafe { + let mut activation = ptr::null_mut(); + let mut report = ptr::null_mut(); + assert_eq!( + nemo_relay_activate_dynamic_plugins( + config.as_ptr(), + specs.as_ptr(), + &mut activation, + &mut report, + ), + NemoRelayStatus::NotFound + ); + assert!(activation.is_null()); + assert!(report.is_null()); + assert!( + read_last_error() + .unwrap_or_default() + .contains("native plugin load failed") + ); + + let empty_specs = cstring("[]"); + assert_eq!( + nemo_relay_activate_dynamic_plugins( + config.as_ptr(), + empty_specs.as_ptr(), + &mut activation, + &mut report, + ), + NemoRelayStatus::Ok + ); + returned_json(report); + assert_eq!( + nemo_relay_plugin_activation_clear(activation), + NemoRelayStatus::Ok + ); + nemo_relay_plugin_activation_free(&mut activation); + } +} + #[test] fn test_ffi_plugin_registration_validation_and_cleanup() { let _guard = TEST_MUTEX.lock().unwrap(); diff --git a/crates/ffi/tests/unit/api_tests.rs b/crates/ffi/tests/unit/api_tests.rs index cf89acd7e..b95b65be4 100644 --- a/crates/ffi/tests/unit/api_tests.rs +++ b/crates/ffi/tests/unit/api_tests.rs @@ -17,20 +17,20 @@ use crate::convert::nemo_relay_string_free; use crate::error::{NemoRelayStatus, nemo_relay_last_error}; use crate::types::{ FfiAtifExporter, FfiEvent, FfiLLMHandle, FfiLLMRequest, FfiOpenTelemetrySubscriber, - FfiScopeStack, FfiToolHandle, nemo_relay_atif_exporter_free, nemo_relay_event_data, - nemo_relay_event_input, nemo_relay_event_metadata, nemo_relay_event_model_name, - nemo_relay_event_name, nemo_relay_event_output, nemo_relay_event_parent_uuid, - nemo_relay_event_scope_type, nemo_relay_event_timestamp, nemo_relay_event_tool_call_id, - nemo_relay_event_uuid, nemo_relay_llm_handle_attributes, nemo_relay_llm_handle_free, - nemo_relay_llm_handle_name, nemo_relay_llm_handle_parent_uuid, nemo_relay_llm_handle_uuid, - nemo_relay_llm_request_content, nemo_relay_llm_request_free, nemo_relay_llm_request_headers, - nemo_relay_llm_request_new, nemo_relay_otel_subscriber_free, - nemo_relay_scope_handle_attributes, nemo_relay_scope_handle_data, nemo_relay_scope_handle_free, - nemo_relay_scope_handle_metadata, nemo_relay_scope_handle_name, - nemo_relay_scope_handle_parent_uuid, nemo_relay_scope_handle_scope_type, - nemo_relay_scope_handle_uuid, nemo_relay_scope_stack_free, nemo_relay_tool_handle_attributes, - nemo_relay_tool_handle_free, nemo_relay_tool_handle_name, nemo_relay_tool_handle_parent_uuid, - nemo_relay_tool_handle_uuid, + FfiPluginActivation, FfiScopeStack, FfiToolHandle, nemo_relay_atif_exporter_free, + nemo_relay_event_data, nemo_relay_event_input, nemo_relay_event_metadata, + nemo_relay_event_model_name, nemo_relay_event_name, nemo_relay_event_output, + nemo_relay_event_parent_uuid, nemo_relay_event_scope_type, nemo_relay_event_timestamp, + nemo_relay_event_tool_call_id, nemo_relay_event_uuid, nemo_relay_llm_handle_attributes, + nemo_relay_llm_handle_free, nemo_relay_llm_handle_name, nemo_relay_llm_handle_parent_uuid, + nemo_relay_llm_handle_uuid, nemo_relay_llm_request_content, nemo_relay_llm_request_free, + nemo_relay_llm_request_headers, nemo_relay_llm_request_new, nemo_relay_otel_subscriber_free, + nemo_relay_plugin_activation_free, nemo_relay_scope_handle_attributes, + nemo_relay_scope_handle_data, nemo_relay_scope_handle_free, nemo_relay_scope_handle_metadata, + nemo_relay_scope_handle_name, nemo_relay_scope_handle_parent_uuid, + nemo_relay_scope_handle_scope_type, nemo_relay_scope_handle_uuid, nemo_relay_scope_stack_free, + nemo_relay_tool_handle_attributes, nemo_relay_tool_handle_free, nemo_relay_tool_handle_name, + nemo_relay_tool_handle_parent_uuid, nemo_relay_tool_handle_uuid, }; use crate::{api, callable, types}; diff --git a/go/nemo_relay/plugin.go b/go/nemo_relay/plugin.go index 1ec43b80e..07a19daa3 100644 --- a/go/nemo_relay/plugin.go +++ b/go/nemo_relay/plugin.go @@ -8,6 +8,7 @@ package nemo_relay #include typedef struct FfiPluginContext FfiPluginContext; +typedef struct FfiPluginActivation FfiPluginActivation; typedef void (*NemoRelayFreeFn)(void* user_data); typedef char* (*NemoRelayPluginValidateCb)(void* user_data, const char* plugin_config_json); @@ -27,6 +28,9 @@ typedef char* (*NemoRelayToolExecInterceptCb)(void* user_data, const char* args_ extern int32_t nemo_relay_validate_plugin_config(const char* config_json, char** out_json); extern int32_t nemo_relay_initialize_plugins(const char* config_json, char** out_json); +extern int32_t nemo_relay_activate_dynamic_plugins(const char* config_json, const char* dynamic_plugins_json, FfiPluginActivation** out_activation, char** out_report_json); +extern int32_t nemo_relay_plugin_activation_clear(FfiPluginActivation* activation); +extern void nemo_relay_plugin_activation_free(FfiPluginActivation** activation); extern int32_t nemo_relay_clear_plugin_configuration(void); extern int32_t nemo_relay_active_plugin_report_json(char** out_json); extern int32_t nemo_relay_list_plugin_kinds_json(char** out_json); @@ -68,6 +72,8 @@ import "C" import ( "errors" + "runtime" + "sync" "unsafe" ) @@ -108,6 +114,55 @@ var ( C.nemo_relay_string_free(out) }) } + activateDynamicPluginsJSON = func(configJSON, dynamicPluginsJSON string) (unsafe.Pointer, string, error) { + cConfig := C.CString(configJSON) + cDynamicPlugins := C.CString(dynamicPluginsJSON) + defer C.free(unsafe.Pointer(cConfig)) + defer C.free(unsafe.Pointer(cDynamicPlugins)) + runtime.LockOSThread() + defer runtime.UnlockOSThread() + + var activation *C.FfiPluginActivation + var report *C.char + status := C.nemo_relay_activate_dynamic_plugins( + cConfig, + cDynamicPlugins, + &activation, + &report, + ) + if err := checkStatus(status); err != nil { + if report != nil { + C.nemo_relay_string_free(report) + } + if activation != nil { + C.nemo_relay_plugin_activation_clear(activation) + C.nemo_relay_plugin_activation_free(&activation) + } + return nil, "", err + } + if activation == nil || report == nil { + if report != nil { + C.nemo_relay_string_free(report) + } + if activation != nil { + C.nemo_relay_plugin_activation_clear(activation) + C.nemo_relay_plugin_activation_free(&activation) + } + return nil, "", errors.New("dynamic plugin activation returned incomplete outputs") + } + defer C.nemo_relay_string_free(report) + return unsafe.Pointer(activation), C.GoString(report), nil + } + clearPluginActivation = func(ptr unsafe.Pointer) error { + runtime.LockOSThread() + defer runtime.UnlockOSThread() + status := C.nemo_relay_plugin_activation_clear((*C.FfiPluginActivation)(ptr)) + return checkStatus(status) + } + freePluginActivation = func(ptr unsafe.Pointer) { + activation := (*C.FfiPluginActivation)(ptr) + C.nemo_relay_plugin_activation_free(&activation) + } activePluginReportJSON = func() (string, error) { var out *C.char status := C.nemo_relay_active_plugin_report_json(&out) @@ -176,6 +231,35 @@ type PluginConfig struct { Policy *ConfigPolicy `json:"policy,omitempty"` } +// DynamicPluginKind identifies the runtime lane used by a dynamic plugin. +type DynamicPluginKind string + +const ( + // DynamicPluginKindRustDynamic loads an ABI-compatible native shared library. + DynamicPluginKindRustDynamic DynamicPluginKind = "rust_dynamic" + // DynamicPluginKindWorker starts an isolated worker plugin runtime. + DynamicPluginKindWorker DynamicPluginKind = "worker" +) + +// DynamicPluginActivationSpec describes one explicitly resolved plugin to load. +// ManifestRef and EnvironmentRef are resolved by the embedding application; +// Relay does not perform installation-state discovery through this API. +type DynamicPluginActivationSpec struct { + PluginID string `json:"plugin_id"` + Kind DynamicPluginKind `json:"kind"` + ManifestRef string `json:"manifest_ref"` + EnvironmentRef *string `json:"environment_ref,omitempty"` + Config map[string]any `json:"config,omitempty"` +} + +// PluginActivation owns the runtime registrations, native libraries, and +// workers created by ActivateDynamicPlugins. A PluginActivation must not be +// copied after first use. +type PluginActivation struct { + mu sync.Mutex + ptr unsafe.Pointer +} + // PluginContext is the component-scoped registration context passed to plugins. type PluginContext struct { ptr *C.FfiPluginContext @@ -262,6 +346,66 @@ func InitializePlugins(config PluginConfig) (ConfigReport, error) { return report, nil } +// ActivateDynamicPlugins loads explicit dynamic plugins and activates them +// together with the supplied base plugin configuration as one transaction. +// +// The caller must retain the returned activation for as long as plugin +// callbacks may run and call Close during orderly shutdown. +func ActivateDynamicPlugins( + config PluginConfig, + dynamicPlugins []DynamicPluginActivationSpec, +) (*PluginActivation, ConfigReport, error) { + configPayload, err := jsonMarshal(config) + if err != nil { + return nil, ConfigReport{}, err + } + if dynamicPlugins == nil { + dynamicPlugins = []DynamicPluginActivationSpec{} + } + dynamicPluginsPayload, err := jsonMarshal(dynamicPlugins) + if err != nil { + return nil, ConfigReport{}, err + } + + ptr, rawReport, err := activateDynamicPluginsJSON( + string(configPayload), + string(dynamicPluginsPayload), + ) + if err != nil { + return nil, ConfigReport{}, err + } + activation := &PluginActivation{ptr: ptr} + var report ConfigReport + if err := jsonUnmarshal([]byte(rawReport), &report); err != nil { + _ = activation.Close() + return nil, ConfigReport{}, err + } + runtime.SetFinalizer(activation, func(activation *PluginActivation) { + _ = activation.Close() + }) + return activation, report, nil +} + +// Close removes callbacks and subscribers before unloading plugin libraries +// and workers. It is safe to call Close repeatedly or on a nil activation. +func (activation *PluginActivation) Close() error { + if activation == nil { + return nil + } + activation.mu.Lock() + defer activation.mu.Unlock() + if activation.ptr == nil { + return nil + } + + runtime.SetFinalizer(activation, nil) + ptr := activation.ptr + activation.ptr = nil + err := clearPluginActivation(ptr) + freePluginActivation(ptr) + return err +} + // ClearPluginConfiguration removes all active plugin component registrations. // // Registered plugin kinds remain available for future validation or diff --git a/go/nemo_relay/plugin_activation_test.go b/go/nemo_relay/plugin_activation_test.go new file mode 100644 index 000000000..dd1388ac2 --- /dev/null +++ b/go/nemo_relay/plugin_activation_test.go @@ -0,0 +1,557 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package nemo_relay + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "sync" + "testing" + "time" + "unsafe" +) + +var ( + goNativePluginFixtureOnce sync.Once + goNativePluginFixturePath string + goNativePluginFixtureErr error + goWorkerPluginFixtureOnce sync.Once + goWorkerPluginFixturePath string + goWorkerPluginFixtureErr error +) + +func withPluginActivationStubs(t *testing.T) { + t.Helper() + originalActivate := activateDynamicPluginsJSON + originalClear := clearPluginActivation + originalFree := freePluginActivation + t.Cleanup(func() { + activateDynamicPluginsJSON = originalActivate + clearPluginActivation = originalClear + freePluginActivation = originalFree + }) +} + +func TestActivateDynamicPluginsSerializesSpecsAndOwnsCleanup(t *testing.T) { + withPluginActivationStubs(t) + + token := new(byte) + ptr := unsafe.Pointer(token) + var gotConfig PluginConfig + var gotSpecs []DynamicPluginActivationSpec + activateDynamicPluginsJSON = func(configJSON, specsJSON string) (unsafe.Pointer, string, error) { + if err := json.Unmarshal([]byte(configJSON), &gotConfig); err != nil { + t.Fatalf("invalid config JSON: %v", err) + } + if err := json.Unmarshal([]byte(specsJSON), &gotSpecs); err != nil { + t.Fatalf("invalid specs JSON: %v", err) + } + return ptr, `{"diagnostics":[{"level":"warning","code":"fixture.warning","message":"fixture"}]}`, nil + } + var calls []string + clearPluginActivation = func(got unsafe.Pointer) error { + if got != ptr { + t.Fatalf("clear pointer = %p, want %p", got, ptr) + } + calls = append(calls, "clear") + return nil + } + freePluginActivation = func(got unsafe.Pointer) { + if got != ptr { + t.Fatalf("free pointer = %p, want %p", got, ptr) + } + calls = append(calls, "free") + } + + environment := "/tmp/fixture-environment" + activation, report, err := ActivateDynamicPlugins(NewPluginConfig(), []DynamicPluginActivationSpec{ + { + PluginID: "fixture.worker", + Kind: DynamicPluginKindWorker, + ManifestRef: "/tmp/relay-plugin.toml", + EnvironmentRef: &environment, + Config: map[string]any{"tag": "go"}, + }, + }) + if err != nil { + t.Fatalf("ActivateDynamicPlugins() error = %v", err) + } + if gotConfig.Version != 1 { + t.Fatalf("config version = %d, want 1", gotConfig.Version) + } + if len(gotSpecs) != 1 || gotSpecs[0].PluginID != "fixture.worker" { + t.Fatalf("specs = %#v", gotSpecs) + } + if gotSpecs[0].EnvironmentRef == nil || *gotSpecs[0].EnvironmentRef != environment { + t.Fatalf("environment ref = %#v", gotSpecs[0].EnvironmentRef) + } + if len(report.Diagnostics) != 1 || report.Diagnostics[0].Code != "fixture.warning" { + t.Fatalf("report = %#v", report) + } + + if err := activation.Close(); err != nil { + t.Fatalf("Close() error = %v", err) + } + if err := activation.Close(); err != nil { + t.Fatalf("repeated Close() error = %v", err) + } + if strings.Join(calls, ",") != "clear,free" { + t.Fatalf("cleanup calls = %v", calls) + } + runtime.KeepAlive(token) +} + +func TestActivateDynamicPluginsNormalizesNilSpecs(t *testing.T) { + withPluginActivationStubs(t) + + token := new(byte) + activateDynamicPluginsJSON = func(_ string, specsJSON string) (unsafe.Pointer, string, error) { + if specsJSON != "[]" { + t.Fatalf("nil specs encoded as %q, want []", specsJSON) + } + return unsafe.Pointer(token), `{"diagnostics":[]}`, nil + } + clearPluginActivation = func(unsafe.Pointer) error { return nil } + freePluginActivation = func(unsafe.Pointer) {} + + activation, _, err := ActivateDynamicPlugins(NewPluginConfig(), nil) + if err != nil { + t.Fatalf("ActivateDynamicPlugins() error = %v", err) + } + if err := activation.Close(); err != nil { + t.Fatalf("Close() error = %v", err) + } + runtime.KeepAlive(token) +} + +func TestActivateDynamicPluginsCleansUpInvalidReport(t *testing.T) { + withPluginActivationStubs(t) + + token := new(byte) + ptr := unsafe.Pointer(token) + activateDynamicPluginsJSON = func(string, string) (unsafe.Pointer, string, error) { + return ptr, "not-json", nil + } + var calls []string + clearPluginActivation = func(unsafe.Pointer) error { + calls = append(calls, "clear") + return nil + } + freePluginActivation = func(unsafe.Pointer) { calls = append(calls, "free") } + + activation, _, err := ActivateDynamicPlugins(NewPluginConfig(), nil) + if err == nil { + t.Fatal("ActivateDynamicPlugins() error = nil, want invalid report error") + } + if activation != nil { + t.Fatalf("activation = %#v, want nil", activation) + } + if strings.Join(calls, ",") != "clear,free" { + t.Fatalf("cleanup calls = %v", calls) + } + runtime.KeepAlive(token) +} + +func TestPluginActivationCloseFreesAfterClearFailure(t *testing.T) { + withPluginActivationStubs(t) + + token := new(byte) + ptr := unsafe.Pointer(token) + wantErr := errors.New("teardown failed") + var calls []string + clearPluginActivation = func(unsafe.Pointer) error { + calls = append(calls, "clear") + return wantErr + } + freePluginActivation = func(unsafe.Pointer) { calls = append(calls, "free") } + activation := &PluginActivation{ptr: ptr} + + if err := activation.Close(); !errors.Is(err, wantErr) { + t.Fatalf("Close() error = %v, want %v", err, wantErr) + } + if err := activation.Close(); err != nil { + t.Fatalf("repeated Close() error = %v", err) + } + if strings.Join(calls, ",") != "clear,free" { + t.Fatalf("cleanup calls = %v", calls) + } + runtime.KeepAlive(token) +} + +func TestActivateDynamicPluginsSurfacesSerializationAndActivationErrors(t *testing.T) { + withPluginActivationStubs(t) + + activationCalls := 0 + activateDynamicPluginsJSON = func(string, string) (unsafe.Pointer, string, error) { + activationCalls++ + return nil, "", errors.New("load failed") + } + + invalidConfig := NewPluginConfig() + invalidConfig.Components = append(invalidConfig.Components, PluginComponentSpec{ + Kind: "fixture", + Enabled: true, + Config: map[string]any{"invalid": make(chan int)}, + }) + if _, _, err := ActivateDynamicPlugins(invalidConfig, nil); err == nil { + t.Fatal("invalid config serialization error = nil") + } + if activationCalls != 0 { + t.Fatalf("activation calls after config serialization failure = %d", activationCalls) + } + + invalidSpecs := []DynamicPluginActivationSpec{{ + PluginID: "fixture", + Kind: DynamicPluginKindRustDynamic, + ManifestRef: "/tmp/relay-plugin.toml", + Config: map[string]any{"invalid": make(chan int)}, + }} + if _, _, err := ActivateDynamicPlugins(NewPluginConfig(), invalidSpecs); err == nil { + t.Fatal("invalid specs serialization error = nil") + } + if activationCalls != 0 { + t.Fatalf("activation calls after specs serialization failure = %d", activationCalls) + } + + if _, _, err := ActivateDynamicPlugins(NewPluginConfig(), nil); err == nil || err.Error() != "load failed" { + t.Fatalf("activation error = %v, want load failed", err) + } + if activationCalls != 1 { + t.Fatalf("activation calls = %d, want 1", activationCalls) + } +} + +func TestNilPluginActivationCloseIsSafe(t *testing.T) { + var activation *PluginActivation + if err := activation.Close(); err != nil { + t.Fatalf("nil Close() error = %v", err) + } +} + +func TestActivateDynamicPluginsLoadsNativePluginThroughCgo(t *testing.T) { + if err := ClearPluginConfiguration(); err != nil { + t.Fatalf("ClearPluginConfiguration() error = %v", err) + } + + library := goNativePluginFixture(t) + manifest := writeGoNativePluginManifest(t, library) + activation, report, err := ActivateDynamicPlugins(NewPluginConfig(), []DynamicPluginActivationSpec{{ + PluginID: "fixture_native", + Kind: DynamicPluginKindRustDynamic, + ManifestRef: manifest, + Config: map[string]any{}, + }}) + if err != nil { + t.Fatalf("ActivateDynamicPlugins() error = %v", err) + } + defer func() { + if err := activation.Close(); err != nil { + t.Errorf("deferred Close() error = %v", err) + } + }() + if len(report.Diagnostics) != 0 { + t.Fatalf("activation diagnostics = %#v, want none", report.Diagnostics) + } + + transformed, err := ToolRequestIntercepts("go-native-tool", json.RawMessage(`{"input":true}`)) + if err != nil { + t.Fatalf("ToolRequestIntercepts() error = %v", err) + } + var transformedObject map[string]any + if err := json.Unmarshal(transformed, &transformedObject); err != nil { + t.Fatalf("transformed tool args are invalid JSON: %v", err) + } + if transformedObject["native_plugin"] != true { + t.Fatalf("transformed tool args = %s, want native_plugin marker", transformed) + } + + if err := activation.Close(); err != nil { + t.Fatalf("Close() error = %v", err) + } + afterClose, err := ToolRequestIntercepts("go-native-tool", json.RawMessage(`{"input":true}`)) + if err != nil { + t.Fatalf("ToolRequestIntercepts() after Close error = %v", err) + } + if string(afterClose) != `{"input":true}` { + t.Fatalf("tool args after Close = %s, want unchanged args", afterClose) + } + kinds, err := ListPluginKinds() + if err != nil { + t.Fatalf("ListPluginKinds() error = %v", err) + } + for _, kind := range kinds { + if kind == "fixture_native" { + t.Fatal("fixture_native remains registered after Close") + } + } + + _, _, err = ActivateDynamicPlugins(NewPluginConfig(), []DynamicPluginActivationSpec{{ + PluginID: "fixture_missing", + Kind: DynamicPluginKindRustDynamic, + ManifestRef: filepath.Join(t.TempDir(), "missing-relay-plugin.toml"), + }}) + if err == nil || !strings.Contains(err.Error(), "native plugin load failed") { + t.Fatalf("missing-manifest error = %v, want native plugin load diagnostic", err) + } +} + +func TestActivateDynamicPluginsLoadsWorkerPluginThroughCgo(t *testing.T) { + if err := ClearPluginConfiguration(); err != nil { + t.Fatalf("ClearPluginConfiguration() error = %v", err) + } + + executable := goWorkerPluginFixture(t) + manifest := writeGoWorkerPluginManifest(t, executable) + activation, report, err := ActivateDynamicPlugins(NewPluginConfig(), []DynamicPluginActivationSpec{{ + PluginID: "fixture_worker", + Kind: DynamicPluginKindWorker, + ManifestRef: manifest, + Config: map[string]any{}, + }}) + if err != nil { + t.Fatalf("ActivateDynamicPlugins() error = %v", err) + } + defer func() { + if err := activation.Close(); err != nil { + t.Errorf("deferred Close() error = %v", err) + } + }() + if len(report.Diagnostics) != 0 { + t.Fatalf("activation diagnostics = %#v, want none", report.Diagnostics) + } + + transformed, err := ToolRequestIntercepts("go-worker-tool", json.RawMessage(`{"input":true}`)) + if err != nil { + t.Fatalf("ToolRequestIntercepts() error = %v", err) + } + var transformedObject map[string]any + if err := json.Unmarshal(transformed, &transformedObject); err != nil { + t.Fatalf("transformed tool args are invalid JSON: %v", err) + } + if transformedObject["worker_plugin"] != true { + t.Fatalf("transformed tool args = %s, want worker_plugin marker", transformed) + } + + if err := activation.Close(); err != nil { + t.Fatalf("Close() error = %v", err) + } + afterClose, err := ToolRequestIntercepts("go-worker-tool", json.RawMessage(`{"input":true}`)) + if err != nil { + t.Fatalf("ToolRequestIntercepts() after Close error = %v", err) + } + if string(afterClose) != `{"input":true}` { + t.Fatalf("tool args after Close = %s, want unchanged args", afterClose) + } +} + +func TestPluginActivationFinalizerReleasesHostOwnership(t *testing.T) { + if err := ClearPluginConfiguration(); err != nil { + t.Fatalf("ClearPluginConfiguration() error = %v", err) + } + createUnclosedPluginActivation(t) + + deadline := time.Now().Add(10 * time.Second) + for time.Now().Before(deadline) { + runtime.GC() + runtime.Gosched() + activation, _, err := ActivateDynamicPlugins(NewPluginConfig(), nil) + if err == nil { + if closeErr := activation.Close(); closeErr != nil { + t.Fatalf("replacement activation Close() error = %v", closeErr) + } + return + } + time.Sleep(10 * time.Millisecond) + } + t.Fatal("plugin activation finalizer did not release host ownership") +} + +// Keep creation in a separate frame so the activation is unreachable when the +// caller starts forcing collection. +// +//go:noinline +func createUnclosedPluginActivation(t *testing.T) { + t.Helper() + activation, _, err := ActivateDynamicPlugins(NewPluginConfig(), nil) + if err != nil { + t.Fatalf("ActivateDynamicPlugins() error = %v", err) + } + runtime.KeepAlive(activation) +} + +func goNativePluginFixture(t *testing.T) string { + t.Helper() + goNativePluginFixtureOnce.Do(func() { + repoRoot, err := filepath.Abs(filepath.Join("..", "..")) + if err != nil { + goNativePluginFixtureErr = err + return + } + sourceRoot, err := os.MkdirTemp("", "nemo-relay-go-native-plugin-") + if err != nil { + goNativePluginFixtureErr = err + return + } + defer os.RemoveAll(sourceRoot) + fixtureRoot := filepath.Join(sourceRoot, "native_plugin") + if err := os.MkdirAll(filepath.Join(fixtureRoot, "src"), 0o700); err != nil { + goNativePluginFixtureErr = err + return + } + fixtureSource := filepath.Join(repoRoot, "crates", "core", "tests", "fixtures", "native_plugin") + manifestBytes, err := os.ReadFile(filepath.Join(fixtureSource, "Cargo.toml")) + if err != nil { + goNativePluginFixtureErr = err + return + } + pluginPath := filepath.Join(repoRoot, "crates", "plugin") + manifestContents := strings.Replace( + string(manifestBytes), + `nemo-relay-plugin = { path = "../../../../plugin" }`, + fmt.Sprintf("nemo-relay-plugin = { path = %q }", pluginPath), + 1, + ) + manifest := filepath.Join(fixtureRoot, "Cargo.toml") + if err := os.WriteFile(manifest, []byte(manifestContents), 0o600); err != nil { + goNativePluginFixtureErr = err + return + } + librarySource, err := os.ReadFile(filepath.Join(fixtureSource, "src", "lib.rs")) + if err != nil { + goNativePluginFixtureErr = err + return + } + if err := os.WriteFile(filepath.Join(fixtureRoot, "src", "lib.rs"), librarySource, 0o600); err != nil { + goNativePluginFixtureErr = err + return + } + target := filepath.Join(repoRoot, "target") + cargo := os.Getenv("CARGO") + if cargo == "" { + cargo = "cargo" + } + command := exec.Command(cargo, "build", "--quiet", "--manifest-path", manifest, "--target-dir", target) + if output, err := command.CombinedOutput(); err != nil { + goNativePluginFixtureErr = fmt.Errorf("build native plugin fixture: %w\n%s", err, output) + return + } + goNativePluginFixturePath = filepath.Join(target, "debug", goNativeLibraryName()) + if _, err := os.Stat(goNativePluginFixturePath); err != nil { + goNativePluginFixtureErr = fmt.Errorf("native plugin fixture output: %w", err) + } + }) + if goNativePluginFixtureErr != nil { + t.Fatal(goNativePluginFixtureErr) + } + return goNativePluginFixturePath +} + +func goWorkerPluginFixture(t *testing.T) string { + t.Helper() + goWorkerPluginFixtureOnce.Do(func() { + repoRoot, err := filepath.Abs(filepath.Join("..", "..")) + if err != nil { + goWorkerPluginFixtureErr = err + return + } + manifest := filepath.Join(repoRoot, "crates", "core", "tests", "fixtures", "worker_plugin", "Cargo.toml") + target := filepath.Join(repoRoot, "target") + cargo := os.Getenv("CARGO") + if cargo == "" { + cargo = "cargo" + } + command := exec.Command(cargo, "build", "--quiet", "--locked", "--manifest-path", manifest, "--target-dir", target) + if output, err := command.CombinedOutput(); err != nil { + goWorkerPluginFixtureErr = fmt.Errorf("build worker plugin fixture: %w\n%s", err, output) + return + } + executable := "nemo-relay-worker-plugin-fixture" + if runtime.GOOS == "windows" { + executable += ".exe" + } + goWorkerPluginFixturePath = filepath.Join(target, "debug", executable) + if _, err := os.Stat(goWorkerPluginFixturePath); err != nil { + goWorkerPluginFixtureErr = fmt.Errorf("worker plugin fixture output: %w", err) + } + }) + if goWorkerPluginFixtureErr != nil { + t.Fatal(goWorkerPluginFixtureErr) + } + return goWorkerPluginFixturePath +} + +func writeGoNativePluginManifest(t *testing.T, library string) string { + t.Helper() + manifest := filepath.Join(t.TempDir(), "relay-plugin.toml") + contents := fmt.Sprintf(`manifest_version = 1 + +[plugin] +id = "fixture_native" +kind = "rust_dynamic" + +[compat] +relay = "=0.6.0" +native_api = "1" + +[defaults] +enabled = false + +[capabilities] +items = ["plugin_native"] + +[load] +library = %q +symbol = "nemo_relay_fixture_native_plugin" +`, library) + if err := os.WriteFile(manifest, []byte(contents), 0o600); err != nil { + t.Fatalf("write native plugin manifest: %v", err) + } + return manifest +} + +func writeGoWorkerPluginManifest(t *testing.T, executable string) string { + t.Helper() + manifest := filepath.Join(t.TempDir(), "relay-plugin.toml") + contents := fmt.Sprintf(`manifest_version = 1 + +[plugin] +id = "fixture_worker" +kind = "worker" + +[compat] +relay = "=0.6.0" +worker_protocol = "grpc-v1" + +[defaults] +enabled = false + +[capabilities] +items = ["plugin_worker"] + +[load] +runtime = "rust" +entrypoint = %q +`, executable) + if err := os.WriteFile(manifest, []byte(contents), 0o600); err != nil { + t.Fatalf("write worker plugin manifest: %v", err) + } + return manifest +} + +func goNativeLibraryName() string { + switch runtime.GOOS { + case "windows": + return "nemo_relay_plugin_fixture.dll" + case "darwin": + return "libnemo_relay_plugin_fixture.dylib" + default: + return "libnemo_relay_plugin_fixture.so" + } +} From 49683c78ff4219a186e654149f431540f567b8a1 Mon Sep 17 00:00:00 2001 From: Bryan Bednarski Date: Mon, 6 Jul 2026 18:07:14 -0600 Subject: [PATCH 02/11] fix(go): share dynamic activation ownership Signed-off-by: Bryan Bednarski --- go/nemo_relay/plugin.go | 43 +++--- go/nemo_relay/plugin_activation_test.go | 172 +++++++++++++++++++++++- 2 files changed, 199 insertions(+), 16 deletions(-) diff --git a/go/nemo_relay/plugin.go b/go/nemo_relay/plugin.go index 07a19daa3..77c39cdbe 100644 --- a/go/nemo_relay/plugin.go +++ b/go/nemo_relay/plugin.go @@ -220,13 +220,13 @@ type ConfigReport struct { // PluginComponentSpec is one top-level plugin component. type PluginComponentSpec struct { Kind string `json:"kind"` - Enabled bool `json:"enabled,omitempty"` + Enabled bool `json:"enabled"` Config map[string]any `json:"config,omitempty"` } // PluginConfig is the canonical plugin configuration document. type PluginConfig struct { - Version uint32 `json:"version,omitempty"` + Version uint32 `json:"version"` Components []PluginComponentSpec `json:"components,omitempty"` Policy *ConfigPolicy `json:"policy,omitempty"` } @@ -253,9 +253,13 @@ type DynamicPluginActivationSpec struct { } // PluginActivation owns the runtime registrations, native libraries, and -// workers created by ActivateDynamicPlugins. A PluginActivation must not be -// copied after first use. +// workers created by ActivateDynamicPlugins. Copies share one activation +// lifetime and may be closed safely from any copy. type PluginActivation struct { + state *pluginActivationState +} + +type pluginActivationState struct { mu sync.Mutex ptr unsafe.Pointer } @@ -374,33 +378,42 @@ func ActivateDynamicPlugins( if err != nil { return nil, ConfigReport{}, err } - activation := &PluginActivation{ptr: ptr} + activation := newPluginActivation(ptr) var report ConfigReport if err := jsonUnmarshal([]byte(rawReport), &report); err != nil { _ = activation.Close() return nil, ConfigReport{}, err } - runtime.SetFinalizer(activation, func(activation *PluginActivation) { - _ = activation.Close() - }) return activation, report, nil } +func newPluginActivation(ptr unsafe.Pointer) *PluginActivation { + state := &pluginActivationState{ptr: ptr} + runtime.SetFinalizer(state, func(state *pluginActivationState) { + _ = state.close() + }) + return &PluginActivation{state: state} +} + // Close removes callbacks and subscribers before unloading plugin libraries // and workers. It is safe to call Close repeatedly or on a nil activation. func (activation *PluginActivation) Close() error { - if activation == nil { + if activation == nil || activation.state == nil { return nil } - activation.mu.Lock() - defer activation.mu.Unlock() - if activation.ptr == nil { + return activation.state.close() +} + +func (state *pluginActivationState) close() error { + state.mu.Lock() + defer state.mu.Unlock() + if state.ptr == nil { return nil } - runtime.SetFinalizer(activation, nil) - ptr := activation.ptr - activation.ptr = nil + runtime.SetFinalizer(state, nil) + ptr := state.ptr + state.ptr = nil err := clearPluginActivation(ptr) freePluginActivation(ptr) return err diff --git a/go/nemo_relay/plugin_activation_test.go b/go/nemo_relay/plugin_activation_test.go index dd1388ac2..017046e71 100644 --- a/go/nemo_relay/plugin_activation_test.go +++ b/go/nemo_relay/plugin_activation_test.go @@ -108,6 +108,38 @@ func TestActivateDynamicPluginsSerializesSpecsAndOwnsCleanup(t *testing.T) { runtime.KeepAlive(token) } +func TestActivateDynamicPluginsPreservesExplicitZeroAndFalse(t *testing.T) { + withPluginActivationStubs(t) + + token := new(byte) + activateDynamicPluginsJSON = func(configJSON, specsJSON string) (unsafe.Pointer, string, error) { + const wantConfig = `{"version":0,"components":[{"kind":"fixture.disabled","enabled":false}]}` + if configJSON != wantConfig { + t.Fatalf("config JSON = %s, want %s", configJSON, wantConfig) + } + if specsJSON != "[]" { + t.Fatalf("dynamic plugin specs JSON = %s, want []", specsJSON) + } + return unsafe.Pointer(token), `{"diagnostics":[]}`, nil + } + clearPluginActivation = func(unsafe.Pointer) error { return nil } + freePluginActivation = func(unsafe.Pointer) {} + + activation, _, err := ActivateDynamicPlugins(PluginConfig{ + Version: 0, + Components: []PluginComponentSpec{ + {Kind: "fixture.disabled", Enabled: false}, + }, + }, nil) + if err != nil { + t.Fatalf("ActivateDynamicPlugins() error = %v", err) + } + if err := activation.Close(); err != nil { + t.Fatalf("Close() error = %v", err) + } + runtime.KeepAlive(token) +} + func TestActivateDynamicPluginsNormalizesNilSpecs(t *testing.T) { withPluginActivationStubs(t) @@ -171,7 +203,7 @@ func TestPluginActivationCloseFreesAfterClearFailure(t *testing.T) { return wantErr } freePluginActivation = func(unsafe.Pointer) { calls = append(calls, "free") } - activation := &PluginActivation{ptr: ptr} + activation := newPluginActivation(ptr) if err := activation.Close(); !errors.Is(err, wantErr) { t.Fatalf("Close() error = %v, want %v", err, wantErr) @@ -185,6 +217,144 @@ func TestPluginActivationCloseFreesAfterClearFailure(t *testing.T) { runtime.KeepAlive(token) } +func TestPluginActivationCopiesShareCloseState(t *testing.T) { + withPluginActivationStubs(t) + + token := new(byte) + ptr := unsafe.Pointer(token) + var callsMu sync.Mutex + var calls []string + clearPluginActivation = func(got unsafe.Pointer) error { + if got != ptr { + return fmt.Errorf("clear pointer = %p, want %p", got, ptr) + } + callsMu.Lock() + calls = append(calls, "clear") + callsMu.Unlock() + return nil + } + freePluginActivation = func(got unsafe.Pointer) { + callsMu.Lock() + defer callsMu.Unlock() + if got != ptr { + calls = append(calls, "free-wrong-pointer") + return + } + calls = append(calls, "free") + } + + activation := newPluginActivation(ptr) + copyValue := *activation + errors := make(chan error, 2) + var closeCalls sync.WaitGroup + for _, handle := range []*PluginActivation{activation, ©Value} { + closeCalls.Add(1) + go func(handle *PluginActivation) { + defer closeCalls.Done() + errors <- handle.Close() + }(handle) + } + closeCalls.Wait() + close(errors) + for err := range errors { + if err != nil { + t.Fatalf("Close() error = %v", err) + } + } + + callsMu.Lock() + gotCalls := strings.Join(calls, ",") + callsMu.Unlock() + if gotCalls != "clear,free" { + t.Fatalf("cleanup calls = %s, want clear,free", gotCalls) + } + runtime.KeepAlive(token) +} + +func TestPluginActivationCopyPreventsEarlyFinalization(t *testing.T) { + withPluginActivationStubs(t) + + token := new(byte) + ptr := unsafe.Pointer(token) + var callsMu sync.Mutex + var calls []string + clearPluginActivation = func(unsafe.Pointer) error { + callsMu.Lock() + calls = append(calls, "clear") + callsMu.Unlock() + return nil + } + freePluginActivation = func(unsafe.Pointer) { + callsMu.Lock() + calls = append(calls, "free") + callsMu.Unlock() + } + + wrapperCollected := make(chan struct{}) + copyValue := copiedPluginActivationWithGCSentinel(ptr, wrapperCollected) + deadline := time.Now().Add(5 * time.Second) + for { + runtime.GC() + runtime.Gosched() + select { + case <-wrapperCollected: + goto wrapperWasCollected + default: + if time.Now().After(deadline) { + t.Fatal("unreachable activation wrapper was not collected") + } + time.Sleep(10 * time.Millisecond) + } + } + +wrapperWasCollected: + for i := 0; i < 3; i++ { + runtime.GC() + runtime.Gosched() + time.Sleep(10 * time.Millisecond) + } + callsMu.Lock() + gotCalls := strings.Join(calls, ",") + callsMu.Unlock() + if gotCalls != "" { + t.Fatalf("cleanup ran while a copied activation was reachable: %s", gotCalls) + } + + if err := copyValue.Close(); err != nil { + t.Fatalf("copied activation Close() error = %v", err) + } + callsMu.Lock() + gotCalls = strings.Join(calls, ",") + callsMu.Unlock() + if gotCalls != "clear,free" { + t.Fatalf("cleanup calls = %s, want clear,free", gotCalls) + } + runtime.KeepAlive(copyValue) + runtime.KeepAlive(token) +} + +type pluginActivationGCSentinel struct { + activation *PluginActivation + padding [64]byte +} + +//go:noinline +func copiedPluginActivationWithGCSentinel( + ptr unsafe.Pointer, + wrapperCollected chan<- struct{}, +) PluginActivation { + activation := newPluginActivation(ptr) + copyValue := *activation + sentinel := &pluginActivationGCSentinel{activation: activation} + runtime.SetFinalizer(sentinel, func(sentinel *pluginActivationGCSentinel) { + runtime.KeepAlive(sentinel.activation) + close(wrapperCollected) + }) + runtime.KeepAlive(activation) + runtime.KeepAlive(sentinel) + return copyValue +} + func TestActivateDynamicPluginsSurfacesSerializationAndActivationErrors(t *testing.T) { withPluginActivationStubs(t) From 21ccdcb6ea27a5d2a1a115f307575effcc403db4 Mon Sep 17 00:00:00 2001 From: Bryan Bednarski Date: Mon, 6 Jul 2026 18:25:47 -0600 Subject: [PATCH 03/11] fix(go): preserve plugin config presence Signed-off-by: Bryan Bednarski --- go/nemo_relay/adaptive.go | 43 +++++- go/nemo_relay/model_pricing.go | 43 +++++- go/nemo_relay/observability_plugin.go | 43 +++++- go/nemo_relay/pii_redaction.go | 43 +++++- go/nemo_relay/plugin.go | 179 +++++++++++++++++++++-- go/nemo_relay/plugin_activation_test.go | 181 ++++++++++++++++++++++-- 6 files changed, 484 insertions(+), 48 deletions(-) diff --git a/go/nemo_relay/adaptive.go b/go/nemo_relay/adaptive.go index eb7ce81f5..249fb491d 100644 --- a/go/nemo_relay/adaptive.go +++ b/go/nemo_relay/adaptive.go @@ -68,8 +68,9 @@ type AcgConfig struct { // AdaptiveComponentSpec wraps one adaptive config as a top-level plugin component. type AdaptiveComponentSpec struct { - Enabled bool `json:"enabled,omitempty"` - Config AdaptiveConfig `json:"config"` + Enabled bool `json:"enabled,omitempty"` + Config AdaptiveConfig `json:"config"` + enabledSet bool } // NewAdaptiveConfig returns a default adaptive config with version 1. @@ -141,18 +142,50 @@ func NewAcgConfig() AcgConfig { // NewAdaptiveComponentSpec wraps adaptive config as an enabled top-level component. func NewAdaptiveComponentSpec(config AdaptiveConfig) AdaptiveComponentSpec { return AdaptiveComponentSpec{ - Enabled: true, - Config: config, + Enabled: true, + Config: config, + enabledSet: true, } } +// SetEnabled explicitly sets whether this adaptive component is enabled. +func (spec *AdaptiveComponentSpec) SetEnabled(enabled bool) { + spec.Enabled = enabled + spec.enabledSet = true +} + +// WithEnabled returns a copy with an explicitly present enabled value. +func (spec AdaptiveComponentSpec) WithEnabled(enabled bool) AdaptiveComponentSpec { + spec.SetEnabled(enabled) + return spec +} + +// MarshalJSON preserves omitted-default and explicit-false enabled semantics. +func (spec AdaptiveComponentSpec) MarshalJSON() ([]byte, error) { + return marshalComponentWrapper(spec.Enabled, spec.enabledSet, spec.Config) +} + +// UnmarshalJSON records whether enabled was explicitly present. +func (spec *AdaptiveComponentSpec) UnmarshalJSON(payload []byte) error { + enabled, enabledSet, config, err := unmarshalComponentWrapper[AdaptiveConfig](payload) + if err != nil { + return err + } + *spec = AdaptiveComponentSpec{Enabled: enabled, Config: config, enabledSet: enabledSet} + return nil +} + // PluginComponent converts the adaptive component wrapper into the shared plugin shape. func (spec AdaptiveComponentSpec) PluginComponent() PluginComponentSpec { - return PluginComponentSpec{ + component := PluginComponentSpec{ Kind: AdaptivePluginKind, Enabled: spec.Enabled, Config: mustConfigMap(spec.Config), } + if spec.enabledSet || spec.Enabled { + component.SetEnabled(spec.Enabled) + } + return component } // AdaptiveComponent converts adaptive config directly into a shared plugin component. diff --git a/go/nemo_relay/model_pricing.go b/go/nemo_relay/model_pricing.go index de18d116d..d934119df 100644 --- a/go/nemo_relay/model_pricing.go +++ b/go/nemo_relay/model_pricing.go @@ -134,8 +134,9 @@ func (schedule PromptTokenThresholdRateSchedule) MarshalJSON() ([]byte, error) { // PricingComponentSpec wraps one model pricing config as a top-level plugin component. type PricingComponentSpec struct { - Enabled bool `json:"enabled,omitempty"` - Config PricingConfig `json:"config"` + Enabled bool `json:"enabled,omitempty"` + Config PricingConfig `json:"config"` + enabledSet bool } // NewPricingConfig returns an empty model pricing config. @@ -209,18 +210,50 @@ func NewPromptTokenThresholdRateSchedule(tiers ...TokenRateTier) PromptTokenThre // NewPricingComponentSpec wraps model pricing config as an enabled top-level component. func NewPricingComponentSpec(config PricingConfig) PricingComponentSpec { return PricingComponentSpec{ - Enabled: true, - Config: config, + Enabled: true, + Config: config, + enabledSet: true, } } +// SetEnabled explicitly sets whether this model pricing component is enabled. +func (spec *PricingComponentSpec) SetEnabled(enabled bool) { + spec.Enabled = enabled + spec.enabledSet = true +} + +// WithEnabled returns a copy with an explicitly present enabled value. +func (spec PricingComponentSpec) WithEnabled(enabled bool) PricingComponentSpec { + spec.SetEnabled(enabled) + return spec +} + +// MarshalJSON preserves omitted-default and explicit-false enabled semantics. +func (spec PricingComponentSpec) MarshalJSON() ([]byte, error) { + return marshalComponentWrapper(spec.Enabled, spec.enabledSet, spec.Config) +} + +// UnmarshalJSON records whether enabled was explicitly present. +func (spec *PricingComponentSpec) UnmarshalJSON(payload []byte) error { + enabled, enabledSet, config, err := unmarshalComponentWrapper[PricingConfig](payload) + if err != nil { + return err + } + *spec = PricingComponentSpec{Enabled: enabled, Config: config, enabledSet: enabledSet} + return nil +} + // PluginComponent converts the model pricing component wrapper into the shared plugin shape. func (spec PricingComponentSpec) PluginComponent() PluginComponentSpec { - return PluginComponentSpec{ + component := PluginComponentSpec{ Kind: PricingPluginKind, Enabled: spec.Enabled, Config: mustConfigMap(spec.Config), } + if spec.enabledSet || spec.Enabled { + component.SetEnabled(spec.Enabled) + } + return component } // PricingComponent converts model pricing config directly into a shared plugin component. diff --git a/go/nemo_relay/observability_plugin.go b/go/nemo_relay/observability_plugin.go index c79488919..55727f0cb 100644 --- a/go/nemo_relay/observability_plugin.go +++ b/go/nemo_relay/observability_plugin.go @@ -180,8 +180,9 @@ func (config ObservabilityOtlpConfig) MarshalJSON() ([]byte, error) { // ObservabilityComponentSpec wraps one observability config as a top-level plugin component. type ObservabilityComponentSpec struct { - Enabled bool `json:"enabled,omitempty"` - Config ObservabilityConfig `json:"config"` + Enabled bool `json:"enabled,omitempty"` + Config ObservabilityConfig `json:"config"` + enabledSet bool } // NewObservabilityConfig returns a default observability config with version 1. @@ -231,18 +232,50 @@ func NewObservabilityOtlpConfig() ObservabilityOtlpConfig { // NewObservabilityComponentSpec wraps observability config as an enabled top-level component. func NewObservabilityComponentSpec(config ObservabilityConfig) ObservabilityComponentSpec { return ObservabilityComponentSpec{ - Enabled: true, - Config: config, + Enabled: true, + Config: config, + enabledSet: true, } } +// SetEnabled explicitly sets whether this observability component is enabled. +func (spec *ObservabilityComponentSpec) SetEnabled(enabled bool) { + spec.Enabled = enabled + spec.enabledSet = true +} + +// WithEnabled returns a copy with an explicitly present enabled value. +func (spec ObservabilityComponentSpec) WithEnabled(enabled bool) ObservabilityComponentSpec { + spec.SetEnabled(enabled) + return spec +} + +// MarshalJSON preserves omitted-default and explicit-false enabled semantics. +func (spec ObservabilityComponentSpec) MarshalJSON() ([]byte, error) { + return marshalComponentWrapper(spec.Enabled, spec.enabledSet, spec.Config) +} + +// UnmarshalJSON records whether enabled was explicitly present. +func (spec *ObservabilityComponentSpec) UnmarshalJSON(payload []byte) error { + enabled, enabledSet, config, err := unmarshalComponentWrapper[ObservabilityConfig](payload) + if err != nil { + return err + } + *spec = ObservabilityComponentSpec{Enabled: enabled, Config: config, enabledSet: enabledSet} + return nil +} + // PluginComponent converts the observability component wrapper into the shared plugin shape. func (spec ObservabilityComponentSpec) PluginComponent() PluginComponentSpec { - return PluginComponentSpec{ + component := PluginComponentSpec{ Kind: ObservabilityPluginKind, Enabled: spec.Enabled, Config: mustConfigMap(spec.Config), } + if spec.enabledSet || spec.Enabled { + component.SetEnabled(spec.Enabled) + } + return component } // ObservabilityComponent converts observability config directly into a shared plugin component. diff --git a/go/nemo_relay/pii_redaction.go b/go/nemo_relay/pii_redaction.go index 871a83b60..cb807c8c4 100644 --- a/go/nemo_relay/pii_redaction.go +++ b/go/nemo_relay/pii_redaction.go @@ -45,8 +45,9 @@ type PiiRedactionConfig struct { // PiiRedactionComponentSpec wraps one PII redaction config as a top-level plugin component. type PiiRedactionComponentSpec struct { - Enabled bool `json:"enabled,omitempty"` - Config PiiRedactionConfig `json:"config"` + Enabled bool `json:"enabled,omitempty"` + Config PiiRedactionConfig `json:"config"` + enabledSet bool } // NewPiiRedactionConfig returns a default PII redaction config with version 1. @@ -81,18 +82,50 @@ func NewPiiRedactionLocalModelConfig() PiiRedactionLocalModelConfig { // NewPiiRedactionComponentSpec wraps PII redaction config as an enabled component. func NewPiiRedactionComponentSpec(config PiiRedactionConfig) PiiRedactionComponentSpec { return PiiRedactionComponentSpec{ - Enabled: true, - Config: config, + Enabled: true, + Config: config, + enabledSet: true, } } +// SetEnabled explicitly sets whether this PII redaction component is enabled. +func (spec *PiiRedactionComponentSpec) SetEnabled(enabled bool) { + spec.Enabled = enabled + spec.enabledSet = true +} + +// WithEnabled returns a copy with an explicitly present enabled value. +func (spec PiiRedactionComponentSpec) WithEnabled(enabled bool) PiiRedactionComponentSpec { + spec.SetEnabled(enabled) + return spec +} + +// MarshalJSON preserves omitted-default and explicit-false enabled semantics. +func (spec PiiRedactionComponentSpec) MarshalJSON() ([]byte, error) { + return marshalComponentWrapper(spec.Enabled, spec.enabledSet, spec.Config) +} + +// UnmarshalJSON records whether enabled was explicitly present. +func (spec *PiiRedactionComponentSpec) UnmarshalJSON(payload []byte) error { + enabled, enabledSet, config, err := unmarshalComponentWrapper[PiiRedactionConfig](payload) + if err != nil { + return err + } + *spec = PiiRedactionComponentSpec{Enabled: enabled, Config: config, enabledSet: enabledSet} + return nil +} + // PluginComponent converts the PII redaction wrapper into the shared plugin shape. func (spec PiiRedactionComponentSpec) PluginComponent() PluginComponentSpec { - return PluginComponentSpec{ + component := PluginComponentSpec{ Kind: PiiRedactionPluginKind, Enabled: spec.Enabled, Config: mustConfigMap(spec.Config), } + if spec.enabledSet || spec.Enabled { + component.SetEnabled(spec.Enabled) + } + return component } // PiiRedactionComponent converts PII redaction config directly into a shared plugin component. diff --git a/go/nemo_relay/plugin.go b/go/nemo_relay/plugin.go index 77c39cdbe..c2864525c 100644 --- a/go/nemo_relay/plugin.go +++ b/go/nemo_relay/plugin.go @@ -71,6 +71,7 @@ extern char* goToolExecInterceptTrampoline(void*, const char*, NemoRelayToolExec import "C" import ( + "encoding/json" "errors" "runtime" "sync" @@ -219,16 +220,22 @@ type ConfigReport struct { // PluginComponentSpec is one top-level plugin component. type PluginComponentSpec struct { - Kind string `json:"kind"` - Enabled bool `json:"enabled"` - Config map[string]any `json:"config,omitempty"` + Kind string `json:"kind"` + // Enabled uses Relay's default true when omitted. Use SetEnabled or + // WithEnabled to serialize an explicit false value. + Enabled bool `json:"-"` + Config map[string]any `json:"config,omitempty"` + enabledSet bool } // PluginConfig is the canonical plugin configuration document. type PluginConfig struct { - Version uint32 `json:"version"` + // Version uses Relay's default when omitted. Use SetVersion or WithVersion + // to serialize an explicit zero value. + Version uint32 `json:"-"` Components []PluginComponentSpec `json:"components,omitempty"` Policy *ConfigPolicy `json:"policy,omitempty"` + versionSet bool } // DynamicPluginKind identifies the runtime lane used by a dynamic plugin. @@ -260,8 +267,10 @@ type PluginActivation struct { } type pluginActivationState struct { - mu sync.Mutex - ptr unsafe.Pointer + mu sync.Mutex + ptr unsafe.Pointer + closed bool + closeErr error } // PluginContext is the component-scoped registration context passed to plugins. @@ -305,16 +314,153 @@ func NewPluginConfig() PluginConfig { return PluginConfig{ Version: 1, Components: []PluginComponentSpec{}, + versionSet: true, } } +// SetVersion sets the plugin config schema version and records it as explicitly +// present, including when version is zero. +func (config *PluginConfig) SetVersion(version uint32) { + config.Version = version + config.versionSet = true +} + +// WithVersion returns a copy with an explicitly present schema version. +func (config PluginConfig) WithVersion(version uint32) PluginConfig { + config.SetVersion(version) + return config +} + // NewPluginComponent returns an enabled top-level component with empty config. func NewPluginComponent(kind string) PluginComponentSpec { return PluginComponentSpec{ - Kind: kind, - Enabled: true, - Config: map[string]any{}, + Kind: kind, + Enabled: true, + Config: map[string]any{}, + enabledSet: true, + } +} + +// SetEnabled sets whether this component is enabled and records the value as +// explicitly present, including when enabled is false. +func (component *PluginComponentSpec) SetEnabled(enabled bool) { + component.Enabled = enabled + component.enabledSet = true +} + +// WithEnabled returns a copy with an explicitly present enabled value. +func (component PluginComponentSpec) WithEnabled(enabled bool) PluginComponentSpec { + component.SetEnabled(enabled) + return component +} + +// MarshalJSON preserves Relay's default-on-omission semantics while allowing +// callers to explicitly serialize false through SetEnabled or WithEnabled. +func (component PluginComponentSpec) MarshalJSON() ([]byte, error) { + type componentJSON struct { + Kind string `json:"kind"` + Enabled *bool `json:"enabled,omitempty"` + Config map[string]any `json:"config,omitempty"` + } + var enabled *bool + if component.enabledSet || component.Enabled { + enabled = &component.Enabled + } + return json.Marshal(componentJSON{ + Kind: component.Kind, + Enabled: enabled, + Config: component.Config, + }) +} + +// UnmarshalJSON records whether enabled was explicitly present so false +// survives subsequent marshaling instead of becoming Relay's default true. +func (component *PluginComponentSpec) UnmarshalJSON(payload []byte) error { + decoded := struct { + Kind string `json:"kind"` + Enabled *bool `json:"enabled"` + Config map[string]any `json:"config"` + }{} + if err := json.Unmarshal(payload, &decoded); err != nil { + return err + } + *component = PluginComponentSpec{ + Kind: decoded.Kind, + Config: decoded.Config, + } + if decoded.Enabled != nil { + component.SetEnabled(*decoded.Enabled) + } + return nil +} + +// MarshalJSON preserves Relay's default-on-omission semantics while allowing +// callers to explicitly serialize zero through SetVersion or WithVersion. +func (config PluginConfig) MarshalJSON() ([]byte, error) { + type configJSON struct { + Version *uint32 `json:"version,omitempty"` + Components []PluginComponentSpec `json:"components,omitempty"` + Policy *ConfigPolicy `json:"policy,omitempty"` } + var version *uint32 + if config.versionSet || config.Version != 0 { + version = &config.Version + } + return json.Marshal(configJSON{ + Version: version, + Components: config.Components, + Policy: config.Policy, + }) +} + +// UnmarshalJSON records whether version was explicitly present so zero +// survives subsequent marshaling instead of becoming Relay's default version. +func (config *PluginConfig) UnmarshalJSON(payload []byte) error { + decoded := struct { + Version *uint32 `json:"version"` + Components []PluginComponentSpec `json:"components"` + Policy *ConfigPolicy `json:"policy"` + }{} + if err := json.Unmarshal(payload, &decoded); err != nil { + return err + } + *config = PluginConfig{ + Components: decoded.Components, + Policy: decoded.Policy, + } + if decoded.Version != nil { + config.SetVersion(*decoded.Version) + } + return nil +} + +func marshalComponentWrapper[T any](enabled bool, enabledSet bool, config T) ([]byte, error) { + var explicitEnabled *bool + if enabledSet || enabled { + explicitEnabled = &enabled + } + return json.Marshal(struct { + Enabled *bool `json:"enabled,omitempty"` + Config T `json:"config"` + }{ + Enabled: explicitEnabled, + Config: config, + }) +} + +func unmarshalComponentWrapper[T any](payload []byte) (bool, bool, T, error) { + var decoded struct { + Enabled *bool `json:"enabled"` + Config T `json:"config"` + } + if err := json.Unmarshal(payload, &decoded); err != nil { + var zero T + return false, false, zero, err + } + if decoded.Enabled == nil { + return false, false, decoded.Config, nil + } + return *decoded.Enabled, true, decoded.Config, nil } // ValidatePluginConfig validates a plugin config without changing runtime state. @@ -397,6 +543,7 @@ func newPluginActivation(ptr unsafe.Pointer) *PluginActivation { // Close removes callbacks and subscribers before unloading plugin libraries // and workers. It is safe to call Close repeatedly or on a nil activation. +// Copies and repeated calls observe the same first teardown result. func (activation *PluginActivation) Close() error { if activation == nil || activation.state == nil { return nil @@ -407,16 +554,20 @@ func (activation *PluginActivation) Close() error { func (state *pluginActivationState) close() error { state.mu.Lock() defer state.mu.Unlock() - if state.ptr == nil { - return nil + if state.closed { + return state.closeErr } + state.closed = true - runtime.SetFinalizer(state, nil) ptr := state.ptr state.ptr = nil - err := clearPluginActivation(ptr) + if ptr == nil { + return nil + } + runtime.SetFinalizer(state, nil) + state.closeErr = clearPluginActivation(ptr) freePluginActivation(ptr) - return err + return state.closeErr } // ClearPluginConfiguration removes all active plugin component registrations. diff --git a/go/nemo_relay/plugin_activation_test.go b/go/nemo_relay/plugin_activation_test.go index 017046e71..d995c63f2 100644 --- a/go/nemo_relay/plugin_activation_test.go +++ b/go/nemo_relay/plugin_activation_test.go @@ -108,12 +108,20 @@ func TestActivateDynamicPluginsSerializesSpecsAndOwnsCleanup(t *testing.T) { runtime.KeepAlive(token) } -func TestActivateDynamicPluginsPreservesExplicitZeroAndFalse(t *testing.T) { +func TestActivateDynamicPluginsPreservesLegacyOmittedDefaults(t *testing.T) { withPluginActivationStubs(t) + emptyPayload, err := json.Marshal(PluginConfig{}) + if err != nil { + t.Fatalf("marshal empty plugin config: %v", err) + } + if string(emptyPayload) != "{}" { + t.Fatalf("empty plugin config JSON = %s, want {}", emptyPayload) + } + token := new(byte) activateDynamicPluginsJSON = func(configJSON, specsJSON string) (unsafe.Pointer, string, error) { - const wantConfig = `{"version":0,"components":[{"kind":"fixture.disabled","enabled":false}]}` + const wantConfig = `{"components":[{"kind":"fixture.default"}]}` if configJSON != wantConfig { t.Fatalf("config JSON = %s, want %s", configJSON, wantConfig) } @@ -126,9 +134,8 @@ func TestActivateDynamicPluginsPreservesExplicitZeroAndFalse(t *testing.T) { freePluginActivation = func(unsafe.Pointer) {} activation, _, err := ActivateDynamicPlugins(PluginConfig{ - Version: 0, Components: []PluginComponentSpec{ - {Kind: "fixture.disabled", Enabled: false}, + {Kind: "fixture.default"}, }, }, nil) if err != nil { @@ -140,6 +147,148 @@ func TestActivateDynamicPluginsPreservesExplicitZeroAndFalse(t *testing.T) { runtime.KeepAlive(token) } +func TestActivateDynamicPluginsPreservesExplicitZeroAndFalse(t *testing.T) { + withPluginActivationStubs(t) + + token := new(byte) + activateDynamicPluginsJSON = func(configJSON, specsJSON string) (unsafe.Pointer, string, error) { + const wantConfig = `{"version":0,"components":[{"kind":"fixture.disabled","enabled":false}]}` + if configJSON != wantConfig { + t.Fatalf("config JSON = %s, want %s", configJSON, wantConfig) + } + if specsJSON != "[]" { + t.Fatalf("dynamic plugin specs JSON = %s, want []", specsJSON) + } + var roundTrip PluginConfig + if err := json.Unmarshal([]byte(configJSON), &roundTrip); err != nil { + t.Fatalf("unmarshal explicit plugin config: %v", err) + } + roundTripPayload, err := json.Marshal(roundTrip) + if err != nil { + t.Fatalf("remarshal explicit plugin config: %v", err) + } + if string(roundTripPayload) != wantConfig { + t.Fatalf("round-trip config JSON = %s, want %s", roundTripPayload, wantConfig) + } + return unsafe.Pointer(token), `{"diagnostics":[]}`, nil + } + clearPluginActivation = func(unsafe.Pointer) error { return nil } + freePluginActivation = func(unsafe.Pointer) {} + + component := (PluginComponentSpec{Kind: "fixture.disabled"}).WithEnabled(false) + config := PluginConfig{Components: []PluginComponentSpec{component}} + config.SetVersion(0) + activation, _, err := ActivateDynamicPlugins(config, nil) + if err != nil { + t.Fatalf("ActivateDynamicPlugins() error = %v", err) + } + if err := activation.Close(); err != nil { + t.Fatalf("Close() error = %v", err) + } + runtime.KeepAlive(token) +} + +func TestPluginConfigUnmarshalOmissionsResetPresenceAndValues(t *testing.T) { + staleComponent := NewPluginComponent("fixture.stale") + staleComponent.Config = map[string]any{"stale": true} + config := NewPluginConfig() + config.Version = 7 + config.Components = []PluginComponentSpec{staleComponent} + config.Policy = &ConfigPolicy{UnknownField: UnsupportedBehaviorError} + + const payload = `{"components":[{"kind":"fixture.fresh"}]}` + if err := json.Unmarshal([]byte(payload), &config); err != nil { + t.Fatalf("unmarshal config with omitted defaults: %v", err) + } + if config.Version != 0 || config.versionSet { + t.Fatalf("omitted version retained stale state: %#v", config) + } + if config.Policy != nil { + t.Fatalf("omitted policy retained stale state: %#v", config.Policy) + } + if len(config.Components) != 1 { + t.Fatalf("components = %#v, want one", config.Components) + } + component := config.Components[0] + if component.Kind != "fixture.fresh" || component.Enabled || component.enabledSet || component.Config != nil { + t.Fatalf("omitted enabled/config retained stale state: %#v", component) + } + + roundTrip, err := json.Marshal(config) + if err != nil { + t.Fatalf("remarshal config with omitted defaults: %v", err) + } + if string(roundTrip) != payload { + t.Fatalf("round-trip config JSON = %s, want %s", roundTrip, payload) + } +} + +func TestComponentWrapperEnabledPresenceSurvivesConversion(t *testing.T) { + adaptive := NewAdaptiveComponentSpec(NewAdaptiveConfig()) + adaptive.Enabled = false + explicitDisabled := []PluginComponentSpec{ + adaptive.PluginComponent(), + NewObservabilityComponentSpec(NewObservabilityConfig()).WithEnabled(false).PluginComponent(), + NewPricingComponentSpec(NewPricingConfig()).WithEnabled(false).PluginComponent(), + NewPiiRedactionComponentSpec(NewPiiRedactionConfig()).WithEnabled(false).PluginComponent(), + } + legacyDefaults := []PluginComponentSpec{ + (AdaptiveComponentSpec{Config: NewAdaptiveConfig()}).PluginComponent(), + (ObservabilityComponentSpec{Config: NewObservabilityConfig()}).PluginComponent(), + (PricingComponentSpec{Config: NewPricingConfig()}).PluginComponent(), + (PiiRedactionComponentSpec{Config: NewPiiRedactionConfig()}).PluginComponent(), + } + + assertEnabledPresence := func(component PluginComponentSpec, wantPresent bool) { + t.Helper() + payload, err := json.Marshal(component) + if err != nil { + t.Fatalf("marshal %s component: %v", component.Kind, err) + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(payload, &fields); err != nil { + t.Fatalf("unmarshal %s component JSON: %v", component.Kind, err) + } + enabled, present := fields["enabled"] + if present != wantPresent { + t.Fatalf("%s enabled presence = %t, want %t: %s", component.Kind, present, wantPresent, payload) + } + if wantPresent && string(enabled) != "false" { + t.Fatalf("%s enabled JSON = %s, want false", component.Kind, enabled) + } + } + + for _, component := range explicitDisabled { + assertEnabledPresence(component, true) + } + for _, component := range legacyDefaults { + assertEnabledPresence(component, false) + } + + wrapperPayload, err := json.Marshal( + NewAdaptiveComponentSpec(NewAdaptiveConfig()).WithEnabled(false), + ) + if err != nil { + t.Fatalf("marshal explicit adaptive wrapper: %v", err) + } + decodedWrapper := NewAdaptiveComponentSpec(NewAdaptiveConfig()) + if err := json.Unmarshal(wrapperPayload, &decodedWrapper); err != nil { + t.Fatalf("unmarshal explicit adaptive wrapper: %v", err) + } + if decodedWrapper.Enabled || !decodedWrapper.enabledSet { + t.Fatalf("explicit wrapper enabled state was not preserved: %#v", decodedWrapper) + } + assertEnabledPresence(decodedWrapper.PluginComponent(), true) + + if err := json.Unmarshal([]byte(`{"config":{}}`), &decodedWrapper); err != nil { + t.Fatalf("unmarshal adaptive wrapper with omitted enabled: %v", err) + } + if decodedWrapper.Enabled || decodedWrapper.enabledSet { + t.Fatalf("omitted wrapper enabled retained stale state: %#v", decodedWrapper) + } + assertEnabledPresence(decodedWrapper.PluginComponent(), false) +} + func TestActivateDynamicPluginsNormalizesNilSpecs(t *testing.T) { withPluginActivationStubs(t) @@ -208,8 +357,8 @@ func TestPluginActivationCloseFreesAfterClearFailure(t *testing.T) { if err := activation.Close(); !errors.Is(err, wantErr) { t.Fatalf("Close() error = %v, want %v", err, wantErr) } - if err := activation.Close(); err != nil { - t.Fatalf("repeated Close() error = %v", err) + if err := activation.Close(); !errors.Is(err, wantErr) { + t.Fatalf("repeated Close() error = %v, want %v", err, wantErr) } if strings.Join(calls, ",") != "clear,free" { t.Fatalf("cleanup calls = %v", calls) @@ -217,11 +366,12 @@ func TestPluginActivationCloseFreesAfterClearFailure(t *testing.T) { runtime.KeepAlive(token) } -func TestPluginActivationCopiesShareCloseState(t *testing.T) { +func TestPluginActivationCopiesShareCloseStateAndError(t *testing.T) { withPluginActivationStubs(t) token := new(byte) ptr := unsafe.Pointer(token) + wantErr := errors.New("teardown failed") var callsMu sync.Mutex var calls []string clearPluginActivation = func(got unsafe.Pointer) error { @@ -231,7 +381,7 @@ func TestPluginActivationCopiesShareCloseState(t *testing.T) { callsMu.Lock() calls = append(calls, "clear") callsMu.Unlock() - return nil + return wantErr } freePluginActivation = func(got unsafe.Pointer) { callsMu.Lock() @@ -245,22 +395,25 @@ func TestPluginActivationCopiesShareCloseState(t *testing.T) { activation := newPluginActivation(ptr) copyValue := *activation - errors := make(chan error, 2) + closeErrors := make(chan error, 2) var closeCalls sync.WaitGroup for _, handle := range []*PluginActivation{activation, ©Value} { closeCalls.Add(1) go func(handle *PluginActivation) { defer closeCalls.Done() - errors <- handle.Close() + closeErrors <- handle.Close() }(handle) } closeCalls.Wait() - close(errors) - for err := range errors { - if err != nil { - t.Fatalf("Close() error = %v", err) + close(closeErrors) + for err := range closeErrors { + if !errors.Is(err, wantErr) { + t.Fatalf("Close() error = %v, want %v", err, wantErr) } } + if err := activation.Close(); !errors.Is(err, wantErr) { + t.Fatalf("repeated Close() error = %v, want %v", err, wantErr) + } callsMu.Lock() gotCalls := strings.Join(calls, ",") From fa451847f092c51f24485709a2c497a1478d3ab3 Mon Sep 17 00:00:00 2001 From: Bryan Bednarski Date: Tue, 7 Jul 2026 13:54:50 -0600 Subject: [PATCH 04/11] fix(ffi): preserve static-only plugin activation Signed-off-by: Bryan Bednarski --- crates/ffi/README.md | 4 + crates/ffi/nemo_relay.h | 13 ++- crates/ffi/src/api/plugin.rs | 21 ++-- crates/ffi/tests/integration/api_tests.rs | 2 +- crates/ffi/tests/unit/api/plugin_tests.rs | 125 ++++++---------------- crates/ffi/tests/unit/api_tests.rs | 12 +-- go/nemo_relay/README.md | 4 + go/nemo_relay/plugin.go | 17 ++- go/nemo_relay/plugin_activation_test.go | 107 +++++++++++++----- 9 files changed, 159 insertions(+), 146 deletions(-) diff --git a/crates/ffi/README.md b/crates/ffi/README.md index 3b70e7f15..b3b2f73a1 100644 --- a/crates/ffi/README.md +++ b/crates/ffi/README.md @@ -23,6 +23,10 @@ shared Rust runtime contract. This surface is experimental and source-first. The repository-maintained Go binding consumes it through CGo. +> **DO NOT TREAT AS PRODUCTION-READY:** the FFI/Go dynamic-plugin activation +> lifecycle needs a real consumer to validate shutdown, ownership, and error +> handling before it can be promoted to a stable contract. + ## Why Use It? - **Expose NeMo Relay to native consumers**: Call the shared Rust runtime from diff --git a/crates/ffi/nemo_relay.h b/crates/ffi/nemo_relay.h index 35763c40c..d3875eb89 100644 --- a/crates/ffi/nemo_relay.h +++ b/crates/ffi/nemo_relay.h @@ -1419,10 +1419,15 @@ NemoRelayStatus nemo_relay_openinference_subscriber_shutdown(const struct FfiOpe /** * Load and activate dynamic plugins as one owned transaction. * - * `config_json` is the base [`PluginConfig`] document and - * `dynamic_plugins_json` is an array of explicit dynamic-plugin activation - * specifications. On success, the caller owns `out_activation` and must clear - * and free it with `nemo_relay_plugin_activation_clear` and + * **Experimental:** this API needs a production consumer before its lifecycle + * contract is considered stable. + * + * `config_json` is the base [`PluginConfig`] document. Its static components + * initialize before components appended by the dynamic plugins. + * `dynamic_plugins_json` must contain at least one explicit dynamic-plugin + * activation specification; use `nemo_relay_initialize_plugins` for a + * static-only configuration. On success, the caller owns `out_activation` and + * must clear and free it with `nemo_relay_plugin_activation_clear` and * `nemo_relay_plugin_activation_free`. `out_report_json` is a library-owned C * string and must be released with `nemo_relay_string_free`. * diff --git a/crates/ffi/src/api/plugin.rs b/crates/ffi/src/api/plugin.rs index ea0e5fadd..2e49e1a51 100644 --- a/crates/ffi/src/api/plugin.rs +++ b/crates/ffi/src/api/plugin.rs @@ -13,10 +13,10 @@ use super::{ initialize_plugins, json_to_c_string, last_error_message, list_plugin_kinds, nemo_relay_string_free, register_adaptive_component, register_plugin, set_last_error, status_from_plugin_error, tokio_runtime, validate_plugin_config, wrap_event_sanitize_fn, - wrap_event_subscriber, wrap_llm_conditional_fn, - wrap_llm_exec_intercept_fn, wrap_llm_request_intercept_fn, wrap_llm_response_fn, - wrap_llm_sanitize_request_fn, wrap_llm_stream_exec_intercept_fn, wrap_tool_conditional_fn, - wrap_tool_exec_intercept_fn, wrap_tool_request_intercept_fn, wrap_tool_sanitize_fn, + wrap_event_subscriber, wrap_llm_conditional_fn, wrap_llm_exec_intercept_fn, + wrap_llm_request_intercept_fn, wrap_llm_response_fn, wrap_llm_sanitize_request_fn, + wrap_llm_stream_exec_intercept_fn, wrap_tool_conditional_fn, wrap_tool_exec_intercept_fn, + wrap_tool_request_intercept_fn, wrap_tool_sanitize_fn, }; use crate::api::event_registry::Surface; use nemo_relay_pii_redaction::component::register_pii_redaction_component; @@ -170,10 +170,15 @@ fn lock_plugin_activation( /// Load and activate dynamic plugins as one owned transaction. /// -/// `config_json` is the base [`PluginConfig`] document and -/// `dynamic_plugins_json` is an array of explicit dynamic-plugin activation -/// specifications. On success, the caller owns `out_activation` and must clear -/// and free it with `nemo_relay_plugin_activation_clear` and +/// **Experimental:** this API needs a production consumer before its lifecycle +/// contract is considered stable. +/// +/// `config_json` is the base [`PluginConfig`] document. Its static components +/// initialize before components appended by the dynamic plugins. +/// `dynamic_plugins_json` must contain at least one explicit dynamic-plugin +/// activation specification; use `nemo_relay_initialize_plugins` for a +/// static-only configuration. On success, the caller owns `out_activation` and +/// must clear and free it with `nemo_relay_plugin_activation_clear` and /// `nemo_relay_plugin_activation_free`. `out_report_json` is a library-owned C /// string and must be released with `nemo_relay_string_free`. /// diff --git a/crates/ffi/tests/integration/api_tests.rs b/crates/ffi/tests/integration/api_tests.rs index fd934d67b..5afdadf8c 100644 --- a/crates/ffi/tests/integration/api_tests.rs +++ b/crates/ffi/tests/integration/api_tests.rs @@ -27,7 +27,7 @@ use nemo_relay_ffi::types::{ nemo_relay_event_uuid, nemo_relay_llm_handle_attributes, nemo_relay_llm_handle_free, nemo_relay_llm_handle_name, nemo_relay_llm_handle_parent_uuid, nemo_relay_llm_handle_uuid, nemo_relay_llm_request_content, nemo_relay_llm_request_free, nemo_relay_llm_request_headers, - nemo_relay_llm_request_new, nemo_relay_otel_subscriber_free, nemo_relay_plugin_activation_free, + nemo_relay_llm_request_new, nemo_relay_otel_subscriber_free, nemo_relay_scope_handle_attributes, nemo_relay_scope_handle_data, nemo_relay_scope_handle_free, nemo_relay_scope_handle_metadata, nemo_relay_scope_handle_name, nemo_relay_scope_handle_parent_uuid, nemo_relay_scope_handle_scope_type, diff --git a/crates/ffi/tests/unit/api/plugin_tests.rs b/crates/ffi/tests/unit/api/plugin_tests.rs index d151451f8..af5c75fec 100644 --- a/crates/ffi/tests/unit/api/plugin_tests.rs +++ b/crates/ffi/tests/unit/api/plugin_tests.rs @@ -6,96 +6,34 @@ use super::*; #[test] -fn test_ffi_dynamic_plugin_activation_owns_an_idempotent_lifecycle() { +fn test_ffi_dynamic_plugin_activation_rejects_empty_specs_without_outputs() { let _guard = TEST_MUTEX.lock().unwrap(); reset_globals(); let _ = nemo_relay_clear_plugin_configuration(); let config = cstring(r#"{"version":1,"components":[]}"#); let specs = cstring("[]"); - let mut activation = ptr::null_mut(); - let mut report_json = ptr::null_mut(); - unsafe { - assert_eq!( - nemo_relay_activate_dynamic_plugins( - config.as_ptr(), - specs.as_ptr(), - &mut activation, - &mut report_json, - ), - NemoRelayStatus::Ok - ); - assert!(!activation.is_null()); - assert_eq!(returned_json(report_json)["diagnostics"], json!([])); - - let mut conflicting_activation = ptr::null_mut(); - let mut conflicting_report = ptr::null_mut(); - assert_eq!( - nemo_relay_activate_dynamic_plugins( - config.as_ptr(), - specs.as_ptr(), - &mut conflicting_activation, - &mut conflicting_report, - ), - NemoRelayStatus::AlreadyExists - ); - assert!(conflicting_activation.is_null()); - assert!(conflicting_report.is_null()); - assert!( - read_last_error() - .unwrap_or_default() - .contains("active dynamic plugin host") - ); - - assert_eq!( - nemo_relay_plugin_activation_clear(activation), - NemoRelayStatus::Ok - ); - assert_eq!( - nemo_relay_plugin_activation_clear(activation), - NemoRelayStatus::Ok - ); - assert_eq!( - nemo_relay_plugin_activation_clear(ptr::null_mut()), - NemoRelayStatus::Ok - ); - nemo_relay_plugin_activation_free(&mut activation); - assert!(activation.is_null()); - nemo_relay_plugin_activation_free(&mut activation); - nemo_relay_plugin_activation_free(ptr::null_mut()); - - let mut dropped_activation = ptr::null_mut(); - let mut dropped_report = ptr::null_mut(); - assert_eq!( - nemo_relay_activate_dynamic_plugins( - config.as_ptr(), - specs.as_ptr(), - &mut dropped_activation, - &mut dropped_report, - ), - NemoRelayStatus::Ok - ); - returned_json(dropped_report); - nemo_relay_plugin_activation_free(&mut dropped_activation); - assert!(dropped_activation.is_null()); - - let mut after_drop_activation = ptr::null_mut(); - let mut after_drop_report = ptr::null_mut(); - assert_eq!( - nemo_relay_activate_dynamic_plugins( - config.as_ptr(), - specs.as_ptr(), - &mut after_drop_activation, - &mut after_drop_report, - ), - NemoRelayStatus::Ok - ); - returned_json(after_drop_report); - assert_eq!( - nemo_relay_plugin_activation_clear(after_drop_activation), - NemoRelayStatus::Ok - ); - nemo_relay_plugin_activation_free(&mut after_drop_activation); + for _ in 0..2 { + let mut activation = std::ptr::dangling_mut::(); + let mut report_json = std::ptr::dangling_mut::(); + unsafe { + assert_eq!( + nemo_relay_activate_dynamic_plugins( + config.as_ptr(), + specs.as_ptr(), + &mut activation, + &mut report_json, + ), + NemoRelayStatus::InvalidArg + ); + assert!(activation.is_null()); + assert!(report_json.is_null()); + assert!( + read_last_error() + .unwrap_or_default() + .contains("at least one dynamic plugin") + ); + } } } @@ -218,22 +156,19 @@ fn test_ffi_dynamic_plugin_activation_surfaces_load_failures_and_releases_owner( .contains("native plugin load failed") ); - let empty_specs = cstring("[]"); + let mut retry_activation = ptr::null_mut(); + let mut retry_report = ptr::null_mut(); assert_eq!( nemo_relay_activate_dynamic_plugins( config.as_ptr(), - empty_specs.as_ptr(), - &mut activation, - &mut report, + specs.as_ptr(), + &mut retry_activation, + &mut retry_report, ), - NemoRelayStatus::Ok - ); - returned_json(report); - assert_eq!( - nemo_relay_plugin_activation_clear(activation), - NemoRelayStatus::Ok + NemoRelayStatus::NotFound ); - nemo_relay_plugin_activation_free(&mut activation); + assert!(retry_activation.is_null()); + assert!(retry_report.is_null()); } } diff --git a/crates/ffi/tests/unit/api_tests.rs b/crates/ffi/tests/unit/api_tests.rs index b95b65be4..4c8c19c7b 100644 --- a/crates/ffi/tests/unit/api_tests.rs +++ b/crates/ffi/tests/unit/api_tests.rs @@ -25,12 +25,12 @@ use crate::types::{ nemo_relay_llm_handle_free, nemo_relay_llm_handle_name, nemo_relay_llm_handle_parent_uuid, nemo_relay_llm_handle_uuid, nemo_relay_llm_request_content, nemo_relay_llm_request_free, nemo_relay_llm_request_headers, nemo_relay_llm_request_new, nemo_relay_otel_subscriber_free, - nemo_relay_plugin_activation_free, nemo_relay_scope_handle_attributes, - nemo_relay_scope_handle_data, nemo_relay_scope_handle_free, nemo_relay_scope_handle_metadata, - nemo_relay_scope_handle_name, nemo_relay_scope_handle_parent_uuid, - nemo_relay_scope_handle_scope_type, nemo_relay_scope_handle_uuid, nemo_relay_scope_stack_free, - nemo_relay_tool_handle_attributes, nemo_relay_tool_handle_free, nemo_relay_tool_handle_name, - nemo_relay_tool_handle_parent_uuid, nemo_relay_tool_handle_uuid, + nemo_relay_scope_handle_attributes, nemo_relay_scope_handle_data, nemo_relay_scope_handle_free, + nemo_relay_scope_handle_metadata, nemo_relay_scope_handle_name, + nemo_relay_scope_handle_parent_uuid, nemo_relay_scope_handle_scope_type, + nemo_relay_scope_handle_uuid, nemo_relay_scope_stack_free, nemo_relay_tool_handle_attributes, + nemo_relay_tool_handle_free, nemo_relay_tool_handle_name, nemo_relay_tool_handle_parent_uuid, + nemo_relay_tool_handle_uuid, }; use crate::{api, callable, types}; diff --git a/go/nemo_relay/README.md b/go/nemo_relay/README.md index 81baaba83..d0b130cda 100644 --- a/go/nemo_relay/README.md +++ b/go/nemo_relay/README.md @@ -24,6 +24,10 @@ Rust runtime. This binding is experimental and source-first. Rust, Python, and Node.js are the primary supported surfaces. +> **DO NOT TREAT AS PRODUCTION-READY:** the FFI/Go dynamic-plugin activation +> lifecycle needs a real consumer to validate shutdown, ownership, and error +> handling before it can be promoted to a stable contract. + ## Why Use It? - **Use NeMo Relay from Go**: Group agent, tool, and LLM work into the same diff --git a/go/nemo_relay/plugin.go b/go/nemo_relay/plugin.go index c2864525c..7ac23b659 100644 --- a/go/nemo_relay/plugin.go +++ b/go/nemo_relay/plugin.go @@ -262,6 +262,9 @@ type DynamicPluginActivationSpec struct { // PluginActivation owns the runtime registrations, native libraries, and // workers created by ActivateDynamicPlugins. Copies share one activation // lifetime and may be closed safely from any copy. +// +// Experimental: this API needs a production consumer before its lifecycle +// contract is considered stable. type PluginActivation struct { state *pluginActivationState } @@ -498,20 +501,28 @@ func InitializePlugins(config PluginConfig) (ConfigReport, error) { // ActivateDynamicPlugins loads explicit dynamic plugins and activates them // together with the supplied base plugin configuration as one transaction. +// Static components in config are initialized before components appended by +// dynamic plugins. At least one dynamic plugin specification is required; use +// InitializePlugins for a static-only configuration. // // The caller must retain the returned activation for as long as plugin // callbacks may run and call Close during orderly shutdown. +// +// Experimental: this API needs a production consumer before its lifecycle +// contract is considered stable. func ActivateDynamicPlugins( config PluginConfig, dynamicPlugins []DynamicPluginActivationSpec, ) (*PluginActivation, ConfigReport, error) { + if len(dynamicPlugins) == 0 { + return nil, ConfigReport{}, errors.New( + "dynamic plugin activation requires at least one dynamic plugin; use InitializePlugins for a static-only configuration", + ) + } configPayload, err := jsonMarshal(config) if err != nil { return nil, ConfigReport{}, err } - if dynamicPlugins == nil { - dynamicPlugins = []DynamicPluginActivationSpec{} - } dynamicPluginsPayload, err := jsonMarshal(dynamicPlugins) if err != nil { return nil, ConfigReport{}, err diff --git a/go/nemo_relay/plugin_activation_test.go b/go/nemo_relay/plugin_activation_test.go index d995c63f2..2ac1a235b 100644 --- a/go/nemo_relay/plugin_activation_test.go +++ b/go/nemo_relay/plugin_activation_test.go @@ -39,6 +39,14 @@ func withPluginActivationStubs(t *testing.T) { }) } +func fixtureDynamicPluginSpecs() []DynamicPluginActivationSpec { + return []DynamicPluginActivationSpec{{ + PluginID: "fixture.native", + Kind: DynamicPluginKindRustDynamic, + ManifestRef: "/tmp/relay-plugin.toml", + }} +} + func TestActivateDynamicPluginsSerializesSpecsAndOwnsCleanup(t *testing.T) { withPluginActivationStubs(t) @@ -125,8 +133,9 @@ func TestActivateDynamicPluginsPreservesLegacyOmittedDefaults(t *testing.T) { if configJSON != wantConfig { t.Fatalf("config JSON = %s, want %s", configJSON, wantConfig) } - if specsJSON != "[]" { - t.Fatalf("dynamic plugin specs JSON = %s, want []", specsJSON) + const wantSpecs = `[{"plugin_id":"fixture.native","kind":"rust_dynamic","manifest_ref":"/tmp/relay-plugin.toml"}]` + if specsJSON != wantSpecs { + t.Fatalf("dynamic plugin specs JSON = %s, want %s", specsJSON, wantSpecs) } return unsafe.Pointer(token), `{"diagnostics":[]}`, nil } @@ -137,7 +146,7 @@ func TestActivateDynamicPluginsPreservesLegacyOmittedDefaults(t *testing.T) { Components: []PluginComponentSpec{ {Kind: "fixture.default"}, }, - }, nil) + }, fixtureDynamicPluginSpecs()) if err != nil { t.Fatalf("ActivateDynamicPlugins() error = %v", err) } @@ -156,8 +165,9 @@ func TestActivateDynamicPluginsPreservesExplicitZeroAndFalse(t *testing.T) { if configJSON != wantConfig { t.Fatalf("config JSON = %s, want %s", configJSON, wantConfig) } - if specsJSON != "[]" { - t.Fatalf("dynamic plugin specs JSON = %s, want []", specsJSON) + const wantSpecs = `[{"plugin_id":"fixture.native","kind":"rust_dynamic","manifest_ref":"/tmp/relay-plugin.toml"}]` + if specsJSON != wantSpecs { + t.Fatalf("dynamic plugin specs JSON = %s, want %s", specsJSON, wantSpecs) } var roundTrip PluginConfig if err := json.Unmarshal([]byte(configJSON), &roundTrip); err != nil { @@ -178,7 +188,7 @@ func TestActivateDynamicPluginsPreservesExplicitZeroAndFalse(t *testing.T) { component := (PluginComponentSpec{Kind: "fixture.disabled"}).WithEnabled(false) config := PluginConfig{Components: []PluginComponentSpec{component}} config.SetVersion(0) - activation, _, err := ActivateDynamicPlugins(config, nil) + activation, _, err := ActivateDynamicPlugins(config, fixtureDynamicPluginSpecs()) if err != nil { t.Fatalf("ActivateDynamicPlugins() error = %v", err) } @@ -289,27 +299,27 @@ func TestComponentWrapperEnabledPresenceSurvivesConversion(t *testing.T) { assertEnabledPresence(decodedWrapper.PluginComponent(), false) } -func TestActivateDynamicPluginsNormalizesNilSpecs(t *testing.T) { +func TestActivateDynamicPluginsRejectsEmptySpecsWithoutCallingCgo(t *testing.T) { withPluginActivationStubs(t) - token := new(byte) - activateDynamicPluginsJSON = func(_ string, specsJSON string) (unsafe.Pointer, string, error) { - if specsJSON != "[]" { - t.Fatalf("nil specs encoded as %q, want []", specsJSON) - } - return unsafe.Pointer(token), `{"diagnostics":[]}`, nil + activationCalls := 0 + activateDynamicPluginsJSON = func(string, string) (unsafe.Pointer, string, error) { + activationCalls++ + return nil, "", errors.New("unexpected CGo call") } - clearPluginActivation = func(unsafe.Pointer) error { return nil } - freePluginActivation = func(unsafe.Pointer) {} - activation, _, err := ActivateDynamicPlugins(NewPluginConfig(), nil) - if err != nil { - t.Fatalf("ActivateDynamicPlugins() error = %v", err) + for _, specs := range [][]DynamicPluginActivationSpec{nil, {}} { + activation, report, err := ActivateDynamicPlugins(NewPluginConfig(), specs) + if err == nil || !strings.Contains(err.Error(), "at least one dynamic plugin") { + t.Fatalf("ActivateDynamicPlugins(%#v) error = %v, want empty-spec diagnostic", specs, err) + } + if activation != nil || len(report.Diagnostics) != 0 { + t.Fatalf("ActivateDynamicPlugins(%#v) = (%#v, %#v), want empty outputs", specs, activation, report) + } } - if err := activation.Close(); err != nil { - t.Fatalf("Close() error = %v", err) + if activationCalls != 0 { + t.Fatalf("activation calls = %d, want 0", activationCalls) } - runtime.KeepAlive(token) } func TestActivateDynamicPluginsCleansUpInvalidReport(t *testing.T) { @@ -327,7 +337,7 @@ func TestActivateDynamicPluginsCleansUpInvalidReport(t *testing.T) { } freePluginActivation = func(unsafe.Pointer) { calls = append(calls, "free") } - activation, _, err := ActivateDynamicPlugins(NewPluginConfig(), nil) + activation, _, err := ActivateDynamicPlugins(NewPluginConfig(), fixtureDynamicPluginSpecs()) if err == nil { t.Fatal("ActivateDynamicPlugins() error = nil, want invalid report error") } @@ -523,7 +533,7 @@ func TestActivateDynamicPluginsSurfacesSerializationAndActivationErrors(t *testi Enabled: true, Config: map[string]any{"invalid": make(chan int)}, }) - if _, _, err := ActivateDynamicPlugins(invalidConfig, nil); err == nil { + if _, _, err := ActivateDynamicPlugins(invalidConfig, fixtureDynamicPluginSpecs()); err == nil { t.Fatal("invalid config serialization error = nil") } if activationCalls != 0 { @@ -543,7 +553,7 @@ func TestActivateDynamicPluginsSurfacesSerializationAndActivationErrors(t *testi t.Fatalf("activation calls after specs serialization failure = %d", activationCalls) } - if _, _, err := ActivateDynamicPlugins(NewPluginConfig(), nil); err == nil || err.Error() != "load failed" { + if _, _, err := ActivateDynamicPlugins(NewPluginConfig(), fixtureDynamicPluginSpecs()); err == nil || err.Error() != "load failed" { t.Fatalf("activation error = %v, want load failed", err) } if activationCalls != 1 { @@ -562,10 +572,38 @@ func TestActivateDynamicPluginsLoadsNativePluginThroughCgo(t *testing.T) { if err := ClearPluginConfiguration(); err != nil { t.Fatalf("ClearPluginConfiguration() error = %v", err) } + const staticKind = "go.fixture.static_base" + if err := RegisterPlugin(staticKind, PluginFuncs{ + RegisterFunc: func(_ map[string]any, ctx *PluginContext) error { + return ctx.RegisterToolRequestIntercept( + "go_static_base", + -1, + false, + func(_ string, args json.RawMessage) json.RawMessage { + var payload map[string]any + if err := json.Unmarshal(args, &payload); err != nil { + return args + } + payload["go_static_base"] = true + out, _ := json.Marshal(payload) + return out + }, + ) + }, + }); err != nil { + t.Fatalf("RegisterPlugin() error = %v", err) + } + defer func() { + if err := DeregisterPlugin(staticKind); err != nil { + t.Errorf("DeregisterPlugin() error = %v", err) + } + }() library := goNativePluginFixture(t) manifest := writeGoNativePluginManifest(t, library) - activation, report, err := ActivateDynamicPlugins(NewPluginConfig(), []DynamicPluginActivationSpec{{ + config := NewPluginConfig() + config.Components = append(config.Components, NewPluginComponent(staticKind)) + activation, report, err := ActivateDynamicPlugins(config, []DynamicPluginActivationSpec{{ PluginID: "fixture_native", Kind: DynamicPluginKindRustDynamic, ManifestRef: manifest, @@ -594,6 +632,9 @@ func TestActivateDynamicPluginsLoadsNativePluginThroughCgo(t *testing.T) { if transformedObject["native_plugin"] != true { t.Fatalf("transformed tool args = %s, want native_plugin marker", transformed) } + if transformedObject["go_static_base"] != true { + t.Fatalf("transformed tool args = %s, want static base marker", transformed) + } if err := activation.Close(); err != nil { t.Fatalf("Close() error = %v", err) @@ -678,13 +719,21 @@ func TestPluginActivationFinalizerReleasesHostOwnership(t *testing.T) { if err := ClearPluginConfiguration(); err != nil { t.Fatalf("ClearPluginConfiguration() error = %v", err) } - createUnclosedPluginActivation(t) + library := goNativePluginFixture(t) + manifest := writeGoNativePluginManifest(t, library) + specs := []DynamicPluginActivationSpec{{ + PluginID: "fixture_native", + Kind: DynamicPluginKindRustDynamic, + ManifestRef: manifest, + Config: map[string]any{}, + }} + createUnclosedPluginActivation(t, specs) deadline := time.Now().Add(10 * time.Second) for time.Now().Before(deadline) { runtime.GC() runtime.Gosched() - activation, _, err := ActivateDynamicPlugins(NewPluginConfig(), nil) + activation, _, err := ActivateDynamicPlugins(NewPluginConfig(), specs) if err == nil { if closeErr := activation.Close(); closeErr != nil { t.Fatalf("replacement activation Close() error = %v", closeErr) @@ -700,9 +749,9 @@ func TestPluginActivationFinalizerReleasesHostOwnership(t *testing.T) { // caller starts forcing collection. // //go:noinline -func createUnclosedPluginActivation(t *testing.T) { +func createUnclosedPluginActivation(t *testing.T, specs []DynamicPluginActivationSpec) { t.Helper() - activation, _, err := ActivateDynamicPlugins(NewPluginConfig(), nil) + activation, _, err := ActivateDynamicPlugins(NewPluginConfig(), specs) if err != nil { t.Fatalf("ActivateDynamicPlugins() error = %v", err) } From f3d06d832ff3b7eb6d3f467adebe518316f2e272 Mon Sep 17 00:00:00 2001 From: Bryan Bednarski Date: Mon, 13 Jul 2026 23:28:00 -0600 Subject: [PATCH 05/11] fix(ffi): harden dynamic activation bindings Signed-off-by: Bryan Bednarski --- crates/ffi/nemo_relay.h | 25 +- crates/ffi/src/api/plugin.rs | 57 +++-- .../integration/plugin_activation_tests.rs | 43 ++++ go/nemo_relay/adaptive.go | 43 +--- go/nemo_relay/model_pricing.go | 43 +--- go/nemo_relay/observability_plugin.go | 43 +--- go/nemo_relay/pii_redaction.go | 43 +--- go/nemo_relay/plugin.go | 185 +++----------- go/nemo_relay/plugin_activation_test.go | 234 +++++++----------- 9 files changed, 259 insertions(+), 457 deletions(-) diff --git a/crates/ffi/nemo_relay.h b/crates/ffi/nemo_relay.h index d3875eb89..f39ddce74 100644 --- a/crates/ffi/nemo_relay.h +++ b/crates/ffi/nemo_relay.h @@ -1422,18 +1422,37 @@ NemoRelayStatus nemo_relay_openinference_subscriber_shutdown(const struct FfiOpe * **Experimental:** this API needs a production consumer before its lifecycle * contract is considered stable. * - * `config_json` is the base [`PluginConfig`] document. Its static components + * `config_json` is the base `PluginConfig` document. Its static components * initialize before components appended by the dynamic plugins. * `dynamic_plugins_json` must contain at least one explicit dynamic-plugin * activation specification; use `nemo_relay_initialize_plugins` for a - * static-only configuration. On success, the caller owns `out_activation` and + * static-only configuration. + * + * The base configuration uses this JSON shape: + * + * ```text + * {"version":1,"components":[{"kind":"static.kind","enabled":true,"config":{}}]} + * ``` + * + * Dynamic plugin specifications use this JSON shape: + * + * ```text + * [{"plugin_id":"example","kind":"rust_dynamic","manifest_ref":"/absolute/path/relay-plugin.toml","config":{}}] + * ``` + * + * `kind` must be `rust_dynamic` or `worker`. `environment_ref` is optional + * and applies only to worker plugins. `manifest_ref` is resolved by the + * embedding application; this API does not discover installed plugins. + * + * On success, the caller owns `out_activation` and * must clear and free it with `nemo_relay_plugin_activation_clear` and * `nemo_relay_plugin_activation_free`. `out_report_json` is a library-owned C * string and must be released with `nemo_relay_string_free`. * * # Safety * Both input pointers must reference valid, null-terminated C strings. - * `out_activation` and `out_report_json` must be valid, non-null output pointers. + * `out_activation` and `out_report_json` must be valid, non-null, non-overlapping + * output pointers. */ NemoRelayStatus nemo_relay_activate_dynamic_plugins(const char *config_json, const char *dynamic_plugins_json, diff --git a/crates/ffi/src/api/plugin.rs b/crates/ffi/src/api/plugin.rs index 2e49e1a51..e04e1c0ea 100644 --- a/crates/ffi/src/api/plugin.rs +++ b/crates/ffi/src/api/plugin.rs @@ -154,15 +154,9 @@ fn parse_dynamic_plugin_specs( } fn lock_plugin_activation( - activation: *mut FfiPluginActivation, -) -> std::result::Result< - std::sync::MutexGuard<'static, Option>, - NemoRelayStatus, -> { - if activation.is_null() { - return Err(NemoRelayStatus::NullPointer); - } - unsafe { &*activation }.0.lock().map_err(|error| { + activation: &FfiPluginActivation, +) -> std::result::Result>, NemoRelayStatus> { + activation.0.lock().map_err(|error| { set_last_error(&format!("plugin activation lock poisoned: {error}")); NemoRelayStatus::Internal }) @@ -173,18 +167,37 @@ fn lock_plugin_activation( /// **Experimental:** this API needs a production consumer before its lifecycle /// contract is considered stable. /// -/// `config_json` is the base [`PluginConfig`] document. Its static components +/// `config_json` is the base `PluginConfig` document. Its static components /// initialize before components appended by the dynamic plugins. /// `dynamic_plugins_json` must contain at least one explicit dynamic-plugin /// activation specification; use `nemo_relay_initialize_plugins` for a -/// static-only configuration. On success, the caller owns `out_activation` and +/// static-only configuration. +/// +/// The base configuration uses this JSON shape: +/// +/// ```text +/// {"version":1,"components":[{"kind":"static.kind","enabled":true,"config":{}}]} +/// ``` +/// +/// Dynamic plugin specifications use this JSON shape: +/// +/// ```text +/// [{"plugin_id":"example","kind":"rust_dynamic","manifest_ref":"/absolute/path/relay-plugin.toml","config":{}}] +/// ``` +/// +/// `kind` must be `rust_dynamic` or `worker`. `environment_ref` is optional +/// and applies only to worker plugins. `manifest_ref` is resolved by the +/// embedding application; this API does not discover installed plugins. +/// +/// On success, the caller owns `out_activation` and /// must clear and free it with `nemo_relay_plugin_activation_clear` and /// `nemo_relay_plugin_activation_free`. `out_report_json` is a library-owned C /// string and must be released with `nemo_relay_string_free`. /// /// # Safety /// Both input pointers must reference valid, null-terminated C strings. -/// `out_activation` and `out_report_json` must be valid, non-null output pointers. +/// `out_activation` and `out_report_json` must be valid, non-null, non-overlapping +/// output pointers. #[unsafe(no_mangle)] pub unsafe extern "C" fn nemo_relay_activate_dynamic_plugins( config_json: *const c_char, @@ -193,20 +206,27 @@ pub unsafe extern "C" fn nemo_relay_activate_dynamic_plugins( out_report_json: *mut *mut c_char, ) -> NemoRelayStatus { clear_last_error(); - if !out_activation.is_null() { - unsafe { *out_activation = std::ptr::null_mut() }; - } - if !out_report_json.is_null() { - unsafe { *out_report_json = std::ptr::null_mut() }; - } if out_activation.is_null() { + if !out_report_json.is_null() { + unsafe { *out_report_json = std::ptr::null_mut() }; + } set_last_error("out_activation pointer is null"); return NemoRelayStatus::NullPointer; } if out_report_json.is_null() { + unsafe { *out_activation = std::ptr::null_mut() }; set_last_error("out_report_json pointer is null"); return NemoRelayStatus::NullPointer; } + if out_activation.cast::() == out_report_json.cast::() { + unsafe { *out_activation = std::ptr::null_mut() }; + set_last_error("out_activation and out_report_json must not overlap"); + return NemoRelayStatus::InvalidArg; + } + unsafe { + *out_activation = std::ptr::null_mut(); + *out_report_json = std::ptr::null_mut(); + } if let Err(status) = ensure_adaptive_component_registered() { return status; @@ -262,6 +282,7 @@ pub unsafe extern "C" fn nemo_relay_plugin_activation_clear( if activation.is_null() { return NemoRelayStatus::Ok; } + let activation = unsafe { &*activation }; let mut guard = match lock_plugin_activation(activation) { Ok(guard) => guard, Err(status) => return status, diff --git a/crates/ffi/tests/integration/plugin_activation_tests.rs b/crates/ffi/tests/integration/plugin_activation_tests.rs index f3a9d30d0..08c33626b 100644 --- a/crates/ffi/tests/integration/plugin_activation_tests.rs +++ b/crates/ffi/tests/integration/plugin_activation_tests.rs @@ -65,6 +65,49 @@ fn ffi_activation_loads_native_callbacks_and_removes_them_before_free() { ); } +#[test] +fn ffi_activation_rejects_overlapping_outputs_without_claiming_host() { + let _guard = TEST_MUTEX.lock().unwrap(); + let _ = nemo_relay_clear_plugin_configuration(); + + let manifest_dir = TempDir::new().expect("native manifest tempdir"); + let manifest = write_native_manifest(manifest_dir.path(), build_native_fixture()); + let config = cstring(r#"{"version":1,"components":[]}"#); + let specs_value = json!([{ + "plugin_id": "fixture_native", + "kind": "rust_dynamic", + "manifest_ref": manifest, + "config": {} + }]); + let specs = cstring(&specs_value.to_string()); + let mut aliased_output = std::ptr::dangling_mut::(); + let output_slot = &mut aliased_output as *mut *mut std::ffi::c_void; + let status = unsafe { + api::nemo_relay_activate_dynamic_plugins( + config.as_ptr(), + specs.as_ptr(), + output_slot.cast::<*mut FfiPluginActivation>(), + output_slot.cast::<*mut c_char>(), + ) + }; + assert_eq!(status, NemoRelayStatus::InvalidArg); + assert!(aliased_output.is_null()); + assert!( + unsafe { read_last_error() } + .unwrap_or_default() + .contains("must not overlap") + ); + + let (mut activation, _) = activate_plugins(specs_value); + unsafe { + assert_eq!( + api::nemo_relay_plugin_activation_clear(activation), + NemoRelayStatus::Ok + ); + nemo_relay_plugin_activation_free(&mut activation); + } +} + #[test] fn ffi_activation_loads_worker_callbacks_and_stops_worker_on_clear() { let _guard = TEST_MUTEX.lock().unwrap(); diff --git a/go/nemo_relay/adaptive.go b/go/nemo_relay/adaptive.go index 249fb491d..eb7ce81f5 100644 --- a/go/nemo_relay/adaptive.go +++ b/go/nemo_relay/adaptive.go @@ -68,9 +68,8 @@ type AcgConfig struct { // AdaptiveComponentSpec wraps one adaptive config as a top-level plugin component. type AdaptiveComponentSpec struct { - Enabled bool `json:"enabled,omitempty"` - Config AdaptiveConfig `json:"config"` - enabledSet bool + Enabled bool `json:"enabled,omitempty"` + Config AdaptiveConfig `json:"config"` } // NewAdaptiveConfig returns a default adaptive config with version 1. @@ -142,50 +141,18 @@ func NewAcgConfig() AcgConfig { // NewAdaptiveComponentSpec wraps adaptive config as an enabled top-level component. func NewAdaptiveComponentSpec(config AdaptiveConfig) AdaptiveComponentSpec { return AdaptiveComponentSpec{ - Enabled: true, - Config: config, - enabledSet: true, + Enabled: true, + Config: config, } } -// SetEnabled explicitly sets whether this adaptive component is enabled. -func (spec *AdaptiveComponentSpec) SetEnabled(enabled bool) { - spec.Enabled = enabled - spec.enabledSet = true -} - -// WithEnabled returns a copy with an explicitly present enabled value. -func (spec AdaptiveComponentSpec) WithEnabled(enabled bool) AdaptiveComponentSpec { - spec.SetEnabled(enabled) - return spec -} - -// MarshalJSON preserves omitted-default and explicit-false enabled semantics. -func (spec AdaptiveComponentSpec) MarshalJSON() ([]byte, error) { - return marshalComponentWrapper(spec.Enabled, spec.enabledSet, spec.Config) -} - -// UnmarshalJSON records whether enabled was explicitly present. -func (spec *AdaptiveComponentSpec) UnmarshalJSON(payload []byte) error { - enabled, enabledSet, config, err := unmarshalComponentWrapper[AdaptiveConfig](payload) - if err != nil { - return err - } - *spec = AdaptiveComponentSpec{Enabled: enabled, Config: config, enabledSet: enabledSet} - return nil -} - // PluginComponent converts the adaptive component wrapper into the shared plugin shape. func (spec AdaptiveComponentSpec) PluginComponent() PluginComponentSpec { - component := PluginComponentSpec{ + return PluginComponentSpec{ Kind: AdaptivePluginKind, Enabled: spec.Enabled, Config: mustConfigMap(spec.Config), } - if spec.enabledSet || spec.Enabled { - component.SetEnabled(spec.Enabled) - } - return component } // AdaptiveComponent converts adaptive config directly into a shared plugin component. diff --git a/go/nemo_relay/model_pricing.go b/go/nemo_relay/model_pricing.go index d934119df..de18d116d 100644 --- a/go/nemo_relay/model_pricing.go +++ b/go/nemo_relay/model_pricing.go @@ -134,9 +134,8 @@ func (schedule PromptTokenThresholdRateSchedule) MarshalJSON() ([]byte, error) { // PricingComponentSpec wraps one model pricing config as a top-level plugin component. type PricingComponentSpec struct { - Enabled bool `json:"enabled,omitempty"` - Config PricingConfig `json:"config"` - enabledSet bool + Enabled bool `json:"enabled,omitempty"` + Config PricingConfig `json:"config"` } // NewPricingConfig returns an empty model pricing config. @@ -210,50 +209,18 @@ func NewPromptTokenThresholdRateSchedule(tiers ...TokenRateTier) PromptTokenThre // NewPricingComponentSpec wraps model pricing config as an enabled top-level component. func NewPricingComponentSpec(config PricingConfig) PricingComponentSpec { return PricingComponentSpec{ - Enabled: true, - Config: config, - enabledSet: true, + Enabled: true, + Config: config, } } -// SetEnabled explicitly sets whether this model pricing component is enabled. -func (spec *PricingComponentSpec) SetEnabled(enabled bool) { - spec.Enabled = enabled - spec.enabledSet = true -} - -// WithEnabled returns a copy with an explicitly present enabled value. -func (spec PricingComponentSpec) WithEnabled(enabled bool) PricingComponentSpec { - spec.SetEnabled(enabled) - return spec -} - -// MarshalJSON preserves omitted-default and explicit-false enabled semantics. -func (spec PricingComponentSpec) MarshalJSON() ([]byte, error) { - return marshalComponentWrapper(spec.Enabled, spec.enabledSet, spec.Config) -} - -// UnmarshalJSON records whether enabled was explicitly present. -func (spec *PricingComponentSpec) UnmarshalJSON(payload []byte) error { - enabled, enabledSet, config, err := unmarshalComponentWrapper[PricingConfig](payload) - if err != nil { - return err - } - *spec = PricingComponentSpec{Enabled: enabled, Config: config, enabledSet: enabledSet} - return nil -} - // PluginComponent converts the model pricing component wrapper into the shared plugin shape. func (spec PricingComponentSpec) PluginComponent() PluginComponentSpec { - component := PluginComponentSpec{ + return PluginComponentSpec{ Kind: PricingPluginKind, Enabled: spec.Enabled, Config: mustConfigMap(spec.Config), } - if spec.enabledSet || spec.Enabled { - component.SetEnabled(spec.Enabled) - } - return component } // PricingComponent converts model pricing config directly into a shared plugin component. diff --git a/go/nemo_relay/observability_plugin.go b/go/nemo_relay/observability_plugin.go index 55727f0cb..c79488919 100644 --- a/go/nemo_relay/observability_plugin.go +++ b/go/nemo_relay/observability_plugin.go @@ -180,9 +180,8 @@ func (config ObservabilityOtlpConfig) MarshalJSON() ([]byte, error) { // ObservabilityComponentSpec wraps one observability config as a top-level plugin component. type ObservabilityComponentSpec struct { - Enabled bool `json:"enabled,omitempty"` - Config ObservabilityConfig `json:"config"` - enabledSet bool + Enabled bool `json:"enabled,omitempty"` + Config ObservabilityConfig `json:"config"` } // NewObservabilityConfig returns a default observability config with version 1. @@ -232,50 +231,18 @@ func NewObservabilityOtlpConfig() ObservabilityOtlpConfig { // NewObservabilityComponentSpec wraps observability config as an enabled top-level component. func NewObservabilityComponentSpec(config ObservabilityConfig) ObservabilityComponentSpec { return ObservabilityComponentSpec{ - Enabled: true, - Config: config, - enabledSet: true, + Enabled: true, + Config: config, } } -// SetEnabled explicitly sets whether this observability component is enabled. -func (spec *ObservabilityComponentSpec) SetEnabled(enabled bool) { - spec.Enabled = enabled - spec.enabledSet = true -} - -// WithEnabled returns a copy with an explicitly present enabled value. -func (spec ObservabilityComponentSpec) WithEnabled(enabled bool) ObservabilityComponentSpec { - spec.SetEnabled(enabled) - return spec -} - -// MarshalJSON preserves omitted-default and explicit-false enabled semantics. -func (spec ObservabilityComponentSpec) MarshalJSON() ([]byte, error) { - return marshalComponentWrapper(spec.Enabled, spec.enabledSet, spec.Config) -} - -// UnmarshalJSON records whether enabled was explicitly present. -func (spec *ObservabilityComponentSpec) UnmarshalJSON(payload []byte) error { - enabled, enabledSet, config, err := unmarshalComponentWrapper[ObservabilityConfig](payload) - if err != nil { - return err - } - *spec = ObservabilityComponentSpec{Enabled: enabled, Config: config, enabledSet: enabledSet} - return nil -} - // PluginComponent converts the observability component wrapper into the shared plugin shape. func (spec ObservabilityComponentSpec) PluginComponent() PluginComponentSpec { - component := PluginComponentSpec{ + return PluginComponentSpec{ Kind: ObservabilityPluginKind, Enabled: spec.Enabled, Config: mustConfigMap(spec.Config), } - if spec.enabledSet || spec.Enabled { - component.SetEnabled(spec.Enabled) - } - return component } // ObservabilityComponent converts observability config directly into a shared plugin component. diff --git a/go/nemo_relay/pii_redaction.go b/go/nemo_relay/pii_redaction.go index cb807c8c4..871a83b60 100644 --- a/go/nemo_relay/pii_redaction.go +++ b/go/nemo_relay/pii_redaction.go @@ -45,9 +45,8 @@ type PiiRedactionConfig struct { // PiiRedactionComponentSpec wraps one PII redaction config as a top-level plugin component. type PiiRedactionComponentSpec struct { - Enabled bool `json:"enabled,omitempty"` - Config PiiRedactionConfig `json:"config"` - enabledSet bool + Enabled bool `json:"enabled,omitempty"` + Config PiiRedactionConfig `json:"config"` } // NewPiiRedactionConfig returns a default PII redaction config with version 1. @@ -82,50 +81,18 @@ func NewPiiRedactionLocalModelConfig() PiiRedactionLocalModelConfig { // NewPiiRedactionComponentSpec wraps PII redaction config as an enabled component. func NewPiiRedactionComponentSpec(config PiiRedactionConfig) PiiRedactionComponentSpec { return PiiRedactionComponentSpec{ - Enabled: true, - Config: config, - enabledSet: true, + Enabled: true, + Config: config, } } -// SetEnabled explicitly sets whether this PII redaction component is enabled. -func (spec *PiiRedactionComponentSpec) SetEnabled(enabled bool) { - spec.Enabled = enabled - spec.enabledSet = true -} - -// WithEnabled returns a copy with an explicitly present enabled value. -func (spec PiiRedactionComponentSpec) WithEnabled(enabled bool) PiiRedactionComponentSpec { - spec.SetEnabled(enabled) - return spec -} - -// MarshalJSON preserves omitted-default and explicit-false enabled semantics. -func (spec PiiRedactionComponentSpec) MarshalJSON() ([]byte, error) { - return marshalComponentWrapper(spec.Enabled, spec.enabledSet, spec.Config) -} - -// UnmarshalJSON records whether enabled was explicitly present. -func (spec *PiiRedactionComponentSpec) UnmarshalJSON(payload []byte) error { - enabled, enabledSet, config, err := unmarshalComponentWrapper[PiiRedactionConfig](payload) - if err != nil { - return err - } - *spec = PiiRedactionComponentSpec{Enabled: enabled, Config: config, enabledSet: enabledSet} - return nil -} - // PluginComponent converts the PII redaction wrapper into the shared plugin shape. func (spec PiiRedactionComponentSpec) PluginComponent() PluginComponentSpec { - component := PluginComponentSpec{ + return PluginComponentSpec{ Kind: PiiRedactionPluginKind, Enabled: spec.Enabled, Config: mustConfigMap(spec.Config), } - if spec.enabledSet || spec.Enabled { - component.SetEnabled(spec.Enabled) - } - return component } // PiiRedactionComponent converts PII redaction config directly into a shared plugin component. diff --git a/go/nemo_relay/plugin.go b/go/nemo_relay/plugin.go index 7ac23b659..6b0091568 100644 --- a/go/nemo_relay/plugin.go +++ b/go/nemo_relay/plugin.go @@ -71,8 +71,8 @@ extern char* goToolExecInterceptTrampoline(void*, const char*, NemoRelayToolExec import "C" import ( - "encoding/json" "errors" + "log" "runtime" "sync" "unsafe" @@ -220,22 +220,16 @@ type ConfigReport struct { // PluginComponentSpec is one top-level plugin component. type PluginComponentSpec struct { - Kind string `json:"kind"` - // Enabled uses Relay's default true when omitted. Use SetEnabled or - // WithEnabled to serialize an explicit false value. - Enabled bool `json:"-"` - Config map[string]any `json:"config,omitempty"` - enabledSet bool + Kind string `json:"kind"` + Enabled bool `json:"enabled,omitempty"` + Config map[string]any `json:"config,omitempty"` } // PluginConfig is the canonical plugin configuration document. type PluginConfig struct { - // Version uses Relay's default when omitted. Use SetVersion or WithVersion - // to serialize an explicit zero value. - Version uint32 `json:"-"` + Version uint32 `json:"version,omitempty"` Components []PluginComponentSpec `json:"components,omitempty"` Policy *ConfigPolicy `json:"policy,omitempty"` - versionSet bool } // DynamicPluginKind identifies the runtime lane used by a dynamic plugin. @@ -317,155 +311,50 @@ func NewPluginConfig() PluginConfig { return PluginConfig{ Version: 1, Components: []PluginComponentSpec{}, - versionSet: true, } } -// SetVersion sets the plugin config schema version and records it as explicitly -// present, including when version is zero. -func (config *PluginConfig) SetVersion(version uint32) { - config.Version = version - config.versionSet = true -} - -// WithVersion returns a copy with an explicitly present schema version. -func (config PluginConfig) WithVersion(version uint32) PluginConfig { - config.SetVersion(version) - return config -} - // NewPluginComponent returns an enabled top-level component with empty config. func NewPluginComponent(kind string) PluginComponentSpec { return PluginComponentSpec{ - Kind: kind, - Enabled: true, - Config: map[string]any{}, - enabledSet: true, + Kind: kind, + Enabled: true, + Config: map[string]any{}, } } -// SetEnabled sets whether this component is enabled and records the value as -// explicitly present, including when enabled is false. -func (component *PluginComponentSpec) SetEnabled(enabled bool) { - component.Enabled = enabled - component.enabledSet = true -} - -// WithEnabled returns a copy with an explicitly present enabled value. -func (component PluginComponentSpec) WithEnabled(enabled bool) PluginComponentSpec { - component.SetEnabled(enabled) - return component -} - -// MarshalJSON preserves Relay's default-on-omission semantics while allowing -// callers to explicitly serialize false through SetEnabled or WithEnabled. -func (component PluginComponentSpec) MarshalJSON() ([]byte, error) { +// marshalPluginActivationConfig serializes the activation-only wire shape. +// It keeps presence handling private so the established public Go config +// structs and their encoding method sets remain unchanged. Component enabled +// values are explicit on this wire because Relay defaults an omitted value to +// true, while the Go field's zero value is false. +func marshalPluginActivationConfig(config PluginConfig) ([]byte, error) { type componentJSON struct { Kind string `json:"kind"` - Enabled *bool `json:"enabled,omitempty"` + Enabled bool `json:"enabled"` Config map[string]any `json:"config,omitempty"` } - var enabled *bool - if component.enabledSet || component.Enabled { - enabled = &component.Enabled - } - return json.Marshal(componentJSON{ - Kind: component.Kind, - Enabled: enabled, - Config: component.Config, - }) -} - -// UnmarshalJSON records whether enabled was explicitly present so false -// survives subsequent marshaling instead of becoming Relay's default true. -func (component *PluginComponentSpec) UnmarshalJSON(payload []byte) error { - decoded := struct { - Kind string `json:"kind"` - Enabled *bool `json:"enabled"` - Config map[string]any `json:"config"` - }{} - if err := json.Unmarshal(payload, &decoded); err != nil { - return err - } - *component = PluginComponentSpec{ - Kind: decoded.Kind, - Config: decoded.Config, - } - if decoded.Enabled != nil { - component.SetEnabled(*decoded.Enabled) - } - return nil -} - -// MarshalJSON preserves Relay's default-on-omission semantics while allowing -// callers to explicitly serialize zero through SetVersion or WithVersion. -func (config PluginConfig) MarshalJSON() ([]byte, error) { type configJSON struct { - Version *uint32 `json:"version,omitempty"` - Components []PluginComponentSpec `json:"components,omitempty"` - Policy *ConfigPolicy `json:"policy,omitempty"` - } - var version *uint32 - if config.versionSet || config.Version != 0 { - version = &config.Version - } - return json.Marshal(configJSON{ - Version: version, - Components: config.Components, - Policy: config.Policy, - }) -} - -// UnmarshalJSON records whether version was explicitly present so zero -// survives subsequent marshaling instead of becoming Relay's default version. -func (config *PluginConfig) UnmarshalJSON(payload []byte) error { - decoded := struct { - Version *uint32 `json:"version"` - Components []PluginComponentSpec `json:"components"` - Policy *ConfigPolicy `json:"policy"` - }{} - if err := json.Unmarshal(payload, &decoded); err != nil { - return err - } - *config = PluginConfig{ - Components: decoded.Components, - Policy: decoded.Policy, + Version uint32 `json:"version,omitempty"` + Components []componentJSON `json:"components,omitempty"` + Policy *ConfigPolicy `json:"policy,omitempty"` } - if decoded.Version != nil { - config.SetVersion(*decoded.Version) - } - return nil -} -func marshalComponentWrapper[T any](enabled bool, enabledSet bool, config T) ([]byte, error) { - var explicitEnabled *bool - if enabledSet || enabled { - explicitEnabled = &enabled + components := make([]componentJSON, len(config.Components)) + for i, component := range config.Components { + components[i] = componentJSON{ + Kind: component.Kind, + Enabled: component.Enabled, + Config: component.Config, + } } - return json.Marshal(struct { - Enabled *bool `json:"enabled,omitempty"` - Config T `json:"config"` - }{ - Enabled: explicitEnabled, - Config: config, + return jsonMarshal(configJSON{ + Version: config.Version, + Components: components, + Policy: config.Policy, }) } -func unmarshalComponentWrapper[T any](payload []byte) (bool, bool, T, error) { - var decoded struct { - Enabled *bool `json:"enabled"` - Config T `json:"config"` - } - if err := json.Unmarshal(payload, &decoded); err != nil { - var zero T - return false, false, zero, err - } - if decoded.Enabled == nil { - return false, false, decoded.Config, nil - } - return *decoded.Enabled, true, decoded.Config, nil -} - // ValidatePluginConfig validates a plugin config without changing runtime state. // // It returns the validation report or an error if the config could not be @@ -519,7 +408,7 @@ func ActivateDynamicPlugins( "dynamic plugin activation requires at least one dynamic plugin; use InitializePlugins for a static-only configuration", ) } - configPayload, err := jsonMarshal(config) + configPayload, err := marshalPluginActivationConfig(config) if err != nil { return nil, ConfigReport{}, err } @@ -546,12 +435,20 @@ func ActivateDynamicPlugins( func newPluginActivation(ptr unsafe.Pointer) *PluginActivation { state := &pluginActivationState{ptr: ptr} - runtime.SetFinalizer(state, func(state *pluginActivationState) { - _ = state.close() - }) + runtime.SetFinalizer(state, finalizePluginActivation) return &PluginActivation{state: state} } +var reportPluginActivationCleanupError = func(err error) { + log.Printf("nemo_relay: dynamic plugin activation cleanup failed during finalization: %v", err) +} + +func finalizePluginActivation(state *pluginActivationState) { + if err := state.close(); err != nil { + reportPluginActivationCleanupError(err) + } +} + // Close removes callbacks and subscribers before unloading plugin libraries // and workers. It is safe to call Close repeatedly or on a nil activation. // Copies and repeated calls observe the same first teardown result. diff --git a/go/nemo_relay/plugin_activation_test.go b/go/nemo_relay/plugin_activation_test.go index 2ac1a235b..f23b36167 100644 --- a/go/nemo_relay/plugin_activation_test.go +++ b/go/nemo_relay/plugin_activation_test.go @@ -11,6 +11,7 @@ import ( "os/exec" "path/filepath" "runtime" + "strconv" "strings" "sync" "testing" @@ -32,10 +33,12 @@ func withPluginActivationStubs(t *testing.T) { originalActivate := activateDynamicPluginsJSON originalClear := clearPluginActivation originalFree := freePluginActivation + originalReporter := reportPluginActivationCleanupError t.Cleanup(func() { activateDynamicPluginsJSON = originalActivate clearPluginActivation = originalClear freePluginActivation = originalFree + reportPluginActivationCleanupError = originalReporter }) } @@ -116,8 +119,26 @@ func TestActivateDynamicPluginsSerializesSpecsAndOwnsCleanup(t *testing.T) { runtime.KeepAlive(token) } -func TestActivateDynamicPluginsPreservesLegacyOmittedDefaults(t *testing.T) { - withPluginActivationStubs(t) +func TestPluginConfigPublicJSONShapeRemainsCompatible(t *testing.T) { + type envelope struct { + PluginConfig + Name string `json:"name"` + } + + payload, err := json.Marshal(envelope{ + PluginConfig: PluginConfig{Version: 1}, + Name: "fixture", + }) + if err != nil { + t.Fatalf("marshal embedded plugin config: %v", err) + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(payload, &fields); err != nil { + t.Fatalf("unmarshal embedded plugin config: %v", err) + } + if string(fields["name"]) != `"fixture"` || string(fields["version"]) != "1" { + t.Fatalf("embedded plugin config lost envelope fields: %s", payload) + } emptyPayload, err := json.Marshal(PluginConfig{}) if err != nil { @@ -126,42 +147,24 @@ func TestActivateDynamicPluginsPreservesLegacyOmittedDefaults(t *testing.T) { if string(emptyPayload) != "{}" { t.Fatalf("empty plugin config JSON = %s, want {}", emptyPayload) } +} - token := new(byte) - activateDynamicPluginsJSON = func(configJSON, specsJSON string) (unsafe.Pointer, string, error) { - const wantConfig = `{"components":[{"kind":"fixture.default"}]}` - if configJSON != wantConfig { - t.Fatalf("config JSON = %s, want %s", configJSON, wantConfig) - } - const wantSpecs = `[{"plugin_id":"fixture.native","kind":"rust_dynamic","manifest_ref":"/tmp/relay-plugin.toml"}]` - if specsJSON != wantSpecs { - t.Fatalf("dynamic plugin specs JSON = %s, want %s", specsJSON, wantSpecs) - } - return unsafe.Pointer(token), `{"diagnostics":[]}`, nil - } - clearPluginActivation = func(unsafe.Pointer) error { return nil } - freePluginActivation = func(unsafe.Pointer) {} +func TestActivateDynamicPluginsUsesPrivateConfigWireShape(t *testing.T) { + withPluginActivationStubs(t) - activation, _, err := ActivateDynamicPlugins(PluginConfig{ - Components: []PluginComponentSpec{ - {Kind: "fixture.default"}, - }, - }, fixtureDynamicPluginSpecs()) + config := PluginConfig{Components: []PluginComponentSpec{{Kind: "fixture.disabled"}}} + publicPayload, err := json.Marshal(config) if err != nil { - t.Fatalf("ActivateDynamicPlugins() error = %v", err) + t.Fatalf("marshal public plugin config: %v", err) } - if err := activation.Close(); err != nil { - t.Fatalf("Close() error = %v", err) + const wantPublic = `{"components":[{"kind":"fixture.disabled"}]}` + if string(publicPayload) != wantPublic { + t.Fatalf("public plugin config JSON = %s, want %s", publicPayload, wantPublic) } - runtime.KeepAlive(token) -} - -func TestActivateDynamicPluginsPreservesExplicitZeroAndFalse(t *testing.T) { - withPluginActivationStubs(t) token := new(byte) activateDynamicPluginsJSON = func(configJSON, specsJSON string) (unsafe.Pointer, string, error) { - const wantConfig = `{"version":0,"components":[{"kind":"fixture.disabled","enabled":false}]}` + const wantConfig = `{"components":[{"kind":"fixture.disabled","enabled":false}]}` if configJSON != wantConfig { t.Fatalf("config JSON = %s, want %s", configJSON, wantConfig) } @@ -169,25 +172,11 @@ func TestActivateDynamicPluginsPreservesExplicitZeroAndFalse(t *testing.T) { if specsJSON != wantSpecs { t.Fatalf("dynamic plugin specs JSON = %s, want %s", specsJSON, wantSpecs) } - var roundTrip PluginConfig - if err := json.Unmarshal([]byte(configJSON), &roundTrip); err != nil { - t.Fatalf("unmarshal explicit plugin config: %v", err) - } - roundTripPayload, err := json.Marshal(roundTrip) - if err != nil { - t.Fatalf("remarshal explicit plugin config: %v", err) - } - if string(roundTripPayload) != wantConfig { - t.Fatalf("round-trip config JSON = %s, want %s", roundTripPayload, wantConfig) - } return unsafe.Pointer(token), `{"diagnostics":[]}`, nil } clearPluginActivation = func(unsafe.Pointer) error { return nil } freePluginActivation = func(unsafe.Pointer) {} - component := (PluginComponentSpec{Kind: "fixture.disabled"}).WithEnabled(false) - config := PluginConfig{Components: []PluginComponentSpec{component}} - config.SetVersion(0) activation, _, err := ActivateDynamicPlugins(config, fixtureDynamicPluginSpecs()) if err != nil { t.Fatalf("ActivateDynamicPlugins() error = %v", err) @@ -198,105 +187,18 @@ func TestActivateDynamicPluginsPreservesExplicitZeroAndFalse(t *testing.T) { runtime.KeepAlive(token) } -func TestPluginConfigUnmarshalOmissionsResetPresenceAndValues(t *testing.T) { - staleComponent := NewPluginComponent("fixture.stale") - staleComponent.Config = map[string]any{"stale": true} - config := NewPluginConfig() - config.Version = 7 - config.Components = []PluginComponentSpec{staleComponent} - config.Policy = &ConfigPolicy{UnknownField: UnsupportedBehaviorError} - - const payload = `{"components":[{"kind":"fixture.fresh"}]}` - if err := json.Unmarshal([]byte(payload), &config); err != nil { - t.Fatalf("unmarshal config with omitted defaults: %v", err) - } - if config.Version != 0 || config.versionSet { - t.Fatalf("omitted version retained stale state: %#v", config) - } - if config.Policy != nil { - t.Fatalf("omitted policy retained stale state: %#v", config.Policy) - } - if len(config.Components) != 1 { - t.Fatalf("components = %#v, want one", config.Components) - } - component := config.Components[0] - if component.Kind != "fixture.fresh" || component.Enabled || component.enabledSet || component.Config != nil { - t.Fatalf("omitted enabled/config retained stale state: %#v", component) - } - - roundTrip, err := json.Marshal(config) - if err != nil { - t.Fatalf("remarshal config with omitted defaults: %v", err) - } - if string(roundTrip) != payload { - t.Fatalf("round-trip config JSON = %s, want %s", roundTrip, payload) - } -} - -func TestComponentWrapperEnabledPresenceSurvivesConversion(t *testing.T) { - adaptive := NewAdaptiveComponentSpec(NewAdaptiveConfig()) - adaptive.Enabled = false - explicitDisabled := []PluginComponentSpec{ - adaptive.PluginComponent(), - NewObservabilityComponentSpec(NewObservabilityConfig()).WithEnabled(false).PluginComponent(), - NewPricingComponentSpec(NewPricingConfig()).WithEnabled(false).PluginComponent(), - NewPiiRedactionComponentSpec(NewPiiRedactionConfig()).WithEnabled(false).PluginComponent(), - } - legacyDefaults := []PluginComponentSpec{ +func TestComponentWrappersPreserveDisabledValuesDuringConversion(t *testing.T) { + disabled := []PluginComponentSpec{ (AdaptiveComponentSpec{Config: NewAdaptiveConfig()}).PluginComponent(), (ObservabilityComponentSpec{Config: NewObservabilityConfig()}).PluginComponent(), (PricingComponentSpec{Config: NewPricingConfig()}).PluginComponent(), (PiiRedactionComponentSpec{Config: NewPiiRedactionConfig()}).PluginComponent(), } - - assertEnabledPresence := func(component PluginComponentSpec, wantPresent bool) { - t.Helper() - payload, err := json.Marshal(component) - if err != nil { - t.Fatalf("marshal %s component: %v", component.Kind, err) - } - var fields map[string]json.RawMessage - if err := json.Unmarshal(payload, &fields); err != nil { - t.Fatalf("unmarshal %s component JSON: %v", component.Kind, err) - } - enabled, present := fields["enabled"] - if present != wantPresent { - t.Fatalf("%s enabled presence = %t, want %t: %s", component.Kind, present, wantPresent, payload) + for _, component := range disabled { + if component.Enabled { + t.Fatalf("%s component became enabled during conversion", component.Kind) } - if wantPresent && string(enabled) != "false" { - t.Fatalf("%s enabled JSON = %s, want false", component.Kind, enabled) - } - } - - for _, component := range explicitDisabled { - assertEnabledPresence(component, true) - } - for _, component := range legacyDefaults { - assertEnabledPresence(component, false) - } - - wrapperPayload, err := json.Marshal( - NewAdaptiveComponentSpec(NewAdaptiveConfig()).WithEnabled(false), - ) - if err != nil { - t.Fatalf("marshal explicit adaptive wrapper: %v", err) - } - decodedWrapper := NewAdaptiveComponentSpec(NewAdaptiveConfig()) - if err := json.Unmarshal(wrapperPayload, &decodedWrapper); err != nil { - t.Fatalf("unmarshal explicit adaptive wrapper: %v", err) } - if decodedWrapper.Enabled || !decodedWrapper.enabledSet { - t.Fatalf("explicit wrapper enabled state was not preserved: %#v", decodedWrapper) - } - assertEnabledPresence(decodedWrapper.PluginComponent(), true) - - if err := json.Unmarshal([]byte(`{"config":{}}`), &decodedWrapper); err != nil { - t.Fatalf("unmarshal adaptive wrapper with omitted enabled: %v", err) - } - if decodedWrapper.Enabled || decodedWrapper.enabledSet { - t.Fatalf("omitted wrapper enabled retained stale state: %#v", decodedWrapper) - } - assertEnabledPresence(decodedWrapper.PluginComponent(), false) } func TestActivateDynamicPluginsRejectsEmptySpecsWithoutCallingCgo(t *testing.T) { @@ -376,6 +278,23 @@ func TestPluginActivationCloseFreesAfterClearFailure(t *testing.T) { runtime.KeepAlive(token) } +func TestPluginActivationFinalizerReportsClearFailure(t *testing.T) { + withPluginActivationStubs(t) + + token := new(byte) + wantErr := errors.New("teardown failed") + clearPluginActivation = func(unsafe.Pointer) error { return wantErr } + freePluginActivation = func(unsafe.Pointer) {} + var reported error + reportPluginActivationCleanupError = func(err error) { reported = err } + + finalizePluginActivation(&pluginActivationState{ptr: unsafe.Pointer(token)}) + if !errors.Is(reported, wantErr) { + t.Fatalf("reported finalizer error = %v, want %v", reported, wantErr) + } + runtime.KeepAlive(token) +} + func TestPluginActivationCopiesShareCloseStateAndError(t *testing.T) { withPluginActivationStubs(t) @@ -869,7 +788,7 @@ id = "fixture_native" kind = "rust_dynamic" [compat] -relay = "=0.6.0" +relay = "=%s" native_api = "1" [defaults] @@ -881,7 +800,7 @@ items = ["plugin_native"] [load] library = %q symbol = "nemo_relay_fixture_native_plugin" -`, library) +`, relayWorkspaceVersion(t), library) if err := os.WriteFile(manifest, []byte(contents), 0o600); err != nil { t.Fatalf("write native plugin manifest: %v", err) } @@ -898,7 +817,7 @@ id = "fixture_worker" kind = "worker" [compat] -relay = "=0.6.0" +relay = "=%s" worker_protocol = "grpc-v1" [defaults] @@ -910,13 +829,48 @@ items = ["plugin_worker"] [load] runtime = "rust" entrypoint = %q -`, executable) +`, relayWorkspaceVersion(t), executable) if err := os.WriteFile(manifest, []byte(contents), 0o600); err != nil { t.Fatalf("write worker plugin manifest: %v", err) } return manifest } +func relayWorkspaceVersion(t *testing.T) string { + t.Helper() + repoRoot, err := filepath.Abs(filepath.Join("..", "..")) + if err != nil { + t.Fatalf("resolve repository root: %v", err) + } + payload, err := os.ReadFile(filepath.Join(repoRoot, "Cargo.toml")) + if err != nil { + t.Fatalf("read workspace Cargo.toml: %v", err) + } + section := string(payload) + const header = "[workspace.package]" + start := strings.Index(section, header) + if start < 0 { + t.Fatal("workspace Cargo.toml has no [workspace.package] section") + } + section = section[start+len(header):] + if end := strings.Index(section, "\n["); end >= 0 { + section = section[:end] + } + for _, line := range strings.Split(section, "\n") { + line = strings.TrimSpace(line) + if !strings.HasPrefix(line, "version =") { + continue + } + version, err := strconv.Unquote(strings.TrimSpace(strings.TrimPrefix(line, "version ="))) + if err != nil { + t.Fatalf("parse workspace package version: %v", err) + } + return version + } + t.Fatal("workspace package version not found") + return "" +} + func goNativeLibraryName() string { switch runtime.GOOS { case "windows": From af30646c2f3ee4fc0bf0a2c524186a7b33327f6a Mon Sep 17 00:00:00 2001 From: Bryan Bednarski Date: Mon, 13 Jul 2026 23:42:43 -0600 Subject: [PATCH 06/11] fix(ffi): layer discovered plugin configuration Signed-off-by: Bryan Bednarski --- crates/ffi/nemo_relay.h | 9 +- crates/ffi/src/api/plugin.rs | 20 +- .../integration/plugin_activation_tests.rs | 224 +++++++++++++++++- go/nemo_relay/plugin.go | 13 +- go/nemo_relay/plugin_activation_test.go | 71 +++++- 5 files changed, 314 insertions(+), 23 deletions(-) diff --git a/crates/ffi/nemo_relay.h b/crates/ffi/nemo_relay.h index f39ddce74..00535e621 100644 --- a/crates/ffi/nemo_relay.h +++ b/crates/ffi/nemo_relay.h @@ -1422,13 +1422,16 @@ NemoRelayStatus nemo_relay_openinference_subscriber_shutdown(const struct FfiOpe * **Experimental:** this API needs a production consumer before its lifecycle * contract is considered stable. * - * `config_json` is the base `PluginConfig` document. Its static components - * initialize before components appended by the dynamic plugins. + * Relay discovers `plugins.toml` once during this startup call and layers + * `config_json` over the discovered configuration. Explicit values take + * precedence where both sources configure the same setting. Static components + * from the resolved configuration initialize before components appended by + * the dynamic plugins. Configuration files are not watched or reloaded. * `dynamic_plugins_json` must contain at least one explicit dynamic-plugin * activation specification; use `nemo_relay_initialize_plugins` for a * static-only configuration. * - * The base configuration uses this JSON shape: + * The explicit configuration uses this JSON shape: * * ```text * {"version":1,"components":[{"kind":"static.kind","enabled":true,"config":{}}]} diff --git a/crates/ffi/src/api/plugin.rs b/crates/ffi/src/api/plugin.rs index e04e1c0ea..f50b63c18 100644 --- a/crates/ffi/src/api/plugin.rs +++ b/crates/ffi/src/api/plugin.rs @@ -167,13 +167,16 @@ fn lock_plugin_activation( /// **Experimental:** this API needs a production consumer before its lifecycle /// contract is considered stable. /// -/// `config_json` is the base `PluginConfig` document. Its static components -/// initialize before components appended by the dynamic plugins. +/// Relay discovers `plugins.toml` once during this startup call and layers +/// `config_json` over the discovered configuration. Explicit values take +/// precedence where both sources configure the same setting. Static components +/// from the resolved configuration initialize before components appended by +/// the dynamic plugins. Configuration files are not watched or reloaded. /// `dynamic_plugins_json` must contain at least one explicit dynamic-plugin /// activation specification; use `nemo_relay_initialize_plugins` for a /// static-only configuration. /// -/// The base configuration uses this JSON shape: +/// The explicit configuration uses this JSON shape: /// /// ```text /// {"version":1,"components":[{"kind":"static.kind","enabled":true,"config":{}}]} @@ -242,11 +245,12 @@ pub unsafe extern "C" fn nemo_relay_activate_dynamic_plugins( Ok(dynamic_plugins) => dynamic_plugins, Err(status) => return status, }; - let (activation, report) = - match tokio_runtime().block_on(PluginHostActivation::activate(config, dynamic_plugins)) { - Ok(result) => result, - Err(error) => return status_from_plugin_error(&error), - }; + let (activation, report) = match tokio_runtime().block_on( + PluginHostActivation::activate_with_discovered_config(config, dynamic_plugins), + ) { + Ok(result) => result, + Err(error) => return status_from_plugin_error(&error), + }; let report_json = match serde_json::to_value(report) { Ok(value) => value, Err(error) => { diff --git a/crates/ffi/tests/integration/plugin_activation_tests.rs b/crates/ffi/tests/integration/plugin_activation_tests.rs index 08c33626b..40f12deca 100644 --- a/crates/ffi/tests/integration/plugin_activation_tests.rs +++ b/crates/ffi/tests/integration/plugin_activation_tests.rs @@ -5,11 +5,233 @@ use super::*; use std::path::{Path, PathBuf}; use std::process::Command; use std::ptr; -use std::sync::OnceLock; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::{Mutex, OnceLock}; use nemo_relay_ffi::types::{FfiPluginActivation, nemo_relay_plugin_activation_free}; use tempfile::TempDir; +const DISCOVERY_CHILD_ENV: &str = "NEMO_RELAY_FFI_DISCOVERY_CHILD"; +const DISCOVERED_STATIC_PLUGIN_KIND: &str = "ffi_discovered_static"; +static DISCOVERED_STATIC_REGISTRATIONS: AtomicUsize = AtomicUsize::new(0); +static DISCOVERED_STATIC_CALLBACKS: AtomicUsize = AtomicUsize::new(0); +static DISCOVERED_STATIC_CONFIG: Mutex> = Mutex::new(None); + +struct PluginDiscoveryTestEnv { + previous_cwd: PathBuf, + previous_xdg_config_home: Option, +} + +impl PluginDiscoveryTestEnv { + fn enter(cwd: &Path, xdg_config_home: &Path) -> Self { + let guard = Self { + previous_cwd: std::env::current_dir().expect("current directory"), + previous_xdg_config_home: std::env::var_os("XDG_CONFIG_HOME"), + }; + std::env::set_current_dir(cwd).expect("set project directory"); + // SAFETY: this runs in a dedicated child test process and Drop restores + // the environment before that process exits. + unsafe { std::env::set_var("XDG_CONFIG_HOME", xdg_config_home) }; + guard + } +} + +impl Drop for PluginDiscoveryTestEnv { + fn drop(&mut self) { + let _ = std::env::set_current_dir(&self.previous_cwd); + // SAFETY: see PluginDiscoveryTestEnv::enter. + unsafe { + match &self.previous_xdg_config_home { + Some(value) => std::env::set_var("XDG_CONFIG_HOME", value), + None => std::env::remove_var("XDG_CONFIG_HOME"), + } + } + } +} + +#[test] +fn ffi_activation_layers_discovered_static_and_explicit_dynamic_plugins() { + if std::env::var_os(DISCOVERY_CHILD_ENV).is_some() { + run_discovered_config_activation_test(); + return; + } + + let output = Command::new(std::env::current_exe().expect("current test executable")) + .arg("--exact") + .arg( + "plugin_activation_tests::ffi_activation_layers_discovered_static_and_explicit_dynamic_plugins", + ) + .arg("--nocapture") + .env(DISCOVERY_CHILD_ENV, "1") + .output() + .expect("discovery child test should start"); + assert!( + output.status.success(), + "discovery child test failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} + +fn run_discovered_config_activation_test() { + let _ = nemo_relay_clear_plugin_configuration(); + DISCOVERED_STATIC_REGISTRATIONS.store(0, Ordering::SeqCst); + DISCOVERED_STATIC_CALLBACKS.store(0, Ordering::SeqCst); + *DISCOVERED_STATIC_CONFIG.lock().unwrap() = None; + + let environment = TempDir::new().expect("plugin discovery environment"); + let project_config_dir = environment.path().join(".nemo-relay"); + let xdg_config_home = environment.path().join("xdg"); + std::fs::create_dir_all(&project_config_dir).expect("project config directory"); + std::fs::create_dir_all(&xdg_config_home).expect("isolated user config directory"); + let plugins_toml = project_config_dir.join("plugins.toml"); + std::fs::write(&plugins_toml, "invalid = [").expect("write invalid plugin config"); + let _environment = PluginDiscoveryTestEnv::enter(environment.path(), &xdg_config_home); + + // Empty specifications fail before discovery or ownership. The malformed + // file would otherwise produce a TOML error, and the successful activation + // below proves this attempt did not retain the process-wide host claim. + let config = cstring(r#"{"version":1,"components":[]}"#); + let empty_specs = cstring("[]"); + let mut empty_activation = ptr::null_mut(); + let mut empty_report = ptr::null_mut(); + assert_eq!( + unsafe { + api::nemo_relay_activate_dynamic_plugins( + config.as_ptr(), + empty_specs.as_ptr(), + &mut empty_activation, + &mut empty_report, + ) + }, + NemoRelayStatus::InvalidArg + ); + assert!(empty_activation.is_null()); + assert!(empty_report.is_null()); + assert!( + unsafe { read_last_error() } + .unwrap_or_default() + .contains("at least one dynamic plugin") + ); + + std::fs::write( + &plugins_toml, + format!( + r#"version = 999 + +[[components]] +kind = {DISCOVERED_STATIC_PLUGIN_KIND:?} +enabled = true + +[components.config] +source = "project-file" +"# + ), + ) + .expect("write project plugin config"); + + let plugin_kind = cstring(DISCOVERED_STATIC_PLUGIN_KIND); + assert_eq!( + unsafe { + api::nemo_relay_register_plugin( + plugin_kind.as_ptr(), + None, + discovered_static_register, + ptr::null_mut(), + None, + ) + }, + NemoRelayStatus::Ok + ); + + let manifest_dir = TempDir::new().expect("native manifest tempdir"); + let manifest = write_native_manifest(manifest_dir.path(), build_native_fixture()); + let (mut activation, report) = activate_plugins(json!([{ + "plugin_id": "fixture_native", + "kind": "rust_dynamic", + "manifest_ref": manifest, + "config": {} + }])); + + // The explicit version 1 must override the discovered version 999. The + // file-only component and its config must still survive the merge. + assert_eq!(report["diagnostics"], json!([])); + assert_eq!(DISCOVERED_STATIC_REGISTRATIONS.load(Ordering::SeqCst), 1); + assert_eq!( + DISCOVERED_STATIC_CONFIG.lock().unwrap().as_ref(), + Some(&json!({"source": "project-file"})) + ); + assert!(plugin_kinds().iter().any(|kind| kind == "fixture_native")); + + // Mutating the file after startup has no effect: discovery is one-shot. + std::fs::write(&plugins_toml, "invalid = [").expect("mutate plugin config after startup"); + let intercepted = tool_request_intercepts("ffi-layered-tool", json!({"input": true})); + assert_eq!(intercepted["file_static"], true); + assert_eq!(intercepted["static_saw_dynamic"], false); + assert_eq!(intercepted["native_plugin"], true); + assert_eq!(DISCOVERED_STATIC_CALLBACKS.load(Ordering::SeqCst), 1); + + unsafe { + assert_eq!( + api::nemo_relay_plugin_activation_clear(activation), + NemoRelayStatus::Ok + ); + nemo_relay_plugin_activation_free(&mut activation); + } + assert!(!plugin_kinds().iter().any(|kind| kind == "fixture_native")); + assert_eq!( + tool_request_intercepts("ffi-layered-tool", json!({"input": true})), + json!({"input": true}) + ); + assert_eq!(DISCOVERED_STATIC_CALLBACKS.load(Ordering::SeqCst), 1); + assert_eq!( + unsafe { api::nemo_relay_deregister_plugin(plugin_kind.as_ptr()) }, + NemoRelayStatus::Ok + ); +} + +unsafe extern "C" fn discovered_static_register( + _user_data: *mut libc::c_void, + plugin_config_json: *const c_char, + ctx: *mut FfiPluginContext, +) -> NemoRelayStatus { + let config = unsafe { CStr::from_ptr(plugin_config_json) } + .to_str() + .ok() + .and_then(|value| serde_json::from_str(value).ok()); + *DISCOVERED_STATIC_CONFIG.lock().unwrap() = config; + DISCOVERED_STATIC_REGISTRATIONS.fetch_add(1, Ordering::SeqCst); + let name = cstring("project_file_intercept"); + unsafe { + api::nemo_relay_plugin_context_register_tool_request_intercept( + ctx, + name.as_ptr(), + -1, + false, + discovered_static_tool_request, + ptr::null_mut(), + None, + ) + } +} + +unsafe extern "C" fn discovered_static_tool_request( + _user_data: *mut libc::c_void, + _name: *const c_char, + args_json: *const c_char, +) -> *mut c_char { + DISCOVERED_STATIC_CALLBACKS.fetch_add(1, Ordering::SeqCst); + let mut args: Json = serde_json::from_str( + unsafe { CStr::from_ptr(args_json) } + .to_str() + .unwrap_or("null"), + ) + .unwrap_or_else(|_| json!({})); + args["static_saw_dynamic"] = json!(args.get("native_plugin").is_some()); + args["file_static"] = json!(true); + CString::new(args.to_string()).unwrap().into_raw() +} + #[test] fn ffi_activation_loads_native_callbacks_and_removes_them_before_free() { let _guard = TEST_MUTEX.lock().unwrap(); diff --git a/go/nemo_relay/plugin.go b/go/nemo_relay/plugin.go index 6b0091568..83046c332 100644 --- a/go/nemo_relay/plugin.go +++ b/go/nemo_relay/plugin.go @@ -388,11 +388,14 @@ func InitializePlugins(config PluginConfig) (ConfigReport, error) { return report, nil } -// ActivateDynamicPlugins loads explicit dynamic plugins and activates them -// together with the supplied base plugin configuration as one transaction. -// Static components in config are initialized before components appended by -// dynamic plugins. At least one dynamic plugin specification is required; use -// InitializePlugins for a static-only configuration. +// ActivateDynamicPlugins discovers plugins.toml once during startup, layers the +// supplied config over the discovered configuration, and activates explicit +// dynamic plugins in the same transaction. Explicit values take precedence +// where both sources configure the same setting. Static components in the +// resolved configuration are initialized before components appended by dynamic +// plugins. Configuration files are not watched or reloaded. At least one +// dynamic plugin specification is required; use InitializePlugins for a +// static-only configuration. // // The caller must retain the returned activation for as long as plugin // callbacks may run and call Close during orderly shutdown. diff --git a/go/nemo_relay/plugin_activation_test.go b/go/nemo_relay/plugin_activation_test.go index f23b36167..d6baa02c1 100644 --- a/go/nemo_relay/plugin_activation_test.go +++ b/go/nemo_relay/plugin_activation_test.go @@ -14,6 +14,7 @@ import ( "strconv" "strings" "sync" + "sync/atomic" "testing" "time" "unsafe" @@ -491,18 +492,60 @@ func TestActivateDynamicPluginsLoadsNativePluginThroughCgo(t *testing.T) { if err := ClearPluginConfiguration(); err != nil { t.Fatalf("ClearPluginConfiguration() error = %v", err) } + library := goNativePluginFixture(t) + manifest := writeGoNativePluginManifest(t, library) + projectDir := t.TempDir() + projectConfigDir := filepath.Join(projectDir, ".nemo-relay") + if err := os.MkdirAll(projectConfigDir, 0o700); err != nil { + t.Fatalf("MkdirAll(project config) error = %v", err) + } + pluginsTOML := filepath.Join(projectConfigDir, "plugins.toml") const staticKind = "go.fixture.static_base" + fileConfig := fmt.Sprintf(`version = 999 + +[[components]] +kind = %q +enabled = true + +[components.config] +source = "project-file" +`, staticKind) + if err := os.WriteFile(pluginsTOML, []byte(fileConfig), 0o600); err != nil { + t.Fatalf("WriteFile(plugins.toml) error = %v", err) + } + previousCWD, err := os.Getwd() + if err != nil { + t.Fatalf("Getwd() error = %v", err) + } + if err := os.Chdir(projectDir); err != nil { + t.Fatalf("Chdir(project) error = %v", err) + } + t.Cleanup(func() { + if err := os.Chdir(previousCWD); err != nil { + t.Errorf("restore working directory error = %v", err) + } + }) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(projectDir, "xdg")) + + var staticRegistrations atomic.Int32 + var staticCallbacks atomic.Int32 if err := RegisterPlugin(staticKind, PluginFuncs{ - RegisterFunc: func(_ map[string]any, ctx *PluginContext) error { + RegisterFunc: func(config map[string]any, ctx *PluginContext) error { + if config["source"] != "project-file" { + return fmt.Errorf("static plugin config = %#v, want project-file source", config) + } + staticRegistrations.Add(1) return ctx.RegisterToolRequestIntercept( "go_static_base", -1, false, func(_ string, args json.RawMessage) json.RawMessage { + staticCallbacks.Add(1) var payload map[string]any if err := json.Unmarshal(args, &payload); err != nil { return args } + payload["static_saw_dynamic"] = payload["native_plugin"] == true payload["go_static_base"] = true out, _ := json.Marshal(payload) return out @@ -518,11 +561,7 @@ func TestActivateDynamicPluginsLoadsNativePluginThroughCgo(t *testing.T) { } }() - library := goNativePluginFixture(t) - manifest := writeGoNativePluginManifest(t, library) - config := NewPluginConfig() - config.Components = append(config.Components, NewPluginComponent(staticKind)) - activation, report, err := ActivateDynamicPlugins(config, []DynamicPluginActivationSpec{{ + activation, report, err := ActivateDynamicPlugins(NewPluginConfig(), []DynamicPluginActivationSpec{{ PluginID: "fixture_native", Kind: DynamicPluginKindRustDynamic, ManifestRef: manifest, @@ -539,6 +578,14 @@ func TestActivateDynamicPluginsLoadsNativePluginThroughCgo(t *testing.T) { if len(report.Diagnostics) != 0 { t.Fatalf("activation diagnostics = %#v, want none", report.Diagnostics) } + if got := staticRegistrations.Load(); got != 1 { + t.Fatalf("static registrations = %d, want 1", got) + } + // The activation has already resolved its configuration; subsequent file + // mutations are neither watched nor reloaded. + if err := os.WriteFile(pluginsTOML, []byte("invalid = ["), 0o600); err != nil { + t.Fatalf("mutate plugins.toml error = %v", err) + } transformed, err := ToolRequestIntercepts("go-native-tool", json.RawMessage(`{"input":true}`)) if err != nil { @@ -554,6 +601,12 @@ func TestActivateDynamicPluginsLoadsNativePluginThroughCgo(t *testing.T) { if transformedObject["go_static_base"] != true { t.Fatalf("transformed tool args = %s, want static base marker", transformed) } + if transformedObject["static_saw_dynamic"] != false { + t.Fatalf("transformed tool args = %s, want static callback before dynamic callback", transformed) + } + if got := staticCallbacks.Load(); got != 1 { + t.Fatalf("static callbacks = %d, want 1", got) + } if err := activation.Close(); err != nil { t.Fatalf("Close() error = %v", err) @@ -565,6 +618,9 @@ func TestActivateDynamicPluginsLoadsNativePluginThroughCgo(t *testing.T) { if string(afterClose) != `{"input":true}` { t.Fatalf("tool args after Close = %s, want unchanged args", afterClose) } + if got := staticCallbacks.Load(); got != 1 { + t.Fatalf("static callbacks after Close = %d, want 1", got) + } kinds, err := ListPluginKinds() if err != nil { t.Fatalf("ListPluginKinds() error = %v", err) @@ -574,6 +630,9 @@ func TestActivateDynamicPluginsLoadsNativePluginThroughCgo(t *testing.T) { t.Fatal("fixture_native remains registered after Close") } } + if err := os.Remove(pluginsTOML); err != nil { + t.Fatalf("Remove(plugins.toml) error = %v", err) + } _, _, err = ActivateDynamicPlugins(NewPluginConfig(), []DynamicPluginActivationSpec{{ PluginID: "fixture_missing", From e19fd943f6f201f3b706dcd53bc32a5b4ae83308 Mon Sep 17 00:00:00 2001 From: Bryan Bednarski Date: Tue, 14 Jul 2026 00:40:29 -0600 Subject: [PATCH 07/11] test(go): verify disabled component wire values Signed-off-by: Bryan Bednarski --- go/nemo_relay/plugin_activation_test.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/go/nemo_relay/plugin_activation_test.go b/go/nemo_relay/plugin_activation_test.go index d6baa02c1..5a1bc6b53 100644 --- a/go/nemo_relay/plugin_activation_test.go +++ b/go/nemo_relay/plugin_activation_test.go @@ -196,8 +196,22 @@ func TestComponentWrappersPreserveDisabledValuesDuringConversion(t *testing.T) { (PiiRedactionComponentSpec{Config: NewPiiRedactionConfig()}).PluginComponent(), } for _, component := range disabled { - if component.Enabled { - t.Fatalf("%s component became enabled during conversion", component.Kind) + payload, err := marshalPluginActivationConfig(PluginConfig{ + Components: []PluginComponentSpec{component}, + }) + if err != nil { + t.Fatalf("marshal %s component: %v", component.Kind, err) + } + var wire struct { + Components []struct { + Enabled *bool `json:"enabled"` + } `json:"components"` + } + if err := json.Unmarshal(payload, &wire); err != nil { + t.Fatalf("unmarshal %s component wire payload: %v", component.Kind, err) + } + if len(wire.Components) != 1 || wire.Components[0].Enabled == nil || *wire.Components[0].Enabled { + t.Fatalf("%s component wire payload = %s, want enabled false", component.Kind, payload) } } } From 0abca4df925eada376d179a682380f87ec9def4e Mon Sep 17 00:00:00 2001 From: Bryan Bednarski Date: Tue, 14 Jul 2026 07:23:04 -0600 Subject: [PATCH 08/11] docs(ffi): clarify activation teardown ownership Signed-off-by: Bryan Bednarski --- crates/ffi/nemo_relay.h | 17 ++++++++++++++--- crates/ffi/src/api/plugin.rs | 9 ++++++++- crates/ffi/src/types/mod.rs | 8 ++++++-- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/crates/ffi/nemo_relay.h b/crates/ffi/nemo_relay.h index 00535e621..48ddacd18 100644 --- a/crates/ffi/nemo_relay.h +++ b/crates/ffi/nemo_relay.h @@ -1466,12 +1466,19 @@ NemoRelayStatus nemo_relay_activate_dynamic_plugins(const char *config_json, * Clear one owned dynamic plugin activation. * * This operation is idempotent. A null handle is treated as already cleared. + * If teardown fails, the error is reported only by the call that performs the + * teardown. The activation is consumed regardless of the outcome, so a later + * clear returns success and does not report the earlier error again. + * Concurrent clear calls for the same handle are serialized, but they must not + * overlap with `nemo_relay_plugin_activation_free`. * The handle allocation remains owned by the caller and must still be passed * to `nemo_relay_plugin_activation_free`. * * # Safety * `activation` must be a valid activation handle returned by - * `nemo_relay_activate_dynamic_plugins`, or null. + * `nemo_relay_activate_dynamic_plugins`, or null. The caller must ensure the + * handle remains allocated for this call and that + * `nemo_relay_plugin_activation_free` does not run concurrently with it. */ NemoRelayStatus nemo_relay_plugin_activation_clear(struct FfiPluginActivation *activation); @@ -2525,11 +2532,15 @@ void nemo_relay_adaptive_runtime_free(struct FfiAdaptiveRuntime *ptr); * Any activation that has not already been explicitly cleared is cleaned up * best-effort by its Rust destructor before the allocation is released. The * caller's handle is set to null before cleanup, making repeated calls through - * the same handle variable safe. + * the same handle variable safe when they are sequential. * * # Safety * `ptr` must be null or point to a writable activation-handle variable whose - * value is null or was returned by `nemo_relay_activate_dynamic_plugins`. + * value is null or was returned by `nemo_relay_activate_dynamic_plugins`. The + * caller must ensure that no operation, including + * `nemo_relay_plugin_activation_clear`, accesses the activation concurrently + * with this call and that no operation can use the handle after this call + * begins. */ void nemo_relay_plugin_activation_free(struct FfiPluginActivation **ptr); diff --git a/crates/ffi/src/api/plugin.rs b/crates/ffi/src/api/plugin.rs index f50b63c18..f576b887e 100644 --- a/crates/ffi/src/api/plugin.rs +++ b/crates/ffi/src/api/plugin.rs @@ -272,12 +272,19 @@ pub unsafe extern "C" fn nemo_relay_activate_dynamic_plugins( /// Clear one owned dynamic plugin activation. /// /// This operation is idempotent. A null handle is treated as already cleared. +/// If teardown fails, the error is reported only by the call that performs the +/// teardown. The activation is consumed regardless of the outcome, so a later +/// clear returns success and does not report the earlier error again. +/// Concurrent clear calls for the same handle are serialized, but they must not +/// overlap with `nemo_relay_plugin_activation_free`. /// The handle allocation remains owned by the caller and must still be passed /// to `nemo_relay_plugin_activation_free`. /// /// # Safety /// `activation` must be a valid activation handle returned by -/// `nemo_relay_activate_dynamic_plugins`, or null. +/// `nemo_relay_activate_dynamic_plugins`, or null. The caller must ensure the +/// handle remains allocated for this call and that +/// `nemo_relay_plugin_activation_free` does not run concurrently with it. #[unsafe(no_mangle)] pub unsafe extern "C" fn nemo_relay_plugin_activation_clear( activation: *mut FfiPluginActivation, diff --git a/crates/ffi/src/types/mod.rs b/crates/ffi/src/types/mod.rs index 871811ec9..84532ffd6 100644 --- a/crates/ffi/src/types/mod.rs +++ b/crates/ffi/src/types/mod.rs @@ -299,11 +299,15 @@ pub unsafe extern "C" fn nemo_relay_adaptive_runtime_free(ptr: *mut FfiAdaptiveR /// Any activation that has not already been explicitly cleared is cleaned up /// best-effort by its Rust destructor before the allocation is released. The /// caller's handle is set to null before cleanup, making repeated calls through -/// the same handle variable safe. +/// the same handle variable safe when they are sequential. /// /// # Safety /// `ptr` must be null or point to a writable activation-handle variable whose -/// value is null or was returned by `nemo_relay_activate_dynamic_plugins`. +/// value is null or was returned by `nemo_relay_activate_dynamic_plugins`. The +/// caller must ensure that no operation, including +/// `nemo_relay_plugin_activation_clear`, accesses the activation concurrently +/// with this call and that no operation can use the handle after this call +/// begins. #[unsafe(no_mangle)] pub unsafe extern "C" fn nemo_relay_plugin_activation_free(ptr: *mut *mut FfiPluginActivation) { if ptr.is_null() { From 183ad3329543ef8a64c8c6a5bba879c0a2166d6e Mon Sep 17 00:00:00 2001 From: Bryan Bednarski Date: Tue, 14 Jul 2026 07:23:35 -0600 Subject: [PATCH 09/11] fix(go): keep plugin finalizers nonblocking Signed-off-by: Bryan Bednarski --- go/nemo_relay/plugin.go | 34 +++++++-------- go/nemo_relay/plugin_activation_test.go | 55 +++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 21 deletions(-) diff --git a/go/nemo_relay/plugin.go b/go/nemo_relay/plugin.go index 83046c332..e893f7b9a 100644 --- a/go/nemo_relay/plugin.go +++ b/go/nemo_relay/plugin.go @@ -132,23 +132,11 @@ var ( &report, ) if err := checkStatus(status); err != nil { - if report != nil { - C.nemo_relay_string_free(report) - } - if activation != nil { - C.nemo_relay_plugin_activation_clear(activation) - C.nemo_relay_plugin_activation_free(&activation) - } + cleanupPartialPluginActivation(activation, report) return nil, "", err } if activation == nil || report == nil { - if report != nil { - C.nemo_relay_string_free(report) - } - if activation != nil { - C.nemo_relay_plugin_activation_clear(activation) - C.nemo_relay_plugin_activation_free(&activation) - } + cleanupPartialPluginActivation(activation, report) return nil, "", errors.New("dynamic plugin activation returned incomplete outputs") } defer C.nemo_relay_string_free(report) @@ -180,6 +168,16 @@ var ( } ) +func cleanupPartialPluginActivation(activation *C.FfiPluginActivation, report *C.char) { + if report != nil { + C.nemo_relay_string_free(report) + } + if activation != nil { + C.nemo_relay_plugin_activation_clear(activation) + C.nemo_relay_plugin_activation_free(&activation) + } +} + // DiagnosticLevel is the severity level for one plugin diagnostic. type DiagnosticLevel string @@ -447,9 +445,11 @@ var reportPluginActivationCleanupError = func(err error) { } func finalizePluginActivation(state *pluginActivationState) { - if err := state.close(); err != nil { - reportPluginActivationCleanupError(err) - } + go func() { + if err := state.close(); err != nil { + reportPluginActivationCleanupError(err) + } + }() } // Close removes callbacks and subscribers before unloading plugin libraries diff --git a/go/nemo_relay/plugin_activation_test.go b/go/nemo_relay/plugin_activation_test.go index 5a1bc6b53..8465b6233 100644 --- a/go/nemo_relay/plugin_activation_test.go +++ b/go/nemo_relay/plugin_activation_test.go @@ -300,12 +300,59 @@ func TestPluginActivationFinalizerReportsClearFailure(t *testing.T) { wantErr := errors.New("teardown failed") clearPluginActivation = func(unsafe.Pointer) error { return wantErr } freePluginActivation = func(unsafe.Pointer) {} - var reported error - reportPluginActivationCleanupError = func(err error) { reported = err } + reported := make(chan error, 1) + reportPluginActivationCleanupError = func(err error) { reported <- err } finalizePluginActivation(&pluginActivationState{ptr: unsafe.Pointer(token)}) - if !errors.Is(reported, wantErr) { - t.Fatalf("reported finalizer error = %v, want %v", reported, wantErr) + select { + case err := <-reported: + if !errors.Is(err, wantErr) { + t.Fatalf("reported finalizer error = %v, want %v", err, wantErr) + } + case <-time.After(time.Second): + t.Fatal("finalizer did not report the clear failure") + } + runtime.KeepAlive(token) +} + +func TestPluginActivationFinalizerDoesNotBlockOnCleanup(t *testing.T) { + withPluginActivationStubs(t) + + token := new(byte) + cleanupStarted := make(chan struct{}) + releaseCleanup := make(chan struct{}) + cleanupFinished := make(chan struct{}) + clearPluginActivation = func(unsafe.Pointer) error { + close(cleanupStarted) + <-releaseCleanup + return nil + } + freePluginActivation = func(unsafe.Pointer) { close(cleanupFinished) } + + finalizerReturned := make(chan struct{}) + go func() { + finalizePluginActivation(&pluginActivationState{ptr: unsafe.Pointer(token)}) + close(finalizerReturned) + }() + + select { + case <-cleanupStarted: + case <-time.After(time.Second): + close(releaseCleanup) + t.Fatal("finalizer cleanup did not start") + } + select { + case <-finalizerReturned: + case <-time.After(time.Second): + close(releaseCleanup) + t.Fatal("finalizer blocked while native cleanup was in progress") + } + + close(releaseCleanup) + select { + case <-cleanupFinished: + case <-time.After(time.Second): + t.Fatal("asynchronous finalizer cleanup did not finish") } runtime.KeepAlive(token) } From ccf3991aed62c8187cb3bea58949b1fd62679c8a Mon Sep 17 00:00:00 2001 From: Bryan Bednarski Date: Tue, 14 Jul 2026 07:23:40 -0600 Subject: [PATCH 10/11] test(go): harden workspace version parsing Signed-off-by: Bryan Bednarski --- go/nemo_relay/plugin_activation_test.go | 89 +++++++++++++++++++------ 1 file changed, 68 insertions(+), 21 deletions(-) diff --git a/go/nemo_relay/plugin_activation_test.go b/go/nemo_relay/plugin_activation_test.go index 8465b6233..ea2c3ff91 100644 --- a/go/nemo_relay/plugin_activation_test.go +++ b/go/nemo_relay/plugin_activation_test.go @@ -10,8 +10,8 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "runtime" - "strconv" "strings" "sync" "sync/atomic" @@ -27,6 +27,8 @@ var ( goWorkerPluginFixtureOnce sync.Once goWorkerPluginFixturePath string goWorkerPluginFixtureErr error + workspacePackagePattern = regexp.MustCompile(`(?ms)^[\t ]*\[workspace\.package\][\t ]*(?:#[^\r\n]*)?\r?\n(.*?)(?:^[\t ]*\[|\z)`) + workspaceVersionPattern = regexp.MustCompile(`(?m)^[\t ]*version[\t ]*=[\t ]*(?:"([^"\r\n]+)"|'([^'\r\n]+)')[\t ]*(?:#[^\r\n]*)?\r?$`) ) func withPluginActivationStubs(t *testing.T) { @@ -966,29 +968,74 @@ func relayWorkspaceVersion(t *testing.T) string { if err != nil { t.Fatalf("read workspace Cargo.toml: %v", err) } - section := string(payload) - const header = "[workspace.package]" - start := strings.Index(section, header) - if start < 0 { - t.Fatal("workspace Cargo.toml has no [workspace.package] section") + version, err := workspaceVersionFromCargoTOML(payload) + if err != nil { + t.Fatal(err) } - section = section[start+len(header):] - if end := strings.Index(section, "\n["); end >= 0 { - section = section[:end] + return version +} + +func workspaceVersionFromCargoTOML(payload []byte) (string, error) { + section := workspacePackagePattern.FindSubmatch(payload) + if section == nil { + return "", errors.New("workspace Cargo.toml has no [workspace.package] section") } - for _, line := range strings.Split(section, "\n") { - line = strings.TrimSpace(line) - if !strings.HasPrefix(line, "version =") { - continue - } - version, err := strconv.Unquote(strings.TrimSpace(strings.TrimPrefix(line, "version ="))) - if err != nil { - t.Fatalf("parse workspace package version: %v", err) - } - return version + version := workspaceVersionPattern.FindSubmatch(section[1]) + if version == nil { + return "", errors.New("workspace package version not found") + } + if len(version[1]) != 0 { + return string(version[1]), nil + } + return string(version[2]), nil +} + +func TestWorkspaceVersionFromCargoTOML(t *testing.T) { + tests := []struct { + name string + payload string + want string + wantErr string + }{ + { + name: "standard workspace package", + payload: "[workspace.package]\nversion = \"0.6.0\"\nedition = \"2024\"\n\n[workspace.dependencies]\n", + want: "0.6.0", + }, + { + name: "whitespace comments CRLF and literal string", + payload: "[package]\r\nversion = \"9.9.9\"\r\n\r\n [workspace.package] # inherited metadata\r\n version\t=\t'0.7.0' # next release\r\n\r\n[[workspace.metadata.fixture]]\r\n", + want: "0.7.0", + }, + { + name: "missing workspace package", + payload: "[package]\nversion = \"0.6.0\"\n", + wantErr: "no [workspace.package] section", + }, + { + name: "missing workspace version", + payload: "[workspace.package]\nedition = \"2024\"\n", + wantErr: "workspace package version not found", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, err := workspaceVersionFromCargoTOML([]byte(test.payload)) + if test.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), test.wantErr) { + t.Fatalf("workspaceVersionFromCargoTOML() error = %v, want %q", err, test.wantErr) + } + return + } + if err != nil { + t.Fatalf("workspaceVersionFromCargoTOML() error = %v", err) + } + if got != test.want { + t.Fatalf("workspaceVersionFromCargoTOML() = %q, want %q", got, test.want) + } + }) } - t.Fatal("workspace package version not found") - return "" } func goNativeLibraryName() string { From d68108f49827cef93bbea546ff2a0ec260b0eb83 Mon Sep 17 00:00:00 2001 From: Bryan Bednarski Date: Tue, 14 Jul 2026 09:14:28 -0600 Subject: [PATCH 11/11] refactor(ffi): clarify dynamic plugin initialization Signed-off-by: Bryan Bednarski --- crates/ffi/README.md | 7 +- crates/ffi/nemo_relay.h | 14 ++-- crates/ffi/src/api/plugin.rs | 4 +- crates/ffi/src/types/mod.rs | 4 +- .../integration/plugin_activation_tests.rs | 22 +++--- crates/ffi/tests/unit/api/plugin_tests.rs | 14 ++-- go/nemo_relay/README.md | 7 +- go/nemo_relay/plugin.go | 14 ++-- go/nemo_relay/plugin_activation_test.go | 68 +++++++++---------- 9 files changed, 78 insertions(+), 76 deletions(-) diff --git a/crates/ffi/README.md b/crates/ffi/README.md index b3b2f73a1..ceb5e5cad 100644 --- a/crates/ffi/README.md +++ b/crates/ffi/README.md @@ -23,9 +23,10 @@ shared Rust runtime contract. This surface is experimental and source-first. The repository-maintained Go binding consumes it through CGo. -> **DO NOT TREAT AS PRODUCTION-READY:** the FFI/Go dynamic-plugin activation -> lifecycle needs a real consumer to validate shutdown, ownership, and error -> handling before it can be promoted to a stable contract. +> **DO NOT TREAT AS PRODUCTION-READY:** the experimental +> `nemo_relay_initialize_with_dynamic_plugins` lifecycle needs a real consumer +> to validate shutdown, ownership, and error handling before it can be promoted +> to a stable contract. ## Why Use It? diff --git a/crates/ffi/nemo_relay.h b/crates/ffi/nemo_relay.h index 48ddacd18..90bcdf566 100644 --- a/crates/ffi/nemo_relay.h +++ b/crates/ffi/nemo_relay.h @@ -1457,10 +1457,10 @@ NemoRelayStatus nemo_relay_openinference_subscriber_shutdown(const struct FfiOpe * `out_activation` and `out_report_json` must be valid, non-null, non-overlapping * output pointers. */ -NemoRelayStatus nemo_relay_activate_dynamic_plugins(const char *config_json, - const char *dynamic_plugins_json, - struct FfiPluginActivation **out_activation, - char **out_report_json); +NemoRelayStatus nemo_relay_initialize_with_dynamic_plugins(const char *config_json, + const char *dynamic_plugins_json, + struct FfiPluginActivation **out_activation, + char **out_report_json); /** * Clear one owned dynamic plugin activation. @@ -1476,7 +1476,7 @@ NemoRelayStatus nemo_relay_activate_dynamic_plugins(const char *config_json, * * # Safety * `activation` must be a valid activation handle returned by - * `nemo_relay_activate_dynamic_plugins`, or null. The caller must ensure the + * `nemo_relay_initialize_with_dynamic_plugins`, or null. The caller must ensure the * handle remains allocated for this call and that * `nemo_relay_plugin_activation_free` does not run concurrently with it. */ @@ -2527,7 +2527,7 @@ void nemo_relay_adaptive_runtime_free(struct FfiAdaptiveRuntime *ptr); /** * Free a dynamic plugin activation handle previously returned by - * `nemo_relay_activate_dynamic_plugins`. + * `nemo_relay_initialize_with_dynamic_plugins`. * * Any activation that has not already been explicitly cleared is cleaned up * best-effort by its Rust destructor before the allocation is released. The @@ -2536,7 +2536,7 @@ void nemo_relay_adaptive_runtime_free(struct FfiAdaptiveRuntime *ptr); * * # Safety * `ptr` must be null or point to a writable activation-handle variable whose - * value is null or was returned by `nemo_relay_activate_dynamic_plugins`. The + * value is null or was returned by `nemo_relay_initialize_with_dynamic_plugins`. The * caller must ensure that no operation, including * `nemo_relay_plugin_activation_clear`, accesses the activation concurrently * with this call and that no operation can use the handle after this call diff --git a/crates/ffi/src/api/plugin.rs b/crates/ffi/src/api/plugin.rs index f576b887e..9130a2f94 100644 --- a/crates/ffi/src/api/plugin.rs +++ b/crates/ffi/src/api/plugin.rs @@ -202,7 +202,7 @@ fn lock_plugin_activation( /// `out_activation` and `out_report_json` must be valid, non-null, non-overlapping /// output pointers. #[unsafe(no_mangle)] -pub unsafe extern "C" fn nemo_relay_activate_dynamic_plugins( +pub unsafe extern "C" fn nemo_relay_initialize_with_dynamic_plugins( config_json: *const c_char, dynamic_plugins_json: *const c_char, out_activation: *mut *mut FfiPluginActivation, @@ -282,7 +282,7 @@ pub unsafe extern "C" fn nemo_relay_activate_dynamic_plugins( /// /// # Safety /// `activation` must be a valid activation handle returned by -/// `nemo_relay_activate_dynamic_plugins`, or null. The caller must ensure the +/// `nemo_relay_initialize_with_dynamic_plugins`, or null. The caller must ensure the /// handle remains allocated for this call and that /// `nemo_relay_plugin_activation_free` does not run concurrently with it. #[unsafe(no_mangle)] diff --git a/crates/ffi/src/types/mod.rs b/crates/ffi/src/types/mod.rs index 84532ffd6..e66f0c4e1 100644 --- a/crates/ffi/src/types/mod.rs +++ b/crates/ffi/src/types/mod.rs @@ -294,7 +294,7 @@ pub unsafe extern "C" fn nemo_relay_adaptive_runtime_free(ptr: *mut FfiAdaptiveR } /// Free a dynamic plugin activation handle previously returned by -/// `nemo_relay_activate_dynamic_plugins`. +/// `nemo_relay_initialize_with_dynamic_plugins`. /// /// Any activation that has not already been explicitly cleared is cleaned up /// best-effort by its Rust destructor before the allocation is released. The @@ -303,7 +303,7 @@ pub unsafe extern "C" fn nemo_relay_adaptive_runtime_free(ptr: *mut FfiAdaptiveR /// /// # Safety /// `ptr` must be null or point to a writable activation-handle variable whose -/// value is null or was returned by `nemo_relay_activate_dynamic_plugins`. The +/// value is null or was returned by `nemo_relay_initialize_with_dynamic_plugins`. The /// caller must ensure that no operation, including /// `nemo_relay_plugin_activation_clear`, accesses the activation concurrently /// with this call and that no operation can use the handle after this call diff --git a/crates/ffi/tests/integration/plugin_activation_tests.rs b/crates/ffi/tests/integration/plugin_activation_tests.rs index 40f12deca..5ecfb74a6 100644 --- a/crates/ffi/tests/integration/plugin_activation_tests.rs +++ b/crates/ffi/tests/integration/plugin_activation_tests.rs @@ -97,7 +97,7 @@ fn run_discovered_config_activation_test() { let mut empty_report = ptr::null_mut(); assert_eq!( unsafe { - api::nemo_relay_activate_dynamic_plugins( + api::nemo_relay_initialize_with_dynamic_plugins( config.as_ptr(), empty_specs.as_ptr(), &mut empty_activation, @@ -146,7 +146,7 @@ source = "project-file" let manifest_dir = TempDir::new().expect("native manifest tempdir"); let manifest = write_native_manifest(manifest_dir.path(), build_native_fixture()); - let (mut activation, report) = activate_plugins(json!([{ + let (mut activation, report) = initialize_with_dynamic_plugins(json!([{ "plugin_id": "fixture_native", "kind": "rust_dynamic", "manifest_ref": manifest, @@ -239,7 +239,7 @@ fn ffi_activation_loads_native_callbacks_and_removes_them_before_free() { let manifest_dir = TempDir::new().expect("native manifest tempdir"); let manifest = write_native_manifest(manifest_dir.path(), build_native_fixture()); - let (mut activation, report) = activate_plugins(json!([{ + let (mut activation, report) = initialize_with_dynamic_plugins(json!([{ "plugin_id": "fixture_native", "kind": "rust_dynamic", "manifest_ref": manifest, @@ -270,7 +270,7 @@ fn ffi_activation_loads_native_callbacks_and_removes_them_before_free() { json!({"input": true}) ); - let (mut drop_activation, _) = activate_plugins(json!([{ + let (mut drop_activation, _) = initialize_with_dynamic_plugins(json!([{ "plugin_id": "fixture_native", "kind": "rust_dynamic", "manifest_ref": manifest, @@ -305,7 +305,7 @@ fn ffi_activation_rejects_overlapping_outputs_without_claiming_host() { let mut aliased_output = std::ptr::dangling_mut::(); let output_slot = &mut aliased_output as *mut *mut std::ffi::c_void; let status = unsafe { - api::nemo_relay_activate_dynamic_plugins( + api::nemo_relay_initialize_with_dynamic_plugins( config.as_ptr(), specs.as_ptr(), output_slot.cast::<*mut FfiPluginActivation>(), @@ -320,7 +320,7 @@ fn ffi_activation_rejects_overlapping_outputs_without_claiming_host() { .contains("must not overlap") ); - let (mut activation, _) = activate_plugins(specs_value); + let (mut activation, _) = initialize_with_dynamic_plugins(specs_value); unsafe { assert_eq!( api::nemo_relay_plugin_activation_clear(activation), @@ -337,7 +337,7 @@ fn ffi_activation_loads_worker_callbacks_and_stops_worker_on_clear() { let manifest_dir = TempDir::new().expect("worker manifest tempdir"); let manifest = write_worker_manifest(manifest_dir.path(), build_worker_fixture()); - let (mut activation, report) = activate_plugins(json!([{ + let (mut activation, report) = initialize_with_dynamic_plugins(json!([{ "plugin_id": "fixture_worker", "kind": "worker", "manifest_ref": manifest, @@ -393,7 +393,7 @@ fn ffi_activation_rolls_back_an_earlier_native_load_when_a_later_load_fails() { let mut activation = ptr::null_mut(); let mut report = ptr::null_mut(); let status = unsafe { - api::nemo_relay_activate_dynamic_plugins( + api::nemo_relay_initialize_with_dynamic_plugins( config.as_ptr(), specs.as_ptr(), &mut activation, @@ -409,7 +409,7 @@ fn ffi_activation_rolls_back_an_earlier_native_load_when_a_later_load_fails() { json!({"input": true}) ); - let (mut activation, _) = activate_plugins(json!([{ + let (mut activation, _) = initialize_with_dynamic_plugins(json!([{ "plugin_id": "fixture_native", "kind": "rust_dynamic", "manifest_ref": manifest, @@ -424,13 +424,13 @@ fn ffi_activation_rolls_back_an_earlier_native_load_when_a_later_load_fails() { } } -fn activate_plugins(specs: Json) -> (*mut FfiPluginActivation, Json) { +fn initialize_with_dynamic_plugins(specs: Json) -> (*mut FfiPluginActivation, Json) { let config = cstring(r#"{"version":1,"components":[]}"#); let specs = cstring(&specs.to_string()); let mut activation = ptr::null_mut(); let mut report = ptr::null_mut(); let status = unsafe { - api::nemo_relay_activate_dynamic_plugins( + api::nemo_relay_initialize_with_dynamic_plugins( config.as_ptr(), specs.as_ptr(), &mut activation, diff --git a/crates/ffi/tests/unit/api/plugin_tests.rs b/crates/ffi/tests/unit/api/plugin_tests.rs index af5c75fec..26c75e904 100644 --- a/crates/ffi/tests/unit/api/plugin_tests.rs +++ b/crates/ffi/tests/unit/api/plugin_tests.rs @@ -18,7 +18,7 @@ fn test_ffi_dynamic_plugin_activation_rejects_empty_specs_without_outputs() { let mut report_json = std::ptr::dangling_mut::(); unsafe { assert_eq!( - nemo_relay_activate_dynamic_plugins( + nemo_relay_initialize_with_dynamic_plugins( config.as_ptr(), specs.as_ptr(), &mut activation, @@ -49,7 +49,7 @@ fn test_ffi_dynamic_plugin_activation_rejects_invalid_inputs_without_outputs() { unsafe { let mut report = std::ptr::dangling_mut::(); assert_eq!( - nemo_relay_activate_dynamic_plugins( + nemo_relay_initialize_with_dynamic_plugins( config.as_ptr(), specs.as_ptr(), ptr::null_mut(), @@ -61,7 +61,7 @@ fn test_ffi_dynamic_plugin_activation_rejects_invalid_inputs_without_outputs() { let mut activation = std::ptr::dangling_mut::(); assert_eq!( - nemo_relay_activate_dynamic_plugins( + nemo_relay_initialize_with_dynamic_plugins( config.as_ptr(), specs.as_ptr(), &mut activation, @@ -78,7 +78,7 @@ fn test_ffi_dynamic_plugin_activation_rejects_invalid_inputs_without_outputs() { let mut activation = std::ptr::dangling_mut::(); let mut report = std::ptr::dangling_mut::(); assert_eq!( - nemo_relay_activate_dynamic_plugins( + nemo_relay_initialize_with_dynamic_plugins( config_json, specs_json, &mut activation, @@ -99,7 +99,7 @@ fn test_ffi_dynamic_plugin_activation_rejects_invalid_inputs_without_outputs() { let mut activation = ptr::null_mut(); let mut report = ptr::null_mut(); assert_eq!( - nemo_relay_activate_dynamic_plugins( + nemo_relay_initialize_with_dynamic_plugins( config.as_ptr(), invalid_shape.as_ptr(), &mut activation, @@ -140,7 +140,7 @@ fn test_ffi_dynamic_plugin_activation_surfaces_load_failures_and_releases_owner( let mut activation = ptr::null_mut(); let mut report = ptr::null_mut(); assert_eq!( - nemo_relay_activate_dynamic_plugins( + nemo_relay_initialize_with_dynamic_plugins( config.as_ptr(), specs.as_ptr(), &mut activation, @@ -159,7 +159,7 @@ fn test_ffi_dynamic_plugin_activation_surfaces_load_failures_and_releases_owner( let mut retry_activation = ptr::null_mut(); let mut retry_report = ptr::null_mut(); assert_eq!( - nemo_relay_activate_dynamic_plugins( + nemo_relay_initialize_with_dynamic_plugins( config.as_ptr(), specs.as_ptr(), &mut retry_activation, diff --git a/go/nemo_relay/README.md b/go/nemo_relay/README.md index d0b130cda..c7a07459c 100644 --- a/go/nemo_relay/README.md +++ b/go/nemo_relay/README.md @@ -24,9 +24,10 @@ Rust runtime. This binding is experimental and source-first. Rust, Python, and Node.js are the primary supported surfaces. -> **DO NOT TREAT AS PRODUCTION-READY:** the FFI/Go dynamic-plugin activation -> lifecycle needs a real consumer to validate shutdown, ownership, and error -> handling before it can be promoted to a stable contract. +> **DO NOT TREAT AS PRODUCTION-READY:** the experimental +> `InitializeWithDynamicPlugins` lifecycle needs a real consumer to validate +> shutdown, ownership, and error handling before it can be promoted to a stable +> contract. ## Why Use It? diff --git a/go/nemo_relay/plugin.go b/go/nemo_relay/plugin.go index e893f7b9a..7da42b475 100644 --- a/go/nemo_relay/plugin.go +++ b/go/nemo_relay/plugin.go @@ -28,7 +28,7 @@ typedef char* (*NemoRelayToolExecInterceptCb)(void* user_data, const char* args_ extern int32_t nemo_relay_validate_plugin_config(const char* config_json, char** out_json); extern int32_t nemo_relay_initialize_plugins(const char* config_json, char** out_json); -extern int32_t nemo_relay_activate_dynamic_plugins(const char* config_json, const char* dynamic_plugins_json, FfiPluginActivation** out_activation, char** out_report_json); +extern int32_t nemo_relay_initialize_with_dynamic_plugins(const char* config_json, const char* dynamic_plugins_json, FfiPluginActivation** out_activation, char** out_report_json); extern int32_t nemo_relay_plugin_activation_clear(FfiPluginActivation* activation); extern void nemo_relay_plugin_activation_free(FfiPluginActivation** activation); extern int32_t nemo_relay_clear_plugin_configuration(void); @@ -115,7 +115,7 @@ var ( C.nemo_relay_string_free(out) }) } - activateDynamicPluginsJSON = func(configJSON, dynamicPluginsJSON string) (unsafe.Pointer, string, error) { + initializeWithDynamicPluginsJSON = func(configJSON, dynamicPluginsJSON string) (unsafe.Pointer, string, error) { cConfig := C.CString(configJSON) cDynamicPlugins := C.CString(dynamicPluginsJSON) defer C.free(unsafe.Pointer(cConfig)) @@ -125,7 +125,7 @@ var ( var activation *C.FfiPluginActivation var report *C.char - status := C.nemo_relay_activate_dynamic_plugins( + status := C.nemo_relay_initialize_with_dynamic_plugins( cConfig, cDynamicPlugins, &activation, @@ -252,7 +252,7 @@ type DynamicPluginActivationSpec struct { } // PluginActivation owns the runtime registrations, native libraries, and -// workers created by ActivateDynamicPlugins. Copies share one activation +// workers created by InitializeWithDynamicPlugins. Copies share one activation // lifetime and may be closed safely from any copy. // // Experimental: this API needs a production consumer before its lifecycle @@ -386,7 +386,7 @@ func InitializePlugins(config PluginConfig) (ConfigReport, error) { return report, nil } -// ActivateDynamicPlugins discovers plugins.toml once during startup, layers the +// InitializeWithDynamicPlugins discovers plugins.toml once during startup, layers the // supplied config over the discovered configuration, and activates explicit // dynamic plugins in the same transaction. Explicit values take precedence // where both sources configure the same setting. Static components in the @@ -400,7 +400,7 @@ func InitializePlugins(config PluginConfig) (ConfigReport, error) { // // Experimental: this API needs a production consumer before its lifecycle // contract is considered stable. -func ActivateDynamicPlugins( +func InitializeWithDynamicPlugins( config PluginConfig, dynamicPlugins []DynamicPluginActivationSpec, ) (*PluginActivation, ConfigReport, error) { @@ -418,7 +418,7 @@ func ActivateDynamicPlugins( return nil, ConfigReport{}, err } - ptr, rawReport, err := activateDynamicPluginsJSON( + ptr, rawReport, err := initializeWithDynamicPluginsJSON( string(configPayload), string(dynamicPluginsPayload), ) diff --git a/go/nemo_relay/plugin_activation_test.go b/go/nemo_relay/plugin_activation_test.go index ea2c3ff91..c14b88c50 100644 --- a/go/nemo_relay/plugin_activation_test.go +++ b/go/nemo_relay/plugin_activation_test.go @@ -33,12 +33,12 @@ var ( func withPluginActivationStubs(t *testing.T) { t.Helper() - originalActivate := activateDynamicPluginsJSON + originalInitialize := initializeWithDynamicPluginsJSON originalClear := clearPluginActivation originalFree := freePluginActivation originalReporter := reportPluginActivationCleanupError t.Cleanup(func() { - activateDynamicPluginsJSON = originalActivate + initializeWithDynamicPluginsJSON = originalInitialize clearPluginActivation = originalClear freePluginActivation = originalFree reportPluginActivationCleanupError = originalReporter @@ -53,14 +53,14 @@ func fixtureDynamicPluginSpecs() []DynamicPluginActivationSpec { }} } -func TestActivateDynamicPluginsSerializesSpecsAndOwnsCleanup(t *testing.T) { +func TestInitializeWithDynamicPluginsSerializesSpecsAndOwnsCleanup(t *testing.T) { withPluginActivationStubs(t) token := new(byte) ptr := unsafe.Pointer(token) var gotConfig PluginConfig var gotSpecs []DynamicPluginActivationSpec - activateDynamicPluginsJSON = func(configJSON, specsJSON string) (unsafe.Pointer, string, error) { + initializeWithDynamicPluginsJSON = func(configJSON, specsJSON string) (unsafe.Pointer, string, error) { if err := json.Unmarshal([]byte(configJSON), &gotConfig); err != nil { t.Fatalf("invalid config JSON: %v", err) } @@ -85,7 +85,7 @@ func TestActivateDynamicPluginsSerializesSpecsAndOwnsCleanup(t *testing.T) { } environment := "/tmp/fixture-environment" - activation, report, err := ActivateDynamicPlugins(NewPluginConfig(), []DynamicPluginActivationSpec{ + activation, report, err := InitializeWithDynamicPlugins(NewPluginConfig(), []DynamicPluginActivationSpec{ { PluginID: "fixture.worker", Kind: DynamicPluginKindWorker, @@ -95,7 +95,7 @@ func TestActivateDynamicPluginsSerializesSpecsAndOwnsCleanup(t *testing.T) { }, }) if err != nil { - t.Fatalf("ActivateDynamicPlugins() error = %v", err) + t.Fatalf("InitializeWithDynamicPlugins() error = %v", err) } if gotConfig.Version != 1 { t.Fatalf("config version = %d, want 1", gotConfig.Version) @@ -152,7 +152,7 @@ func TestPluginConfigPublicJSONShapeRemainsCompatible(t *testing.T) { } } -func TestActivateDynamicPluginsUsesPrivateConfigWireShape(t *testing.T) { +func TestInitializeWithDynamicPluginsUsesPrivateConfigWireShape(t *testing.T) { withPluginActivationStubs(t) config := PluginConfig{Components: []PluginComponentSpec{{Kind: "fixture.disabled"}}} @@ -166,7 +166,7 @@ func TestActivateDynamicPluginsUsesPrivateConfigWireShape(t *testing.T) { } token := new(byte) - activateDynamicPluginsJSON = func(configJSON, specsJSON string) (unsafe.Pointer, string, error) { + initializeWithDynamicPluginsJSON = func(configJSON, specsJSON string) (unsafe.Pointer, string, error) { const wantConfig = `{"components":[{"kind":"fixture.disabled","enabled":false}]}` if configJSON != wantConfig { t.Fatalf("config JSON = %s, want %s", configJSON, wantConfig) @@ -180,9 +180,9 @@ func TestActivateDynamicPluginsUsesPrivateConfigWireShape(t *testing.T) { clearPluginActivation = func(unsafe.Pointer) error { return nil } freePluginActivation = func(unsafe.Pointer) {} - activation, _, err := ActivateDynamicPlugins(config, fixtureDynamicPluginSpecs()) + activation, _, err := InitializeWithDynamicPlugins(config, fixtureDynamicPluginSpecs()) if err != nil { - t.Fatalf("ActivateDynamicPlugins() error = %v", err) + t.Fatalf("InitializeWithDynamicPlugins() error = %v", err) } if err := activation.Close(); err != nil { t.Fatalf("Close() error = %v", err) @@ -218,22 +218,22 @@ func TestComponentWrappersPreserveDisabledValuesDuringConversion(t *testing.T) { } } -func TestActivateDynamicPluginsRejectsEmptySpecsWithoutCallingCgo(t *testing.T) { +func TestInitializeWithDynamicPluginsRejectsEmptySpecsWithoutCallingCgo(t *testing.T) { withPluginActivationStubs(t) activationCalls := 0 - activateDynamicPluginsJSON = func(string, string) (unsafe.Pointer, string, error) { + initializeWithDynamicPluginsJSON = func(string, string) (unsafe.Pointer, string, error) { activationCalls++ return nil, "", errors.New("unexpected CGo call") } for _, specs := range [][]DynamicPluginActivationSpec{nil, {}} { - activation, report, err := ActivateDynamicPlugins(NewPluginConfig(), specs) + activation, report, err := InitializeWithDynamicPlugins(NewPluginConfig(), specs) if err == nil || !strings.Contains(err.Error(), "at least one dynamic plugin") { - t.Fatalf("ActivateDynamicPlugins(%#v) error = %v, want empty-spec diagnostic", specs, err) + t.Fatalf("InitializeWithDynamicPlugins(%#v) error = %v, want empty-spec diagnostic", specs, err) } if activation != nil || len(report.Diagnostics) != 0 { - t.Fatalf("ActivateDynamicPlugins(%#v) = (%#v, %#v), want empty outputs", specs, activation, report) + t.Fatalf("InitializeWithDynamicPlugins(%#v) = (%#v, %#v), want empty outputs", specs, activation, report) } } if activationCalls != 0 { @@ -241,12 +241,12 @@ func TestActivateDynamicPluginsRejectsEmptySpecsWithoutCallingCgo(t *testing.T) } } -func TestActivateDynamicPluginsCleansUpInvalidReport(t *testing.T) { +func TestInitializeWithDynamicPluginsCleansUpInvalidReport(t *testing.T) { withPluginActivationStubs(t) token := new(byte) ptr := unsafe.Pointer(token) - activateDynamicPluginsJSON = func(string, string) (unsafe.Pointer, string, error) { + initializeWithDynamicPluginsJSON = func(string, string) (unsafe.Pointer, string, error) { return ptr, "not-json", nil } var calls []string @@ -256,9 +256,9 @@ func TestActivateDynamicPluginsCleansUpInvalidReport(t *testing.T) { } freePluginActivation = func(unsafe.Pointer) { calls = append(calls, "free") } - activation, _, err := ActivateDynamicPlugins(NewPluginConfig(), fixtureDynamicPluginSpecs()) + activation, _, err := InitializeWithDynamicPlugins(NewPluginConfig(), fixtureDynamicPluginSpecs()) if err == nil { - t.Fatal("ActivateDynamicPlugins() error = nil, want invalid report error") + t.Fatal("InitializeWithDynamicPlugins() error = nil, want invalid report error") } if activation != nil { t.Fatalf("activation = %#v, want nil", activation) @@ -501,11 +501,11 @@ func copiedPluginActivationWithGCSentinel( return copyValue } -func TestActivateDynamicPluginsSurfacesSerializationAndActivationErrors(t *testing.T) { +func TestInitializeWithDynamicPluginsSurfacesSerializationAndActivationErrors(t *testing.T) { withPluginActivationStubs(t) activationCalls := 0 - activateDynamicPluginsJSON = func(string, string) (unsafe.Pointer, string, error) { + initializeWithDynamicPluginsJSON = func(string, string) (unsafe.Pointer, string, error) { activationCalls++ return nil, "", errors.New("load failed") } @@ -516,7 +516,7 @@ func TestActivateDynamicPluginsSurfacesSerializationAndActivationErrors(t *testi Enabled: true, Config: map[string]any{"invalid": make(chan int)}, }) - if _, _, err := ActivateDynamicPlugins(invalidConfig, fixtureDynamicPluginSpecs()); err == nil { + if _, _, err := InitializeWithDynamicPlugins(invalidConfig, fixtureDynamicPluginSpecs()); err == nil { t.Fatal("invalid config serialization error = nil") } if activationCalls != 0 { @@ -529,14 +529,14 @@ func TestActivateDynamicPluginsSurfacesSerializationAndActivationErrors(t *testi ManifestRef: "/tmp/relay-plugin.toml", Config: map[string]any{"invalid": make(chan int)}, }} - if _, _, err := ActivateDynamicPlugins(NewPluginConfig(), invalidSpecs); err == nil { + if _, _, err := InitializeWithDynamicPlugins(NewPluginConfig(), invalidSpecs); err == nil { t.Fatal("invalid specs serialization error = nil") } if activationCalls != 0 { t.Fatalf("activation calls after specs serialization failure = %d", activationCalls) } - if _, _, err := ActivateDynamicPlugins(NewPluginConfig(), fixtureDynamicPluginSpecs()); err == nil || err.Error() != "load failed" { + if _, _, err := InitializeWithDynamicPlugins(NewPluginConfig(), fixtureDynamicPluginSpecs()); err == nil || err.Error() != "load failed" { t.Fatalf("activation error = %v, want load failed", err) } if activationCalls != 1 { @@ -551,7 +551,7 @@ func TestNilPluginActivationCloseIsSafe(t *testing.T) { } } -func TestActivateDynamicPluginsLoadsNativePluginThroughCgo(t *testing.T) { +func TestInitializeWithDynamicPluginsLoadsNativePluginThroughCgo(t *testing.T) { if err := ClearPluginConfiguration(); err != nil { t.Fatalf("ClearPluginConfiguration() error = %v", err) } @@ -624,14 +624,14 @@ source = "project-file" } }() - activation, report, err := ActivateDynamicPlugins(NewPluginConfig(), []DynamicPluginActivationSpec{{ + activation, report, err := InitializeWithDynamicPlugins(NewPluginConfig(), []DynamicPluginActivationSpec{{ PluginID: "fixture_native", Kind: DynamicPluginKindRustDynamic, ManifestRef: manifest, Config: map[string]any{}, }}) if err != nil { - t.Fatalf("ActivateDynamicPlugins() error = %v", err) + t.Fatalf("InitializeWithDynamicPlugins() error = %v", err) } defer func() { if err := activation.Close(); err != nil { @@ -697,7 +697,7 @@ source = "project-file" t.Fatalf("Remove(plugins.toml) error = %v", err) } - _, _, err = ActivateDynamicPlugins(NewPluginConfig(), []DynamicPluginActivationSpec{{ + _, _, err = InitializeWithDynamicPlugins(NewPluginConfig(), []DynamicPluginActivationSpec{{ PluginID: "fixture_missing", Kind: DynamicPluginKindRustDynamic, ManifestRef: filepath.Join(t.TempDir(), "missing-relay-plugin.toml"), @@ -707,21 +707,21 @@ source = "project-file" } } -func TestActivateDynamicPluginsLoadsWorkerPluginThroughCgo(t *testing.T) { +func TestInitializeWithDynamicPluginsLoadsWorkerPluginThroughCgo(t *testing.T) { if err := ClearPluginConfiguration(); err != nil { t.Fatalf("ClearPluginConfiguration() error = %v", err) } executable := goWorkerPluginFixture(t) manifest := writeGoWorkerPluginManifest(t, executable) - activation, report, err := ActivateDynamicPlugins(NewPluginConfig(), []DynamicPluginActivationSpec{{ + activation, report, err := InitializeWithDynamicPlugins(NewPluginConfig(), []DynamicPluginActivationSpec{{ PluginID: "fixture_worker", Kind: DynamicPluginKindWorker, ManifestRef: manifest, Config: map[string]any{}, }}) if err != nil { - t.Fatalf("ActivateDynamicPlugins() error = %v", err) + t.Fatalf("InitializeWithDynamicPlugins() error = %v", err) } defer func() { if err := activation.Close(); err != nil { @@ -774,7 +774,7 @@ func TestPluginActivationFinalizerReleasesHostOwnership(t *testing.T) { for time.Now().Before(deadline) { runtime.GC() runtime.Gosched() - activation, _, err := ActivateDynamicPlugins(NewPluginConfig(), specs) + activation, _, err := InitializeWithDynamicPlugins(NewPluginConfig(), specs) if err == nil { if closeErr := activation.Close(); closeErr != nil { t.Fatalf("replacement activation Close() error = %v", closeErr) @@ -792,9 +792,9 @@ func TestPluginActivationFinalizerReleasesHostOwnership(t *testing.T) { //go:noinline func createUnclosedPluginActivation(t *testing.T, specs []DynamicPluginActivationSpec) { t.Helper() - activation, _, err := ActivateDynamicPlugins(NewPluginConfig(), specs) + activation, _, err := InitializeWithDynamicPlugins(NewPluginConfig(), specs) if err != nil { - t.Fatalf("ActivateDynamicPlugins() error = %v", err) + t.Fatalf("InitializeWithDynamicPlugins() error = %v", err) } runtime.KeepAlive(activation) }