Refactor NetInteractor.Mcp to inherit from McpServerTool with PlaywrightWebAccessor default#7
Conversation
Co-authored-by: kerryjiang <456060+kerryjiang@users.noreply.github.com>
Co-authored-by: kerryjiang <456060+kerryjiang@users.noreply.github.com>
…ools Co-authored-by: kerryjiang <456060+kerryjiang@users.noreply.github.com>
| { | ||
| /// <summary> | ||
| /// Result of executing a NetInteractor script. | ||
| /// </summary> |
There was a problem hiding this comment.
Why not just use InteractionResult directly?
There was a problem hiding this comment.
Done in commit 4cd079f. Deleted ExecuteScriptResult.cs and now ExecuteScriptAsync returns InteractionResult directly.
| /// MCP Server Tool for NetInteractor web automation. | ||
| /// This tool enables AI agents to execute web interactions and automation scripts using InteractExecutor. | ||
| /// </summary> | ||
| public class NetInteractorTool |
There was a problem hiding this comment.
the class should be base on McpServerTool:
NetInteractorTool : McpServerTool
There was a problem hiding this comment.
Done in commit 4cd079f. NetInteractorTool now inherits from McpServerTool and implements the required abstract members (ProtocolTool, Metadata, InvokeAsync).
| /// Initializes a new instance of the NetInteractorTool class with the default HttpClientWebAccessor. | ||
| /// </summary> | ||
| public NetInteractorTool() | ||
| : this(new HttpClientWebAccessor()) |
There was a problem hiding this comment.
Use PlaywrightWebAccessor by default
There was a problem hiding this comment.
Done in commit 4cd079f. Default constructor now uses PlaywrightWebAccessor() instead of HttpClientWebAccessor().
| ["script"] = new ToolParameterMetadata | ||
| { | ||
| Name = "script", | ||
| Description = "The XML script defining the web interaction workflow", |
There was a problem hiding this comment.
Can you give more explanation so AI can understand how to write NetInteractor script? We better have a way to explain all the targets supported by NetInteractor
There was a problem hiding this comment.
Done in commit 4cd079f. Added comprehensive XML documentation explaining:
- Script structure (InteractConfig, targets)
- All supported actions:
get,post,if,call - Output extraction with XPath, regex, attributes
- Variable substitution
$(Name)syntax - Complete example scripts
Also updated the MCP input schema description with inline documentation.
|
|
||
| return new ExecuteScriptResult | ||
| { | ||
| Success = result.Ok, |
There was a problem hiding this comment.
Again, we can use ExecuteResult of InterationExecutor directly
There was a problem hiding this comment.
Done in commit 4cd079f. ExecuteScriptAsync now returns InteractionResult directly from the InterationExecutor. Deleted ExecuteScriptResult.cs.
…essor, use InteractionResult directly, add comprehensive script documentation Co-authored-by: kerryjiang <456060+kerryjiang@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| /// <param name="inputs">Optional comma-separated key=value pairs for script inputs (e.g., "BaseUrl=https://example.com,Username=user").</param> | ||
| /// <param name="target">Optional target name to execute. If not specified, the default target will be used.</param> | ||
| /// <returns>The InteractionResult containing Ok status, Message, and extracted Outputs.</returns> | ||
| public async Task<InteractionResult> ExecuteScriptAsync( |
There was a problem hiding this comment.
This method doesn't seem required.
There was a problem hiding this comment.
Done in commit 88155fa. Made ExecuteScriptAsync internal (renamed to ExecuteScriptInternalAsync) for testing purposes only.
|
|
||
| return new CallToolResult | ||
| { | ||
| Content = new List<ContentBlock> |
There was a problem hiding this comment.
We better return result.Output to ai agent through CallToolResult
There was a problem hiding this comment.
Done in commit 88155fa. InvokeAsync now returns result.Outputs directly as JSON in the CallToolResult.Content instead of wrapping it in "Success" text.
| { | ||
| Name = "netinteractor_execute_script", | ||
| Description = "Executes a NetInteractor XML script for web automation. Supports GET requests, form POST submissions, conditional logic (if), and calling other targets. Use XPath expressions to extract data from HTML. Variables use $(Name) syntax.", | ||
| InputSchema = JsonDocument.Parse(inputSchemaJson).RootElement |
There was a problem hiding this comment.
should use the return of GetOutputMetadata
There was a problem hiding this comment.
Addressed in commit 88155fa. The Metadata property now returns the output metadata from GetOutputMetadata(). The InputSchema is set on ProtocolTool using JSON schema format.
| /// <summary> | ||
| /// Gets the tool metadata. | ||
| /// </summary> | ||
| public override IReadOnlyList<object> Metadata => Array.Empty<object>(); |
There was a problem hiding this comment.
Should not return empty here, right? OutputMetadata should be returned here?
There was a problem hiding this comment.
Done in commit 88155fa. Metadata now returns the values from GetOutputMetadata() instead of an empty array.
| /// Gets the input metadata for the ExecuteScript tool. | ||
| /// </summary> | ||
| /// <returns>A dictionary describing the input parameters.</returns> | ||
| public static Dictionary<string, ToolParameterMetadata> GetInputMetadata() |
There was a problem hiding this comment.
Done in commit 88155fa. GetInputMetadata() is now private.
| /// Gets the output metadata for the ExecuteScript tool. | ||
| /// </summary> | ||
| /// <returns>A dictionary describing the output properties of InteractionResult.</returns> | ||
| public static Dictionary<string, ToolParameterMetadata> GetOutputMetadata() |
There was a problem hiding this comment.
Done in commit 88155fa. GetOutputMetadata() is now private.
| }; | ||
| } | ||
|
|
||
| private static NameValueCollection ParseInputs(string? inputs) |
There was a problem hiding this comment.
we should need these two methods, right?
There was a problem hiding this comment.
Yes, both ParseInputs and FormatOutputs are needed - they're used internally by the tool. Kept them as private helper methods.
…, populate Metadata with output schema Co-authored-by: kerryjiang <456060+kerryjiang@users.noreply.github.com>
| if (arguments.TryGetValue("script", out var scriptValue) && scriptValue.ValueKind != JsonValueKind.Undefined && scriptValue.ValueKind != JsonValueKind.Null) | ||
| script = scriptValue.GetString(); | ||
| if (arguments.TryGetValue("inputs", out var inputsValue) && inputsValue.ValueKind != JsonValueKind.Undefined && inputsValue.ValueKind != JsonValueKind.Null) | ||
| inputs = inputsValue.GetString(); |
There was a problem hiding this comment.
Done in commit cc007f3. Changed inputs from string? to string[]? and updated the schema to "type": "array", "items": { "type": "string" }. Now reads inputs as an array from JsonValueKind.Array.
| inputs = inputsValue.GetString(); | ||
| if (arguments.TryGetValue("target", out var targetValue) && targetValue.ValueKind != JsonValueKind.Undefined && targetValue.ValueKind != JsonValueKind.Null) | ||
| target = targetValue.GetString(); | ||
| } | ||
|
|
||
| if (string.IsNullOrEmpty(script)) | ||
| { | ||
| return new CallToolResult | ||
| { | ||
| Content = new List<ContentBlock> | ||
| { | ||
| new TextContentBlock { Text = "Error: 'script' parameter is required." } | ||
| }, | ||
| IsError = true | ||
| }; | ||
| } | ||
|
|
||
| var result = await ExecuteScriptInternalAsync(script, inputs, target); | ||
|
|
||
| if (result.Ok) | ||
| { | ||
| // Return the outputs directly as JSON for the AI agent | ||
| var outputsJson = FormatOutputs(result.Outputs); | ||
| return new CallToolResult | ||
| { | ||
| Content = new List<ContentBlock> | ||
| { | ||
| new TextContentBlock { Text = outputsJson } |
There was a problem hiding this comment.
Is there a ContentBlock we can put an object here directly without serializing it into json string?
| { | ||
| Name = "netinteractor_execute_script", | ||
| Description = "Executes a NetInteractor XML script for web automation. Supports GET requests, form POST submissions, conditional logic (if), and calling other targets. Use XPath expressions to extract data from HTML. Variables use $(Name) syntax.", | ||
| InputSchema = JsonDocument.Parse(inputSchemaJson).RootElement |
There was a problem hiding this comment.
Can we put the code into a method? Probably just add back the method GetInputMetadata() and use it directly here. That one has more detailed information about how the script can be used.
There was a problem hiding this comment.
Done in commit cc007f3. Created private GetInputMetadata() method that returns the JSON schema with comprehensive documentation including all supported actions (get, post, if, call), output extraction attributes, and variable substitution syntax.
…extract GetInputMetadata Co-authored-by: kerryjiang <456060+kerryjiang@users.noreply.github.com>
Co-authored-by: kerryjiang <456060+kerryjiang@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| { | ||
| if (arguments.TryGetValue("script", out var scriptValue) && scriptValue.ValueKind != JsonValueKind.Undefined && scriptValue.ValueKind != JsonValueKind.Null) | ||
| script = scriptValue.GetString(); | ||
| if (arguments.TryGetValue("inputs", out var inputsValue) && inputsValue.ValueKind == JsonValueKind.Array) |
There was a problem hiding this comment.
inputs can be JsonElement, all the attributes of it can be used key value string pairs.
There was a problem hiding this comment.
Done in commit 1029f29. Changed inputs to read from a JsonElement object. Schema now uses "type": "object", "additionalProperties": { "type": "string" }. Added ParseInputsFromJsonElement() to extract key-value pairs from the object properties.
| /// <summary> | ||
| /// Gets the input metadata schema as JSON string. | ||
| /// </summary> | ||
| private static string GetInputMetadata() |
There was a problem hiding this comment.
this method can return type JsonElement directly
There was a problem hiding this comment.
Done in commit 1029f29. GetInputMetadata() now returns JsonElement directly using JsonDocument.Parse(schemaJson).RootElement.
| """; | ||
| } | ||
|
|
||
| private static Dictionary<string, ToolParameterMetadata> GetOutputMetadata() |
There was a problem hiding this comment.
return type can be JsonElement directly
There was a problem hiding this comment.
Done in commit 1029f29. GetOutputMetadata() now returns JsonElement directly using JsonDocument.Parse(schemaJson).RootElement.
| Name = "netinteractor_execute_script", | ||
| Description = "Executes a NetInteractor XML script for web automation. Supports GET requests, form POST submissions, conditional logic (if), and calling other targets. Use XPath expressions to extract data from HTML. Variables use $(Name) syntax.", | ||
| InputSchema = JsonDocument.Parse(inputSchemaJson).RootElement | ||
| }; |
There was a problem hiding this comment.
Assign output schema here too
There was a problem hiding this comment.
Done in commit 1029f29. Added OutputSchema = GetOutputMetadata() to the Tool definition.
| { | ||
| "type": "object", | ||
| "properties": { | ||
| "script": { |
There was a problem hiding this comment.
We need more detailed information about all the target types can be used in the script to make sure the AI agent know how to pass the script
There was a problem hiding this comment.
Done in commit 1029f29. Added comprehensive documentation in GetInputMetadata() covering:
- Script structure with InteractConfig and targets
- GET request with all attributes (url, expectedHttpStatusCodes)
- POST form submission with all form identifiers (formIndex, formName, action, clientID) and formValue
- Conditional execution with property and value
- Call target action
- Output extraction with all attributes (name, xpath, attr, regex, isMultipleValue, expectedValue)
- Variable substitution syntax
| /// <summary> | ||
| /// Gets the tool metadata including output schema information. | ||
| /// </summary> | ||
| public override IReadOnlyList<object> Metadata => _outputMetadata; |
There was a problem hiding this comment.
I think we should return empty list here. outputMetadata is not returned from here
There was a problem hiding this comment.
Done in commit 1029f29. Metadata now returns Array.Empty<object>(). Output schema is returned via ProtocolTool.OutputSchema instead.
… from metadata methods, set OutputSchema, return empty Metadata, add detailed target documentation Co-authored-by: kerryjiang <456060+kerryjiang@users.noreply.github.com>
| /// </summary> | ||
| private static JsonElement GetInputMetadata() | ||
| { | ||
| var schemaJson = """ |
There was a problem hiding this comment.
Can you operate JsonElement directly instead of construct a long string making the code look bad.
For the description of script, we better have a way to manage those kind data.
There was a problem hiding this comment.
Done in commits e980c19 and c408d0b. Refactored GetInputMetadata() to build JSON schema using JsonObject programmatically. Script description is now in a separate BuildScriptDescription() method using StringBuilder for better maintainability. Also optimized using JsonSerializer.SerializeToElement() instead of round-trip through string.
| /// </summary> | ||
| private static JsonElement GetOutputMetadata() | ||
| { | ||
| var schemaJson = """ |
There was a problem hiding this comment.
Can you operate JsonElement directly instead of construct a long string making the code look bad.
…tead of string literals Co-authored-by: kerryjiang <456060+kerryjiang@users.noreply.github.com>
Co-authored-by: kerryjiang <456060+kerryjiang@users.noreply.github.com>
| sb.AppendLine("Variables are provided via the 'inputs' parameter as key-value pairs."); | ||
| sb.AppendLine(); | ||
| sb.AppendLine("Example:"); | ||
| sb.AppendLine("<get url='$(BaseUrl)/api/$(Endpoint)?id=$(ItemId)' />"); |
There was a problem hiding this comment.
put them into a file and we load that file from here. If there is any change for the features, we will just need to update that file
There was a problem hiding this comment.
…sier maintenance Co-authored-by: kerryjiang <456060+kerryjiang@users.noreply.github.com>
Co-authored-by: kerryjiang <456060+kerryjiang@users.noreply.github.com>
| { | ||
| // Arrange | ||
| var script = $@"<InteractConfig defaultTarget='Main'> | ||
| <target name='Main'> |
There was a problem hiding this comment.
tests should test against the public method InvokeAsync to get more test coverage
There was a problem hiding this comment.
Done in commit b3caa10. Tests now call InvokeAsync instead of the internal method. Added CreateRequestContext and CreateArguments helpers to construct MCP request contexts. Also added a new test for missing script validation (InvokeAsync_MissingScript_ReturnsError).
…kage Co-authored-by: kerryjiang <456060+kerryjiang@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
ExecuteScriptAsynctool (using InteractionExecutor)GetAsyncandPostAsynctools as they are not neededPR Feedback Addressed (Round 1-5):
PR Feedback Addressed (Round 6):
ScriptDescription.txtPR Feedback Addressed (Round 7):
InvokeAsyncmethod (comment 2808261427)CreateRequestContexthelper method for creating MCP request contextsCreateArgumentshelper method for building tool argumentsInvokeAsync_MissingScript_ReturnsErrortest case for missing script validationMoqpackage for mockingMcpServerTests
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.