Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 54 additions & 5 deletions crates/node/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1533,9 +1533,45 @@ impl Drop for PersistentJsFunction {
}
}

struct NodePluginValidateCallback {
direct: PersistentJsFunction,
thread_safe: ThreadsafeFunction<Json, ErrorStrategy::Fatal>,
registration_thread: std::thread::ThreadId,
}

impl NodePluginValidateCallback {
fn call(&self, plugin_config: Json) -> napi::Result<Json> {
if std::thread::current().id() == self.registration_thread {
return self.direct.call_validate(&plugin_config);
}

let (tx, rx) = std::sync::mpsc::sync_channel(1);
let status = self.thread_safe.call_with_return_value(
plugin_config,
ThreadsafeFunctionCallMode::Blocking,
move |value: Option<Json>| {
let result = callable::unwrap_middleware_result(
callback_json(value),
"JS plugin validate callback failed",
)
.map_err(|error| napi::Error::from_reason(error.to_string()));
let _ = tx.send(result);
Ok(())
},
);
if status != napi::Status::Ok {
return Err(napi::Error::from_reason(format!(
"failed to queue JS plugin validate callback: {status:?}"
)));
}
rx.recv()
.map_err(|_| napi::Error::from_reason("JS plugin validate completion channel closed"))?
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

struct NodePlugin {
plugin_kind: String,
validate: Option<PersistentJsFunction>,
validate: Option<NodePluginValidateCallback>,
Comment thread
willkill07 marked this conversation as resolved.
register: ThreadsafeFunction<NodePluginRegisterCall, ErrorStrategy::Fatal>,
}

Expand All @@ -1551,7 +1587,7 @@ impl Plugin for NodePlugin {
let Some(validate) = &self.validate else {
return vec![];
};
match validate.call_validate(&Json::Object(plugin_config.clone())) {
match validate.call(Json::Object(plugin_config.clone())) {
Ok(Json::Null) => vec![],
Ok(value) => {
serde_json::from_value::<Vec<ConfigDiagnostic>>(value).unwrap_or_else(|e| {
Expand Down Expand Up @@ -4748,8 +4784,21 @@ pub fn register_plugin(
validate: Option<JsFunction>,
register: JsFunction,
) -> napi::Result<()> {
let validate_tsfn = match validate {
Some(func) => Some(PersistentJsFunction::new(&env, &func)?),
let validate_callback = match validate {
Some(func) => {
let direct = PersistentJsFunction::new(&env, &func)?;
let callback = callable::safe_middleware_callback(&env, &func)?;
let mut thread_safe = callback.create_threadsafe_function(
0,
|ctx: napi::threadsafe_function::ThreadSafeCallContext<Json>| Ok(vec![ctx.value]),
)?;
thread_safe.unref(&env)?;
Some(NodePluginValidateCallback {
direct,
thread_safe,
registration_thread: std::thread::current().id(),
})
}
None => None,
};
let mut register_tsfn = register
Expand Down Expand Up @@ -4777,7 +4826,7 @@ pub fn register_plugin(

register_plugin_impl(Arc::new(NodePlugin {
plugin_kind,
validate: validate_tsfn,
validate: validate_callback,
register: register_tsfn,
}))
.map_err(|e| napi::Error::from_reason(e.to_string()))
Expand Down
70 changes: 70 additions & 0 deletions crates/node/tests/adaptive_tests.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,48 @@ describe('core plugins', () => {
}
});

it('validates a registered JS plugin during asynchronous initialization', async () => {
const pluginKind = `node.test.async_validate.${Date.now()}`;
const config = {
version: 1,
components: [plugin.ComponentSpec(pluginKind, { marker: 'validated' })],
};
let validateCalls = 0;
let registerCalls = 0;

plugin.register(pluginKind, {
validate(pluginConfig) {
validateCalls += 1;
assert.equal(pluginConfig.marker, 'validated');
return [];
},
register(pluginConfig, context) {
registerCalls += 1;
context.registerToolRequestIntercept('marker', 1, false, (_name, args) => ({
...args,
marker: pluginConfig.marker,
}));
},
});

try {
assert.deepEqual(plugin.validate(config).diagnostics, []);
assert.deepEqual((await plugin.initialize(config)).diagnostics, []);
assert.equal(validateCalls, 2);
assert.equal(registerCalls, 1);
assert.deepEqual(await lib.toolCallExecute('validated_plugin_tool', {}, (args) => args), {
marker: 'validated',
});

plugin.clear();
assert.equal(plugin.deregister(pluginKind), true);
assert.deepEqual(await lib.toolCallExecute('cleared_plugin_tool', {}, (args) => args), {});
} finally {
plugin.clear();
plugin.deregister(pluginKind);
}
});

it('treats implicit undefined plugin validation as no diagnostics', () => {
const pluginKind = `node.test.validate_undefined.${Date.now()}`;

Expand All @@ -99,6 +141,34 @@ describe('core plugins', () => {
}
});

it('reports throwing async plugin validation without terminating Node', async () => {
const pluginKind = `node.test.async_validate_throw.${Date.now()}`;
plugin.register(pluginKind, {
validate() {
throw new Error('plugin validation boom');
},
register() {
assert.fail('registration must not run after validation fails');
},
});

try {
const config = {
version: 1,
components: [plugin.ComponentSpec(pluginKind, {})],
};
await assert.rejects(() => plugin.initialize(config), /plugin validation boom/);
const report = plugin.validate(config);
assert.equal(report.diagnostics.length, 1);
assert.equal(report.diagnostics[0].code, 'plugin.validate_failed');
assert.match(report.diagnostics[0].message, /plugin validation boom/);
assert.deepEqual(await lib.toolCallExecute('validation_error_survives', {}, (args) => args), {});
} finally {
plugin.clear();
plugin.deregister(pluginKind);
}
});

it('invokes top-level plugin registration during plugin configuration', async () => {
const pluginKind = `node.test.register.${Date.now()}`;
let registerCalls = 0;
Expand Down
Loading