feat(automations): add native webhook action - #1733
Draft
nitrobass24 wants to merge 3 commits into
Draft
Conversation
Add a first-class Webhook automation action that POSTs a structured JSON payload to a user-configured HTTP endpoint when an automation rule matches. - Always POST with auto-built JSON payload (torrent name, hash, category, tags, tracker, paths, size, ratio, state, progress, seeds/leeches, rule ID/name, instance ID/name, timestamp) - Optional custom headers for auth (Authorization, X-API-Key, etc.) - 30s timeout, fire-and-forget async execution (like external program) - Per-run cap of 50 webhooks to prevent flooding - Last-rule-wins semantics when multiple rules match - Activity log entries with success/failure outcome - Full validation: URL scheme, empty headers, delete standalone check - Frontend: webhook UI in workflow dialog, badges in rule cards/panels, activity run display Closes #1624
Contributor
WalkthroughAdds a native Webhook automation action: model/types and validation, integrates webhook evaluation into rule processing (last-rule-wins), records dry-run activities, executes HTTP POSTs (capped, async) with activity logging, and updates frontend UI/types for configuring and displaying webhooks. Changes
Sequence DiagramsequenceDiagram
participant Rule as Automation Rule
participant Processor as Rule Processor
participant Service as Automation Service
participant HTTP as HTTP Client
participant Logger as Activity Logger
Rule->>Processor: evaluateConditions(rule, torrent)
Processor->>Processor: check webhook condition (enabled + URL)
alt condition matches
Processor->>Service: enqueue pendingWebhookExec(rule, action, torrent)
end
Service->>Service: executeWebhooksFromAutomation(pendingExecutions)
loop per webhook (deterministic sort + cap)
Service->>HTTP: POST action.URL with JSON payload + headers
HTTP-->>Service: response (status)
alt 2xx
Service->>Logger: logWebhookActivity(success, details)
else
Service->>Logger: logWebhookActivity(failure, details)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
web/src/components/instances/preferences/WorkflowDialog.tsx (3)
1513-1526:⚠️ Potential issue | 🟠 Major
webhookis missing from enabled-action counting.Line [1513] builds
enabledActionsCountbut omitsformState.webhookEnabled. Webhook-only rules are treated as “no action” (blocks dry-run/save and keeps wrong action-picker state).Suggested fix
const enabledActionsCount = [ formState.speedLimitsEnabled, formState.shareLimitsEnabled, formState.pauseEnabled, formState.resumeEnabled, formState.recheckEnabled, formState.reannounceEnabled, formState.autoManagementEnabled, formState.deleteEnabled, formState.tagEnabled, formState.categoryEnabled, formState.moveEnabled, formState.externalProgramEnabled, + formState.webhookEnabled, ].filter(Boolean).length🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/instances/preferences/WorkflowDialog.tsx` around lines 1513 - 1526, The enabledActionsCount array used to compute how many actions are turned on (symbol: enabledActionsCount) omits the webhook flag, so rules that only enable webhooks are treated as having no actions; include formState.webhookEnabled in the array alongside the other flags (e.g., formState.deleteEnabled, formState.externalProgramEnabled) so the count and subsequent logic (dry-run/save gating and action-picker state) correctly recognize webhook-only rules.
990-1003:⚠️ Potential issue | 🟠 MajorWebhook condition is not hydrated into shared
actionCondition.Lines [990-1003] don’t include
conditions.webhook?.condition, so editing a webhook-only rule can silently drop its condition on re-save.Suggested fix
actionCondition = conditions.speedLimits?.condition ?? conditions.shareLimits?.condition ?? conditions.pause?.condition ?? conditions.resume?.condition ?? conditions.recheck?.condition ?? conditions.reannounce?.condition ?? conditions.autoManagement?.condition ?? conditions.delete?.condition ?? conditions.tags?.[0]?.condition ?? conditions.tag?.condition ?? conditions.category?.condition ?? conditions.move?.condition ?? conditions.externalProgram?.condition + ?? conditions.webhook?.condition ?? null🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/instances/preferences/WorkflowDialog.tsx` around lines 990 - 1003, The shared actionCondition assignment in WorkflowDialog.tsx omits conditions.webhook?.condition, causing webhook-only rules to lose their condition on save; update the actionCondition expression (the variable named actionCondition) to include conditions.webhook?.condition in the nullish-coalescing chain (placing it alongside the other conditions like conditions.tags, conditions.tag, conditions.category, etc.) so webhook conditions are properly hydrated before save.
203-215:⚠️ Potential issue | 🟡 MinorDry-run summary omits webhook action text.
webhookis in dry-run labels, but not in the impacted-summary cases (Lines [203-215]), so webhook events fall back to generic “Dry-run completed”.Suggested fix
case "external_program": + case "webhook": case "deleted_ratio": case "deleted_seeding": case "deleted_unregistered": case "deleted_condition": {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/instances/preferences/WorkflowDialog.tsx` around lines 203 - 215, The dry-run summary switch in WorkflowDialog.tsx currently groups several status strings (cases "paused", "resumed", "rechecked", ... "deleted_condition") to return an impacted-count string but omits "webhook", causing webhook events to fall back to the generic message; update the switch that builds the impacted-summary (the case block using details?.count) to include "webhook" among the case labels so webhook dry-run events return the same `${count} torrent(s) impacted` output (using the existing details?.count logic).internal/services/automations/service.go (1)
2209-2210:⚠️ Potential issue | 🟠 MajorCap a deterministic webhook set before dry-run/live split.
webhookExecutionsis built while ranging overstates, which is a Go map. Once the live path trims that slice to 50, the surviving executions are effectively random from run to run. The dry-run path then logs all matches instead of the capped subset, so preview diverges from live behavior as soon as the limit is exceeded. Apply the same cap to a stable order before both dry-run activity creation and live dispatch.Also applies to: 2619-2628, 5473-5483, 6094-6100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/services/automations/service.go` around lines 2209 - 2210, The webhookExecutions slice is populated by iterating over the map variable states (using torrentByHash[hash]), causing nondeterministic order and making the live path's truncation to 50 effectively random and the dry-run preview inconsistent; fix by collecting executions into webhookExecutions, then sort them deterministically (e.g., by a stable key such as hash, webhook ID, or timestamp) and apply the same cap (slice to 50) to the sorted list before doing dry-run activity creation or live dispatch so both paths operate on the identical deterministic subset; apply the same pattern to the other occurrences that build webhookExecutions (the blocks mentioned around the other ranges).
🧹 Nitpick comments (1)
internal/api/handlers/automations.go (1)
1052-1054: Inconsistent gating onEnabledflag for regex validation.Unlike
conditionTreesForValidation(lines 641-643) which gates onconditions.Webhook.Enabled, this regex validation block only checksconditions.Webhook != nil. This means regex patterns in disabled webhooks will be validated, while their condition trees won't be included for grouping-ID validation.For consistency with other validation paths and to avoid validating regexes in disabled actions, consider adding the
Enabledcheck:🔧 Suggested fix for consistency
- if conditions.Webhook != nil { + if conditions.Webhook != nil && conditions.Webhook.Enabled { validateConditionRegex(conditions.Webhook.Condition, "/conditions/webhook/condition", &result) }Note: The current behavior is arguably defensive (catching invalid regexes before enabling), so this is optional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/handlers/automations.go` around lines 1052 - 1054, The regex validation for webhook conditions currently runs if conditions.Webhook != nil but other validation paths (e.g., conditionTreesForValidation) gate on conditions.Webhook.Enabled; update the check around the validateConditionRegex call so it only runs when conditions.Webhook is non-nil and conditions.Webhook.Enabled is true (i.e., mirror the gating used for conditionTreesForValidation) to keep regex validation consistent with disabled actions; locate the call to validateConditionRegex and the conditions.Webhook reference and add the Enabled check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/models/automation.go`:
- Around line 1015-1041: AutomationStore.Create and AutomationStore.Update
currently only call ExternalProgram.Validate() and therefore skip the new
WebhookAction.Validate(); update both Create and Update to invoke
automation.Conditions.Webhook.Validate() (or call WebhookAction.Validate() on
the stored object) when a webhook action is present, and if it returns an error
wrap it with contextual text (e.g., "invalid webhook action: %w") and return it
so invalid webhook URLs/headers are rejected at store write time; ensure you
perform the same check in both AutomationStore.Create and AutomationStore.Update
alongside the existing ExternalProgram.Validate() call.
In `@internal/services/automations/service.go`:
- Around line 6089-6114: The instance lookup currently uses context.Background()
so it ignores the caller timeout; update the function signature
executeWebhooksFromAutomation(_ context.Context, ...) to accept and use a real
context (e.g., ctx context.Context) and call s.instanceStore.Get(ctx,
instanceID) instead of context.Background(); keep the existing nil check for
s.instanceStore and assignment to instanceName but ensure you pass ctx into
instanceStore.Get so the lookup respects the caller timeout (no other behavioral
changes).
In `@web/src/components/instances/preferences/WorkflowDialog.tsx`:
- Line 1424: The headers array is only filtered for empty keys but not
normalized, so whitespace in header names/values can slip into the submitted
payload; in the WorkflowDialog serialization where you build headers from
input.exprWebhookHeaders, first map each header to trimmed key/value (e.g.,
transform each h to { key: h.key.trim(), value: h.value.trim() }), then filter
out entries with empty keys, and assign that result to headers so both keys and
values are normalized before submit.
- Around line 2004-2006: The frontend currently uses new
URL(submitState.exprWebhookUrl.trim()) in WorkflowDialog.tsx which accepts
non-HTTP schemes; update the validation to parse the URL (using new URL(...))
then explicitly verify that url.protocol is "http:" or "https:" and that
url.host (or url.hostname) is non-empty before accepting it, and surface a
validation error if those checks fail so the client-side contract matches the
backend's http/https + non-empty host requirement for
submitState.exprWebhookUrl.
---
Outside diff comments:
In `@internal/services/automations/service.go`:
- Around line 2209-2210: The webhookExecutions slice is populated by iterating
over the map variable states (using torrentByHash[hash]), causing
nondeterministic order and making the live path's truncation to 50 effectively
random and the dry-run preview inconsistent; fix by collecting executions into
webhookExecutions, then sort them deterministically (e.g., by a stable key such
as hash, webhook ID, or timestamp) and apply the same cap (slice to 50) to the
sorted list before doing dry-run activity creation or live dispatch so both
paths operate on the identical deterministic subset; apply the same pattern to
the other occurrences that build webhookExecutions (the blocks mentioned around
the other ranges).
In `@web/src/components/instances/preferences/WorkflowDialog.tsx`:
- Around line 1513-1526: The enabledActionsCount array used to compute how many
actions are turned on (symbol: enabledActionsCount) omits the webhook flag, so
rules that only enable webhooks are treated as having no actions; include
formState.webhookEnabled in the array alongside the other flags (e.g.,
formState.deleteEnabled, formState.externalProgramEnabled) so the count and
subsequent logic (dry-run/save gating and action-picker state) correctly
recognize webhook-only rules.
- Around line 990-1003: The shared actionCondition assignment in
WorkflowDialog.tsx omits conditions.webhook?.condition, causing webhook-only
rules to lose their condition on save; update the actionCondition expression
(the variable named actionCondition) to include conditions.webhook?.condition in
the nullish-coalescing chain (placing it alongside the other conditions like
conditions.tags, conditions.tag, conditions.category, etc.) so webhook
conditions are properly hydrated before save.
- Around line 203-215: The dry-run summary switch in WorkflowDialog.tsx
currently groups several status strings (cases "paused", "resumed", "rechecked",
... "deleted_condition") to return an impacted-count string but omits "webhook",
causing webhook events to fall back to the generic message; update the switch
that builds the impacted-summary (the case block using details?.count) to
include "webhook" among the case labels so webhook dry-run events return the
same `${count} torrent(s) impacted` output (using the existing details?.count
logic).
---
Nitpick comments:
In `@internal/api/handlers/automations.go`:
- Around line 1052-1054: The regex validation for webhook conditions currently
runs if conditions.Webhook != nil but other validation paths (e.g.,
conditionTreesForValidation) gate on conditions.Webhook.Enabled; update the
check around the validateConditionRegex call so it only runs when
conditions.Webhook is non-nil and conditions.Webhook.Enabled is true (i.e.,
mirror the gating used for conditionTreesForValidation) to keep regex validation
consistent with disabled actions; locate the call to validateConditionRegex and
the conditions.Webhook reference and add the Enabled check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2ced714d-9055-4d94-937e-dc9a852991c5
📒 Files selected for processing (14)
internal/api/handlers/automations.gointernal/api/handlers/automations_validation_test.gointernal/models/automation.gointernal/models/automation_activity.gointernal/services/automations/grouping.gointernal/services/automations/processor.gointernal/services/automations/processor_test.gointernal/services/automations/service.gointernal/services/automations/service_test.goweb/src/components/instances/preferences/AutomationActivityRunDialog.tsxweb/src/components/instances/preferences/WorkflowDialog.tsxweb/src/components/instances/preferences/WorkflowsOverview.tsxweb/src/components/instances/preferences/WorkflowsPanel.tsxweb/src/types/index.ts
- Add Webhook.Validate() to AutomationStore.Create and Update - Use caller context for instance lookup in executeWebhooksFromAutomation - Trim header keys/values before submit in WorkflowDialog - Validate http/https scheme and non-empty host in frontend URL check - Sort webhook executions by hash for deterministic truncation - Include webhookEnabled in enabledActionsCount array - Add webhook?.condition to actionCondition hydration chain - Add "webhook" case to dry-run summary switch - Gate webhook regex validation on Enabled flag
Contributor
Author
|
@CodeRabbit review |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/models/automation.go`:
- Around line 1042-1045: The current loop over a.Headers only checks for empty
keys; replace that with a validation that trims the key and enforces HTTP token
syntax: implement a helper isValidHTTPHeaderName(name string) that returns false
for names containing spaces, control characters, or characters outside token set
(ALPHA / DIGIT / "!#$%&'*+-.^_`|~"), call it from the for i, h := range
a.Headers loop after strings.TrimSpace(h.Key), and if validation fails return
fmt.Errorf("webhook header %d has invalid name %q", i, key) so
invalid/header-with-spaces or control chars are rejected early.
In `@internal/services/automations/service.go`:
- Around line 6194-6198: The code currently persists remote response bodies into
AutomationActivity.Reason in the else branch of the webhook call; update the
failure logging in the else branch (the s.logWebhookActivity call that uses
models.ActivityOutcomeFailed and fmt.Sprintf("HTTP %d: %s", ...)) to stop
including the response body and instead record only the HTTP status (e.g.,
fmt.Sprintf("HTTP %d", resp.StatusCode)); this change should be applied where
s.logWebhookActivity is invoked for failures using instanceID, torrent, ruleID,
ruleName and models.ActivityOutcomeFailed so no remote body/PII is stored.
- Around line 5473-5483: The dry-run activity currently reports all
webhookExecutions instead of mirroring the live execution's sorting and
truncation to maxWebhooksPerRun; modify the block that builds uniqueHashes
(around webhookExecutions, dedupeHashes, createActivity,
buildRunItemsFromHashes) to first sort webhookExecutions the same way the live
run does and then truncate the list to maxWebhooksPerRun before extracting
hashes and deduping, so the activity count and buildRunItemsFromHashes output
match real execution.
- Line 571: The webhookClient currently uses a default http.Client which follows
redirects; update the webhookClient instantiation (webhookClient:
&http.Client{...}) to disable automatic redirects by setting CheckRedirect to a
function that returns http.ErrUseLastResponse so 3xx responses are returned to
the caller instead of being followed; keep the existing Timeout field intact.
In `@web/src/components/instances/preferences/WorkflowDialog.tsx`:
- Around line 1741-1744: Create a shared URL validation helper (e.g.,
validateWebhookUrl or parseWebhookUrl) that runs the same checks as the
submit-time new URL(...) guard (trimmed non-empty, valid URL, allowed scheme and
host extraction) and return parsed parts or an error; replace the inline
presence check on dryRunInput.exprWebhookUrl and call this helper from the
dry-run path, from live-preview/enable-preview handlers, and from the
save/submit flow so all three paths gate on the same validation before building
webhook payloads to avoid premature backend validation errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 75eaee51-91f3-4fbc-9ba1-5ac655654192
📒 Files selected for processing (4)
internal/api/handlers/automations.gointernal/models/automation.gointernal/services/automations/service.goweb/src/components/instances/preferences/WorkflowDialog.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/api/handlers/automations.go
- Stop persisting remote response bodies in activity log, record HTTP status code only - Mirror live sort+truncation in dry-run webhook activity recording - Disable automatic redirect following on webhook HTTP client
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/services/automations/service.go (1)
3663-3664: Confirm whether webhook actions should participate inNotifysummaries.
notifyAutomationSummary(...)still runs synchronously at the end ofapplyRulesForInstance, but webhook outcomes are only recorded later from background goroutines. As written, webhook-only rules produce no summary notification at all, and mixed runs omit webhook failures from that summary. If that is intentional, ignore this; otherwise the summary needs to move behind webhook completion or pre-account for webhook dispatches.Also applies to: 6097-6137
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/services/automations/service.go` around lines 3663 - 3664, The current flow calls s.executeWebhooksFromAutomation(...) asynchronously so webhook results aren't included in notifyAutomationSummary(...) called at the end of applyRulesForInstance, causing webhook-only rules to be omitted and webhook failures to be missing from mixed summaries; either (A) make s.executeWebhooksFromAutomation join/wait for webhook execution completion (or return a future/result channel) and invoke notifyAutomationSummary(...) after those results are available, or (B) change applyRulesForInstance to pre-record webhook dispatch entries (with pending state) into the summary and have the background webhook goroutines update/patch the same summary when they finish; update functions s.executeWebhooksFromAutomation, applyRulesForInstance, and notifyAutomationSummary to propagate and consume webhook outcome information accordingly so summaries include webhook results.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/services/automations/service.go`:
- Around line 6214-6223: The async activity insert using s.activityStore.Create
(creating a models.AutomationActivity with InstanceID, Hash, TorrentName,
Action, RuleID, RuleName, Outcome, Reason) is unbounded; wrap the Create call in
a short timeout context (e.g., context.WithTimeout of a few seconds) and use
that context for the Create so the goroutine can’t block indefinitely on a slow
store; ensure you cancel the context after use and keep this change only on the
fire-and-forget path so normal synchronous behavior is unchanged.
---
Nitpick comments:
In `@internal/services/automations/service.go`:
- Around line 3663-3664: The current flow calls
s.executeWebhooksFromAutomation(...) asynchronously so webhook results aren't
included in notifyAutomationSummary(...) called at the end of
applyRulesForInstance, causing webhook-only rules to be omitted and webhook
failures to be missing from mixed summaries; either (A) make
s.executeWebhooksFromAutomation join/wait for webhook execution completion (or
return a future/result channel) and invoke notifyAutomationSummary(...) after
those results are available, or (B) change applyRulesForInstance to pre-record
webhook dispatch entries (with pending state) into the summary and have the
background webhook goroutines update/patch the same summary when they finish;
update functions s.executeWebhooksFromAutomation, applyRulesForInstance, and
notifyAutomationSummary to propagate and consume webhook outcome information
accordingly so summaries include webhook results.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 86eaa007-cf75-4415-b862-a14b877b8dee
📒 Files selected for processing (1)
internal/services/automations/service.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
conditionsJSON columnDesign decisions
webhook://notification targets in the notifications package; this adds a per-workflow automation action in an entirely separate packageChanges
Backend
internal/models/automation.go—WebhookAction,WebhookHeaderstructs withValidate(), added toActionConditionsandIsEmpty()internal/models/automation_activity.go—ActivityActionWebhookconstantinternal/services/automations/processor.go— Webhook fields intorrentDesiredState, condition evaluation, stats trackinginternal/services/automations/service.go—webhookPayloadstruct,executeWebhooksFromAutomation(with 50/run cap), HTTP execution with activity logginginternal/services/automations/grouping.go—conditionFromWebhookActioninternal/api/handlers/automations.go— Validation: delete standalone check, URL/header validation, condition field/regex checksFrontend
web/src/types/index.ts—WebhookAction,WebhookHeaderinterfacesweb/src/components/instances/preferences/WorkflowDialog.tsx— Webhook action type, form state, UI section (URL + dynamic headers), validationweb/src/components/instances/preferences/WorkflowsOverview.tsx— Webhook badge, color, labelweb/src/components/instances/preferences/WorkflowsPanel.tsx— Webhook badge with URL tooltipweb/src/components/instances/preferences/AutomationActivityRunDialog.tsx— Webhook activity labelTests
WebhookAction.Validate()— 8 test cases (nil, disabled, valid URLs, bad scheme, empty headers)TestDeleteStandaloneWithWebhook— delete + webhook rejectedTest plan
make lintpasses (Go lint clean; frontend lint warnings are pre-existing)make testpasses (-race -count=3)make buildsucceedsCloses #1624
Summary by CodeRabbit
New Features
Bug Fixes / Validation
Behavior