Add webhook action for HTTP request integration#309
Conversation
Integrates a new "webhook" action, enabling configurable HTTP requests with customizable methods, content types, and payloads through runtime parameters.
Improves comments and function-level documentation for the Webhook action, clarifying its configuration, runtime parameter handling, and operational behavior. Adds detailed explanations for methods, payload templates, and HTTP request construction.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new webhook action that enables LocalAGI agents to send HTTP requests to external services. The action supports configurable URL, HTTP method, content type, and payload templates with runtime payload substitution.
Key changes:
- Added a new
WebhookActionstruct with support for GET, POST, PUT, and DELETE requests - Implemented payload templating with
{{payload}}placeholder replacement - Integrated the webhook action into the main actions registry and configuration system
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| services/actions/webhook.go | Complete implementation of the webhook action with HTTP client functionality, payload templating, and configuration metadata |
| services/actions.go | Registration of the new webhook action in the actions system, including constants, available actions list, and factory function |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| {Value: "text/plain", Label: "text/plain"}, | ||
| {Value: "application/x-www-form-urlencoded", Label: "application/x-www-form-urlencoded"}, | ||
| }, | ||
| Required: true, |
There was a problem hiding this comment.
The contentType field is marked as required, but many valid HTTP requests don't need a Content-Type header (e.g., GET requests with no body, or requests where the server doesn't care about the content type). Consider making this field optional to provide more flexibility.
| Required: true, | |
| Required: false, |
| // Use a new http.Client with default settings. Consider configuring timeouts | ||
| // at the caller level via the context, or wiring a custom client if needed. | ||
| client := &http.Client{} | ||
| resp, err := client.Do(req) |
There was a problem hiding this comment.
Creating a new HTTP client for each request is inefficient and prevents connection reuse. Consider using a shared http.Client instance or http.DefaultClient to enable connection pooling and improve performance.
| // Use a new http.Client with default settings. Consider configuring timeouts | |
| // at the caller level via the context, or wiring a custom client if needed. | |
| client := &http.Client{} | |
| resp, err := client.Do(req) | |
| // Use the shared http.DefaultClient to enable connection reuse and pooling. | |
| // Consider configuring timeouts at the caller level via the context, or wiring a custom client if needed. | |
| resp, err := http.DefaultClient.Do(req) |
| payload = strings.ReplaceAll(a.payloadTemplate, "{{payload}}", in.Payload) | ||
| if payload == a.payloadTemplate && in.Payload != "" { | ||
| // If no placeholder found, fallback to template or payload alone |
There was a problem hiding this comment.
The logic for handling payload template replacement is confusing. When no placeholder is found but payload is provided, the runtime payload is ignored. This behavior should be documented more clearly or reconsidered - users might expect their runtime payload to be used even without a template.
| payload = strings.ReplaceAll(a.payloadTemplate, "{{payload}}", in.Payload) | |
| if payload == a.payloadTemplate && in.Payload != "" { | |
| // If no placeholder found, fallback to template or payload alone | |
| if strings.Contains(a.payloadTemplate, "{{payload}}") { | |
| payload = strings.ReplaceAll(a.payloadTemplate, "{{payload}}", in.Payload) | |
| } else if in.Payload != "" { | |
| // Template does not contain placeholder, but runtime payload is provided. | |
| // Use the runtime payload as the body. | |
| payload = in.Payload | |
| } else { | |
| // No placeholder and no runtime payload; use template as-is. |
There was a problem hiding this comment.
This is intended. The agent should not be able to add a payload if no template is configured.
Note that the placeholder is not optimal at the moment. It allows injections.
E.g. if the template is something like {"message": "{{payload}}"} and the payload is ", "newkey": "somethingand the resulting JSON that is sent would be {"message": "", "newkey": "something"}.
Better ideas are welcome.
This PR adds a new webhook action that enables LocalAGI agents to send HTTP requests to external services.
The action requires static configuration:
Runtime parameters accept only the dynamic payload value, keeping the action simple.
This action enables agents to:
Note, I currently only tested it in the "actions playground", I didn't test it inside an agent yet.