Add support for engine calls from plugins (#12029)

# Description

This allows plugins to make calls back to the engine to get config,
evaluate closures, and do other things that must be done within the
engine process.

Engine calls can both produce and consume streams as necessary. Closures
passed to plugins can both accept stream input and produce stream output
sent back to the plugin.

Engine calls referring to a plugin call's context can be processed as
long either the response hasn't been received, or the response created
streams that haven't ended yet.

This is a breaking API change for plugins. There are some pretty major
changes to the interface that plugins must implement, including:

1. Plugins now run with `&self` and must be `Sync`. Executing multiple
plugin calls in parallel is supported, and there's a chance that a
closure passed to a plugin could invoke the same plugin. Supporting
state across plugin invocations is left up to the plugin author to do in
whichever way they feel best, but the plugin object itself is still
shared. Even though the engine doesn't run multiple plugin calls through
the same process yet, I still considered it important to break the API
in this way at this stage. We might want to consider an optional
threadpool feature for performance.

2. Plugins take a reference to `EngineInterface`, which can be cloned.
This interface allows plugins to make calls back to the engine,
including for getting config and running closures.

3. Plugins no longer take the `config` parameter. This can be accessed
from the interface via the `.get_plugin_config()` engine call.


# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->
Not only does this have plugin protocol changes, it will require plugins
to make some code changes before they will work again. But on the plus
side, the engine call feature is extensible, and we can add more things
to it as needed.

Plugin maintainers will have to change the trait signature at the very
least. If they were using `config`, they will have to call
`engine.get_plugin_config()` instead.

If they were using the mutable reference to the plugin, they will have
to come up with some strategy to work around it (for example, for `Inc`
I just cloned it). This shouldn't be such a big deal at the moment as
it's not like plugins have ever run as daemons with persistent state in
the past, and they don't in this PR either. But I thought it was
important to make the change before we support plugins as daemons, as an
exclusive mutable reference is not compatible with parallel plugin
calls.

I suggest this gets merged sometime *after* the current pending release,
so that we have some time to adjust to the previous plugin protocol
changes that don't require code changes before making ones that do.

# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`


# After Submitting
I will document the additional protocol features (`EngineCall`,
`EngineCallResponse`), and constraints on plugin call processing if
engine calls are used - basically, to be aware that an engine call could
result in a nested plugin call, so the plugin should be able to handle
that.

crates/nu-plugin/src/protocol/mod.rs

@@ -9,13 +9,15 @@ mod tests;
 pub(crate) mod test_util;
 
 pub use evaluated_call::EvaluatedCall;
-use nu_protocol::{PluginSignature, RawStream, ShellError, Span, Spanned, Value};
+use nu_protocol::{
+    engine::Closure, Config, PipelineData, PluginSignature, RawStream, ShellError, Span, Spanned,
+    Value,
+};
 pub use plugin_custom_value::PluginCustomValue;
-pub(crate) use protocol_info::ProtocolInfo;
-use serde::{Deserialize, Serialize};
-
+pub use protocol_info::ProtocolInfo;
 #[cfg(test)]
-pub(crate) use protocol_info::Protocol;
+pub use protocol_info::{Feature, Protocol};
+use serde::{Deserialize, Serialize};
 
 /// A sequential identifier for a stream
 pub type StreamId = usize;
@@ -23,6 +25,9 @@ pub type StreamId = usize;
 /// A sequential identifier for a [`PluginCall`]
 pub type PluginCallId = usize;
 
+/// A sequential identifier for an [`EngineCall`]
+pub type EngineCallId = usize;
+
 /// Information about a plugin command invocation. This includes an [`EvaluatedCall`] as a
 /// serializable representation of [`nu_protocol::ast::Call`]. The type parameter determines
 /// the input type.
@@ -34,8 +39,6 @@ pub struct CallInfo<D> {
     pub call: EvaluatedCall,
     /// Pipeline input. This is usually [`nu_protocol::PipelineData`] or [`PipelineDataHeader`]
     pub input: D,
-    /// Plugin configuration, if available
-    pub config: Option<Value>,
 }
 
 /// The initial (and perhaps only) part of any [`nu_protocol::PipelineData`] sent over the wire.
@@ -57,6 +60,30 @@ pub enum PipelineDataHeader {
     ExternalStream(ExternalStreamInfo),
 }
 
+impl PipelineDataHeader {
+    /// Return a list of stream IDs embedded in the header
+    pub(crate) fn stream_ids(&self) -> Vec<StreamId> {
+        match self {
+            PipelineDataHeader::Empty => vec![],
+            PipelineDataHeader::Value(_) => vec![],
+            PipelineDataHeader::ListStream(info) => vec![info.id],
+            PipelineDataHeader::ExternalStream(info) => {
+                let mut out = vec![];
+                if let Some(stdout) = &info.stdout {
+                    out.push(stdout.id);
+                }
+                if let Some(stderr) = &info.stderr {
+                    out.push(stderr.id);
+                }
+                if let Some(exit_code) = &info.exit_code {
+                    out.push(exit_code.id);
+                }
+                out
+            }
+        }
+    }
+}
+
 /// Additional information about list (value) streams
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
 pub struct ListStreamInfo {
@@ -117,6 +144,9 @@ pub enum PluginInput {
     /// Don't expect any more plugin calls. Exit after all currently executing plugin calls are
     /// finished.
     Goodbye,
+    /// Response to an [`EngineCall`]. The ID should be the same one sent with the engine call this
+    /// is responding to
+    EngineCallResponse(EngineCallId, EngineCallResponse<PipelineDataHeader>),
     /// Stream control or data message. Untagged to keep them as small as possible.
     ///
     /// For example, `Stream(Ack(0))` is encoded as `{"Ack": 0}`
@@ -301,6 +331,15 @@ pub enum PluginOutput {
     /// A response to a [`PluginCall`]. The ID should be the same sent with the plugin call this
     /// is a response to
     CallResponse(PluginCallId, PluginCallResponse<PipelineDataHeader>),
+    /// Execute an [`EngineCall`]. Engine calls must be executed within the `context` of a plugin
+    /// call, and the `id` should not have been used before
+    EngineCall {
+        /// The plugin call (by ID) to execute in the context of
+        context: PluginCallId,
+        /// A new identifier for this engine call. The response will reference this ID
+        id: EngineCallId,
+        call: EngineCall<PipelineDataHeader>,
+    },
     /// Stream control or data message. Untagged to keep them as small as possible.
     ///
     /// For example, `Stream(Ack(0))` is encoded as `{"Ack": 0}`
@@ -324,3 +363,61 @@ impl From<StreamMessage> for PluginOutput {
         PluginOutput::Stream(stream_msg)
     }
 }
+
+/// A remote call back to the engine during the plugin's execution.
+///
+/// The type parameter determines the input type, for calls that take pipeline data.
+#[derive(Serialize, Deserialize, Debug, Clone)]
+pub enum EngineCall<D> {
+    /// Get the full engine configuration
+    GetConfig,
+    /// Get the plugin-specific configuration (`$env.config.plugins.NAME`)
+    GetPluginConfig,
+    /// Evaluate a closure with stream input/output
+    EvalClosure {
+        /// The closure to call.
+        ///
+        /// This may come from a [`Value::Closure`] passed in as an argument to the plugin.
+        closure: Spanned<Closure>,
+        /// Positional arguments to add to the closure call
+        positional: Vec<Value>,
+        /// Input to the closure
+        input: D,
+        /// Whether to redirect stdout from external commands
+        redirect_stdout: bool,
+        /// Whether to redirect stderr from external commands
+        redirect_stderr: bool,
+    },
+}
+
+impl<D> EngineCall<D> {
+    /// Get the name of the engine call so it can be embedded in things like error messages
+    pub fn name(&self) -> &'static str {
+        match self {
+            EngineCall::GetConfig => "GetConfig",
+            EngineCall::GetPluginConfig => "GetPluginConfig",
+            EngineCall::EvalClosure { .. } => "EvalClosure",
+        }
+    }
+}
+
+/// The response to an [EngineCall]. The type parameter determines the output type for pipeline
+/// data.
+#[derive(Serialize, Deserialize, Debug, Clone)]
+pub enum EngineCallResponse<D> {
+    Error(ShellError),
+    PipelineData(D),
+    Config(Box<Config>),
+}
+
+impl EngineCallResponse<PipelineData> {
+    /// Build an [`EngineCallResponse::PipelineData`] from a [`Value`]
+    pub(crate) fn value(value: Value) -> EngineCallResponse<PipelineData> {
+        EngineCallResponse::PipelineData(PipelineData::Value(value, None))
+    }
+
+    /// An [`EngineCallResponse::PipelineData`] with [`PipelineData::Empty`]
+    pub(crate) const fn empty() -> EngineCallResponse<PipelineData> {
+        EngineCallResponse::PipelineData(PipelineData::Empty)
+    }
+}

crates/nu-plugin/src/protocol/plugin_custom_value.rs

@@ -146,6 +146,11 @@ impl PluginCustomValue {
                     Self::add_source(list_value, source);
                 }
             }
+            Value::Closure { ref mut val, .. } => {
+                for (_, captured_value) in val.captures.iter_mut() {
+                    Self::add_source(captured_value, source);
+                }
+            }
             // All of these don't contain other values
             Value::Bool { .. }
             | Value::Int { .. }
@@ -156,7 +161,6 @@ impl PluginCustomValue {
             | Value::String { .. }
             | Value::Glob { .. }
             | Value::Block { .. }
-            | Value::Closure { .. }
             | Value::Nothing { .. }
             | Value::Error { .. }
             | Value::Binary { .. }
@@ -214,6 +218,10 @@ impl PluginCustomValue {
             Value::List { ref mut vals, .. } => vals
                 .iter_mut()
                 .try_for_each(|list_value| Self::verify_source(list_value, source)),
+            Value::Closure { ref mut val, .. } => val
+                .captures
+                .iter_mut()
+                .try_for_each(|(_, captured_value)| Self::verify_source(captured_value, source)),
             // All of these don't contain other values
             Value::Bool { .. }
             | Value::Int { .. }
@@ -224,7 +232,6 @@ impl PluginCustomValue {
             | Value::String { .. }
             | Value::Glob { .. }
             | Value::Block { .. }
-            | Value::Closure { .. }
             | Value::Nothing { .. }
             | Value::Error { .. }
             | Value::Binary { .. }
@@ -266,6 +273,11 @@ impl PluginCustomValue {
             Value::List { ref mut vals, .. } => vals
                 .iter_mut()
                 .try_for_each(Self::serialize_custom_values_in),
+            Value::Closure { ref mut val, .. } => val
+                .captures
+                .iter_mut()
+                .map(|(_, captured_value)| captured_value)
+                .try_for_each(Self::serialize_custom_values_in),
             // All of these don't contain other values
             Value::Bool { .. }
             | Value::Int { .. }
@@ -276,7 +288,6 @@ impl PluginCustomValue {
             | Value::String { .. }
             | Value::Glob { .. }
             | Value::Block { .. }
-            | Value::Closure { .. }
             | Value::Nothing { .. }
             | Value::Error { .. }
             | Value::Binary { .. }
@@ -316,6 +327,11 @@ impl PluginCustomValue {
             Value::List { ref mut vals, .. } => vals
                 .iter_mut()
                 .try_for_each(Self::deserialize_custom_values_in),
+            Value::Closure { ref mut val, .. } => val
+                .captures
+                .iter_mut()
+                .map(|(_, captured_value)| captured_value)
+                .try_for_each(Self::deserialize_custom_values_in),
             // All of these don't contain other values
             Value::Bool { .. }
             | Value::Int { .. }
@@ -326,7 +342,6 @@ impl PluginCustomValue {
             | Value::String { .. }
             | Value::Glob { .. }
             | Value::Block { .. }
-            | Value::Closure { .. }
             | Value::Nothing { .. }
             | Value::Error { .. }
             | Value::Binary { .. }

crates/nu-plugin/src/protocol/plugin_custom_value/tests.rs

@@ -1,4 +1,6 @@
-use nu_protocol::{ast::RangeInclusion, record, CustomValue, Range, ShellError, Span, Value};
+use nu_protocol::{
+    ast::RangeInclusion, engine::Closure, record, CustomValue, Range, ShellError, Span, Value,
+};
 
 use crate::{
     plugin::PluginIdentity,
@@ -180,6 +182,50 @@ fn add_source_nested_list() -> Result<(), ShellError> {
     })
 }
 
+fn check_closure_custom_values(
+    val: &Value,
+    indices: impl IntoIterator<Item = usize>,
+    mut f: impl FnMut(usize, &dyn CustomValue) -> Result<(), ShellError>,
+) -> Result<(), ShellError> {
+    let closure = val.as_closure()?;
+    for index in indices {
+        let val = closure
+            .captures
+            .get(index)
+            .unwrap_or_else(|| panic!("[{index}] not present in closure"));
+        let custom_value = val
+            .1
+            .as_custom_value()
+            .unwrap_or_else(|_| panic!("[{index}] not custom value"));
+        f(index, custom_value)?;
+    }
+    Ok(())
+}
+
+#[test]
+fn add_source_nested_closure() -> Result<(), ShellError> {
+    let orig_custom_val = Value::test_custom_value(Box::new(test_plugin_custom_value()));
+    let mut val = Value::test_closure(Closure {
+        block_id: 0,
+        captures: vec![(0, orig_custom_val.clone()), (1, orig_custom_val.clone())],
+    });
+    let source = PluginIdentity::new_fake("foo");
+    PluginCustomValue::add_source(&mut val, &source);
+
+    check_closure_custom_values(&val, 0..=1, |index, custom_value| {
+        let plugin_custom_value: &PluginCustomValue = custom_value
+            .as_any()
+            .downcast_ref()
+            .unwrap_or_else(|| panic!("[{index}] not PluginCustomValue"));
+        assert_eq!(
+            Some(&source),
+            plugin_custom_value.source.as_ref(),
+            "[{index}] source not set correctly"
+        );
+        Ok(())
+    })
+}
+
 #[test]
 fn verify_source_error_message() -> Result<(), ShellError> {
     let span = Span::new(5, 7);
@@ -322,6 +368,40 @@ fn verify_source_nested_list() -> Result<(), ShellError> {
     Ok(())
 }
 
+#[test]
+fn verify_source_nested_closure() -> Result<(), ShellError> {
+    let native_val = Value::test_custom_value(Box::new(TestCustomValue(32)));
+    let source = PluginIdentity::new_fake("test");
+    for (name, mut val) in [
+        (
+            "first capture",
+            Value::test_closure(Closure {
+                block_id: 0,
+                captures: vec![(0, native_val.clone()), (1, Value::test_nothing())],
+            }),
+        ),
+        (
+            "second capture",
+            Value::test_closure(Closure {
+                block_id: 0,
+                captures: vec![(0, Value::test_nothing()), (1, native_val.clone())],
+            }),
+        ),
+    ] {
+        PluginCustomValue::verify_source(&mut val, &source)
+            .expect_err(&format!("error not generated on {name}"));
+    }
+
+    let mut ok_closure = Value::test_closure(Closure {
+        block_id: 0,
+        captures: vec![(0, Value::test_nothing())],
+    });
+    PluginCustomValue::verify_source(&mut ok_closure, &source)
+        .expect("ok_closure should not generate error");
+
+    Ok(())
+}
+
 #[test]
 fn serialize_in_root() -> Result<(), ShellError> {
     let span = Span::new(4, 10);
@@ -406,6 +486,28 @@ fn serialize_in_list() -> Result<(), ShellError> {
     })
 }
 
+#[test]
+fn serialize_in_closure() -> Result<(), ShellError> {
+    let orig_custom_val = Value::test_custom_value(Box::new(TestCustomValue(24)));
+    let mut val = Value::test_closure(Closure {
+        block_id: 0,
+        captures: vec![(0, orig_custom_val.clone()), (1, orig_custom_val.clone())],
+    });
+    PluginCustomValue::serialize_custom_values_in(&mut val)?;
+
+    check_closure_custom_values(&val, 0..=1, |index, custom_value| {
+        let plugin_custom_value: &PluginCustomValue = custom_value
+            .as_any()
+            .downcast_ref()
+            .unwrap_or_else(|| panic!("[{index}] not PluginCustomValue"));
+        assert_eq!(
+            "TestCustomValue", plugin_custom_value.name,
+            "[{index}] name not set correctly"
+        );
+        Ok(())
+    })
+}
+
 #[test]
 fn deserialize_in_root() -> Result<(), ShellError> {
     let span = Span::new(4, 10);
@@ -490,3 +592,26 @@ fn deserialize_in_list() -> Result<(), ShellError> {
         Ok(())
     })
 }
+
+#[test]
+fn deserialize_in_closure() -> Result<(), ShellError> {
+    let orig_custom_val = Value::test_custom_value(Box::new(test_plugin_custom_value()));
+    let mut val = Value::test_closure(Closure {
+        block_id: 0,
+        captures: vec![(0, orig_custom_val.clone()), (1, orig_custom_val.clone())],
+    });
+    PluginCustomValue::deserialize_custom_values_in(&mut val)?;
+
+    check_closure_custom_values(&val, 0..=1, |index, custom_value| {
+        let test_custom_value: &TestCustomValue = custom_value
+            .as_any()
+            .downcast_ref()
+            .unwrap_or_else(|| panic!("[{index}] not TestCustomValue"));
+        assert_eq!(
+            expected_test_custom_value(),
+            *test_custom_value,
+            "[{index}] name not deserialized correctly"
+        );
+        Ok(())
+    })
+}