feat(automation): trigger external programs automatically via automation rules#1284
Conversation
WalkthroughAdds a centralized externalprograms.Service and integrates external program execution throughout automations, cross‑seed, API handlers, server DI, models, frontend, docs, and tests; implements path allowlisting, terminal/direct command builders, async execution, activity logging, validation, and force‑delete automation cleanup. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
participant RuleProcessor as RuleProcessor
participant AutomationService as AutomationService
participant ExternalProgramService as ExternalProgramService
participant ProgramStore as ProgramStore
participant ActivityStore as ActivityStore
participant OS as OS
end
RuleProcessor->>AutomationService: collect pendingProgramExec(hash, programID, ruleID, ruleName)
AutomationService->>ExternalProgramService: Execute(ctx, {ProgramID, Torrent, InstanceID, RuleID, RuleName})
ExternalProgramService->>ProgramStore: fetch program config (if needed)
ExternalProgramService->>ExternalProgramService: IsPathAllowed & buildCommand(args)
ExternalProgramService->>OS: start process (async)
ExternalProgramService->>ActivityStore: logActivity(result)
ExternalProgramService-->>AutomationService: return ExecuteResult (init/status)
sequenceDiagram
rect rgba(200,255,200,0.5)
participant CrossSeedService as CrossSeedService
participant ExternalProgramService2 as ExternalProgramService
participant ProgramStore2 as ProgramStore
participant ActivityStore2 as ActivityStore
participant OS2 as OS
end
CrossSeedService->>ExternalProgramService2: Execute(ctx, {ProgramID, Torrent, InstanceID})
ExternalProgramService2->>ProgramStore2: resolve program
ExternalProgramService2->>ExternalProgramService2: validate & buildCommand
ExternalProgramService2->>OS2: execute async (terminal/direct)
ExternalProgramService2->>ActivityStore2: logActivity(result)
ExternalProgramService2-->>CrossSeedService: ExecuteResult
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/services/crossseed/service.go (1)
8337-8395: Fire‑and‑forget execution is still tied to a cancelable context.The goroutine passes the caller
ctxintoExecute. If the request/automation run is canceled,exec.CommandContextwill terminate the external program, which conflicts with the “fire‑and‑forget” intent. Consider decoupling cancellation while still preserving values.💡 Proposed fix: detach cancellation while keeping context values
go func() { + execCtx := context.Background() + if ctx != nil { + execCtx = context.WithoutCancel(ctx) + } // Get torrent data from sync manager targetTorrent, found, err := s.syncManager.HasTorrentByAnyHash(context.Background(), instanceID, []string{torrentHash}) if err != nil { log.Error().Err(err).Int("instanceId", instanceID).Str("torrentHash", torrentHash).Msg("Failed to get torrent for external program execution") return } @@ // Execute using the shared external programs service - result := s.externalProgramService.Execute(ctx, externalprograms.ExecuteRequest{ + result := s.externalProgramService.Execute(execCtx, externalprograms.ExecuteRequest{ ProgramID: programID, Torrent: targetTorrent, InstanceID: instanceID, })
🤖 Fix all issues with AI agents
In `@internal/api/handlers/external_programs.go`:
- Around line 86-89: Trim req.Name before validating and storing to avoid
whitespace-only or duplicate names; specifically, call strings.TrimSpace on
req.Name prior to invoking h.validateProgramInput(req.Name, req.Path) and before
any persistence/lookup logic that uses req.Name (the same change should be
applied at both places referenced around validateProgramInput and the later
save/lookup block). Ensure you only trim once and propagate the trimmed value
for validation, error messages, and storage so the saved Program name is
normalized.
🧹 Nitpick comments (3)
web/src/components/instances/preferences/WorkflowsPanel.tsx (2)
414-414: Consider breaking up the long ternary for readability.This single-line nested ternary is functional but quite dense. While it works, extracting it to a helper function or using a mapping object would improve maintainability.
💡 Optional: Extract delete mode description
// Helper function outside component function getDeleteModeDescription(mode?: string): string { const descriptions: Record<string, string> = { deleteWithFilesPreserveCrossSeeds: "Delete with files (preserve cross-seeds)", deleteWithFilesIncludeCrossSeeds: "Delete with files (include cross-seeds)", deleteWithFiles: "Delete with files", } return descriptions[mode ?? ""] ?? "Delete (keep files)" } // Usage in tooltip: <p>{getDeleteModeDescription(conditions.delete.mode)}</p>
451-465: Consider displaying program name instead of just program ID.The tooltip currently shows
Program ID: {programId}which is technical and not user-friendly. If the program name is available (similar to howprogramNameis included inAutomationActivity.details), it would provide better UX.However, if the program name isn't readily available in this context without an additional API call, the current implementation is acceptable.
internal/services/automations/service.go (1)
2451-2513: Consider bounding external program concurrency.This spawns one goroutine per matching torrent via a loop, with each goroutine internally spawning another to execute the program. For large match sets, this can cause resource exhaustion on the host. The external program service itself has no rate-limiting or worker pool. Consider adding a semaphore or small worker pool to cap concurrent process executions.
0rkag
force-pushed
the
feat/execute-command-automation-action
branch
from
aaaee73 to
9657a6e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/services/crossseed/service.go (1)
8336-8395: Guard against nil contexts beforecontext.WithoutCancel. The stdlib implementation ofcontext.WithoutCancel()panics if passed a nil parent context. While the current caller passes a context from the function parameter (unlikely to be nil), defensive nil-normalization is a reasonable safeguard and aligns with the pattern already used elsewhere in this file.♻️ Suggested defensive change
func (s *Service) executeExternalProgram(ctx context.Context, instanceID int, torrentHash string) { if s.externalProgramService == nil { return } + if ctx == nil { + ctx = context.Background() + } // Get current settings to check if external program is configured settings, err := s.GetAutomationSettings(ctx)
0rkag
force-pushed
the
feat/execute-command-automation-action
branch
from
9657a6e to
53ab062
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@internal/services/externalprograms/service.go`:
- Around line 115-196: The Execute method can be called with a nil Service
receiver when req.Program is preloaded, causing panics later in executeProgram
(calls to s.IsPathAllowed, s.logActivity, s.buildCommand, s.executeAsync); add a
nil guard at the start of Execute (e.g. if s == nil { return
FailureResult(errors.New("external program service not initialized")) }) so the
method fails gracefully before any use of s or before calling executeProgram.
🧹 Nitpick comments (1)
web/src/components/instances/preferences/WorkflowsPanel.tsx (1)
457-471: Consider displaying the program name instead of the numeric ID for better UX.The badge implementation follows existing patterns correctly. However, the tooltip shows
Program ID: {programId}which is a numeric identifier. Other badges like Category and Tag display human-readable values.If the program name is available (or could be fetched/passed), showing it would improve clarity for users who may not recall which ID maps to which program.
💡 Example improvement (if program name is available)
<TooltipContent> - <p>Program ID: {conditions.externalProgram.programId}</p> + <p>Program: {conditions.externalProgram.programName ?? `ID ${conditions.externalProgram.programId}`}</p> </TooltipContent>This would require the
Automationtype to includeprogramNameor a lookup mechanism.
0rkag
force-pushed
the
feat/execute-command-automation-action
branch
from
53ab062 to
d34459d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/services/crossseed/service.go (1)
8337-8396: Add nil context guard to prevent panic oncontext.WithoutCancel.
context.WithoutCancel(nil)will panic in Go 1.21+. WhileGetAutomationSettingshas a defensive nil check, it only protects that method's local context variable; the goroutine captures the originalctxparameter unchanged. Since other methods in this file already defensively guard against nil contexts, add the same pattern here:func (s *Service) executeExternalProgram(ctx context.Context, instanceID int, torrentHash string) { if s.externalProgramService == nil { return } + if ctx == nil { + ctx = context.Background() + } // Get current settings to check if external program is configured settings, err := s.GetAutomationSettings(ctx)This aligns with the defensive checks in
GetAutomationSettings,GetSearchSettings,PatchSearchSettings, andReconcileInterruptedRunsin the same file.
🤖 Fix all issues with AI agents
In `@internal/services/externalprograms/service_test.go`:
- Around line 1163-1167: The test "unknown terminal uses LookPath" assumes the
Unix shell "sh" exists and can fail on Windows; update the test in
service_test.go to be OS-aware by either skipping the assertion on Windows (use
runtime.GOOS or t.Skip on windows) or by choosing a platform-agnostic command
lookup strategy; specifically, modify the t.Run block that calls
isTerminalAvailable("sh") to detect Windows and skip or to use a conditional
command name so the assertion only runs on non-Windows platforms.
♻️ Duplicate comments (1)
internal/services/externalprograms/service.go (1)
115-139: Guard against nil receiver when Program is preloaded.
Calling(*Service)(nil).Executewithreq.Programpanics inexecuteProgram(nil receiver). This was flagged previously and still applies.🛡️ Fix: fail fast on nil service
func (s *Service) Execute(ctx context.Context, req ExecuteRequest) ExecuteResult { + if s == nil { + return FailureResult(errors.New("external program service not initialized")) + } // Validate request first if err := req.Validate(); err != nil { return FailureResult(err) }
🧹 Nitpick comments (1)
web/src/components/instances/preferences/WorkflowsOverview.tsx (1)
1308-1310: Consider consolidating program name display.The program name appears twice: once in the summary (lines 1087-1088 via
formatExternalProgramSummary) and again here as "Program: {programName}". This might be intentional for clarity, but if space is a concern, you could remove one occurrence.
| t.Run("unknown terminal uses LookPath", func(t *testing.T) { | ||
| // Test with a command that definitely exists | ||
| result := isTerminalAvailable("sh") | ||
| assert.True(t, result, "sh should be available on all Unix systems") | ||
| }) |
There was a problem hiding this comment.
Avoid assuming sh exists on Windows.
This test can fail on Windows runners.
🧪 Safer OS-conditional assertion
- t.Run("unknown terminal uses LookPath", func(t *testing.T) {
- // Test with a command that definitely exists
- result := isTerminalAvailable("sh")
- assert.True(t, result, "sh should be available on all Unix systems")
- })
+ t.Run("unknown terminal uses LookPath", func(t *testing.T) {
+ if runtime.GOOS == "windows" {
+ result := isTerminalAvailable("cmd")
+ assert.True(t, result, "cmd should be available on Windows")
+ return
+ }
+ result := isTerminalAvailable("sh")
+ assert.True(t, result, "sh should be available on all Unix systems")
+ })🤖 Prompt for AI Agents
In `@internal/services/externalprograms/service_test.go` around lines 1163 - 1167,
The test "unknown terminal uses LookPath" assumes the Unix shell "sh" exists and
can fail on Windows; update the test in service_test.go to be OS-aware by either
skipping the assertion on Windows (use runtime.GOOS or t.Skip on windows) or by
choosing a platform-agnostic command lookup strategy; specifically, modify the
t.Run block that calls isTerminalAvailable("sh") to detect Windows and skip or
to use a conditional command name so the assertion only runs on non-Windows
platforms.
20bb6ae to
7edfcfd
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/lib/api.ts (1)
566-569: MissingextractErrorMessagebreaks builds.
this.extractErrorMessage(...)is still called, but the method no longer exists in the class. This will fail TypeScript compilation.🐛 Proposed fix (restore a wrapper)
class ApiClient { private async request<T>( endpoint: string, options?: RequestInit ): Promise<T> { @@ private async extractErrorData(response: Response): Promise<{ message: string; data?: any }> { @@ } + + private async extractErrorMessage(response: Response): Promise<string> { + const { message } = await this.extractErrorData(response) + return message + }Also applies to: 1371-1374
🤖 Fix all issues with AI agents
In `@internal/services/externalprograms/service.go`:
- Around line 153-160: When a program is disabled the code currently returns
FailureResult without creating an activity entry; before returning in the branch
that checks program.Enabled, create a matching activity/log entry (using the
same activity API used elsewhere in this file — e.g.,
ActivityService.CreateActivity or recordActivity) that records program.ID and
program.Name, marks the outcome as skipped/failed, and includes context (reason
"program is disabled") and the same actor/timestamp semantics as other activity
entries; then return FailureResult(errors.New("program is disabled")) as before.
In `@web/src/components/instances/preferences/WorkflowDialog.tsx`:
- Around line 278-283: The empty-state copy is misleading because useQuery
currently selects only enabled programs via the select filter on
externalPrograms (queryKey ["externalPrograms"], queryFn
api.listExternalPrograms(), enabled open), so the UI can say “No external
programs configured” even when disabled programs exist; either remove the select
filter so externalPrograms contains the full list (and filter later in the
render), or keep the enabled-only select but update the empty-state text in
WorkflowDialog to say something like “No enabled external programs configured”
and add a short hint (e.g., “Disabled programs exist — enable them in settings”)
or a link to manage programs so users understand disabled programs may be
present. Ensure changes reference externalPrograms, useQuery, and
api.listExternalPrograms.
♻️ Duplicate comments (2)
internal/services/externalprograms/service.go (1)
116-140: Guard against a nilServicewhenProgramis preloaded.
Executecan be invoked on a nil receiver whenreq.Programis provided, which bypasses the current nil check and then panics inexecuteProgram/IsPathAllowed/logActivity. Add a top-level guard before dereferencings.🔧 Proposed fix
func (s *Service) Execute(ctx context.Context, req ExecuteRequest) ExecuteResult { + if s == nil { + return FailureResult(errors.New("external program service not initialized")) + } // Validate request first if err := req.Validate(); err != nil { return FailureResult(err) }internal/api/handlers/external_programs.go (1)
43-61: Trimnamebefore validation to prevent whitespace-only names.The
pathis trimmed (both in callers and here), butnameis checked for emptiness without trimming first. A name consisting only of whitespace would pass thename == ""check but create confusing entries.🛠️ Suggested fix
func (h *ExternalProgramsHandler) validateProgramInput(name, path string) (string, int, bool) { + name = strings.TrimSpace(name) if name == "" { return "Name is required", http.StatusBadRequest, false }
🧹 Nitpick comments (2)
internal/services/automations/service.go (1)
2460-2533: Consider bounding external program launches.
executeExternalProgramsFromAutomationspawns a goroutine per execution, andExternalProgramService.Executespins another goroutine internally. Large rule matches could fan out heavily. Consider callingExecutedirectly (it’s already fire‑and‑forget) or introducing a small worker pool/rate limit.internal/api/handlers/external_programs.go (1)
189-221: Consider logging a warning whenautomationStoreis nil.If
automationStoreisnil, the reference check is silently skipped and the program is deleted without clearing any automation references. While this defensive approach allows the delete to proceed, it could leave danglingprogramIdreferences in automations if the store is accidentally unset.Consider adding a warning log to surface this condition:
🛠️ Suggested improvement
// Check if any automations reference this program if h.automationStore != nil { refs, err := h.automationStore.FindByExternalProgramID(ctx, id) + } else { + log.Warn().Int("id", id).Msg("automationStore is nil, skipping automation reference check") }
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@web/src/lib/api.ts`:
- Around line 243-260: The APIError constructor uses Object.assign(this, data)
which can overwrite built-in Error properties (message, name) or class fields
(status); update the constructor in class APIError to only copy non-conflicting
keys from the provided data (e.g., filter out "name", "message", "status" and
any other reserved keys) before assigning, or explicitly set allowed fields onto
this instead of blanket-assigning, ensuring reserved properties remain untouched
while still exposing other response data.
🧹 Nitpick comments (3)
web/src/lib/api.ts (2)
567-569: Consistency issue: PlainErrorthrown instead ofAPIError.This error path still throws a plain
Errorwhile the mainrequestmethod now throwsAPIError. For consistency and to allow consumers to handle all API errors uniformly, consider throwingAPIErrorhere as well.♻️ Suggested fix
if (!response.ok) { const { message } = await this.extractErrorData(response) this.handleAuthError(response.status, `/instances/${instanceId}/backups/import`, message) - throw new Error(message) + throw new APIError(message, response.status) }
1372-1374: Same consistency issue: Consider throwingAPIErrorfor uniform error handling.Similar to
importBackupManifest, this throws a plainErrorwhile the mainrequestmethod throwsAPIError.♻️ Suggested fix
if (!response.ok) { const { message } = await this.extractErrorData(response) this.handleAuthError(response.status, `/instances/${instanceId}/torrents/${encodedHash}/export`, message) - throw new Error(message) + throw new APIError(message, response.status) }web/src/components/settings/ExternalProgramsManager.tsx (1)
39-44: Consider importing or co-locating the type with other shared types.The
AutomationReferenceinterface duplicates the structure defined in the backend. If this type is needed elsewhere or if the backend response shape changes, consider adding it to@/types/index.tsalongside other API response types to maintain a single source of truth.
0rkag
force-pushed
the
feat/execute-command-automation-action
branch
2 times, most recently
from
e7640e1 to
a891c5c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/lib/api.ts (1)
566-573: Inconsistent error type: throwsErrorinstead ofAPIError.This method uses
extractErrorDatabut throws a plainError, while the mainrequest()method throwsAPIError. This inconsistency means callers cannot reliably check forAPIErroror accessstatus/dataproperties when this method fails.🔧 Suggested fix for consistency
if (!response.ok) { - const { message } = await this.extractErrorData(response) + const { message, data } = await this.extractErrorData(response) this.handleAuthError(response.status, `/instances/${instanceId}/backups/import`, message) - throw new Error(message) + throw new APIError(message, response.status, data) }
🤖 Fix all issues with AI agents
In `@internal/services/automations/service.go`:
- Around line 1568-1577: The FREE_SPACE context isn't being initialized when
only an ExternalProgram action requires it because rulesUseCondition doesn't
treat ExternalProgram as a consumer; update the rulesUseCondition function to
include ExternalProgram in its condition-use checks (add the ExternalProgram
case or include it in the switch/if that detects actions requiring FREE_SPACE)
so callers that prepare free-space state (e.g., the code that builds
programExecutions/pendingProgramExec) will initialize FREE_SPACE when an
external program's conditions include FREE_SPACE; reference the
rulesUseCondition function and the ExternalProgram action/type and ensure the
same condition-detection logic used for other actions is applied to
ExternalProgram.
In `@web/src/lib/api.ts`:
- Around line 1371-1375: The code path in the export torrent block uses
this.extractErrorData and this.handleAuthError but ends with throw new
Error(message), causing inconsistent error typing; replace the plain Error with
the standard APIError used elsewhere (same pattern as importBackupManifest and
the request() method) so callers receive an APIError instance (include status
and any extracted error details when constructing the APIError), updating the
throw in the block that calls this.extractErrorData / this.handleAuthError to
create and throw a new APIError instead of Error.
♻️ Duplicate comments (5)
web/src/lib/api.ts (1)
243-260: Object.assign collision risk already flagged.The concern about
Object.assign(this, data)potentially overwriting built-in Error properties (name,message,stack) or class properties (status,data) has been noted in a previous review. The suggested fix to filter reserved keys before assignment remains applicable.internal/services/externalprograms/service_test.go (1)
1163-1167: Make the LookPath test OS-aware to avoid Windows failures.This test assumes
shexists everywhere; Windows runners will fail.🧪 Proposed fix
t.Run("unknown terminal uses LookPath", func(t *testing.T) { - // Test with a command that definitely exists - result := isTerminalAvailable("sh") - assert.True(t, result, "sh should be available on all Unix systems") + if runtime.GOOS == "windows" { + result := isTerminalAvailable("cmd") + assert.True(t, result, "cmd should be available on Windows") + return + } + result := isTerminalAvailable("sh") + assert.True(t, result, "sh should be available on all Unix systems") })internal/services/externalprograms/service.go (2)
113-138: Guard Execute against nil Service even when Program is preloaded.If
Executeis called on a nil receiver withreq.Programset, it will panic inexecuteProgram.🐛 Proposed fix
func (s *Service) Execute(ctx context.Context, req ExecuteRequest) ExecuteResult { + if s == nil { + return FailureResult(errors.New("external program service not initialized")) + } // Validate request first if err := req.Validate(); err != nil { return FailureResult(err) }
150-157: Log activity when a program is disabled.Automation-triggered attempts should still show up in the activity feed.
💡 Suggested patch
if !program.Enabled { log.Debug(). Int("programId", program.ID). Str("programName", program.Name). Msg("external program is disabled, skipping execution") + s.logActivity(ctx, req.InstanceID, req.Torrent, program, req.RuleID, req.RuleName, false, "program is disabled") return FailureResult(errors.New("program is disabled")) }internal/api/handlers/external_programs.go (1)
43-61: Trim program names before validation/storage.
Without trimming, whitespace-only or trailing-space names can pass validation and create confusing duplicates.🛠️ Suggested fix
// Validate and normalize input +req.Name = strings.TrimSpace(req.Name) req.Path = strings.TrimSpace(req.Path) if errMsg, status, ok := h.validateProgramInput(req.Name, req.Path); !ok {// Validate and normalize input +req.Name = strings.TrimSpace(req.Name) req.Path = strings.TrimSpace(req.Path) if errMsg, status, ok := h.validateProgramInput(req.Name, req.Path); !ok {Also applies to: 89-92, 136-139
🧹 Nitpick comments (6)
documentation/docs/features/automations.md (4)
264-266: Consider adding a concrete example to clarify "last matching rule" behavior.The explanation of last-rule-wins for External Program actions is technically accurate but may be confusing for users managing multiple rules. A brief example would help users understand what happens when multiple automation rules match the same torrent.
📝 Suggested clarification example
Consider adding after line 266:
:::note When multiple rules match the same torrent with External Program actions enabled, the **last matching rule** (by sort order) determines which program executes for that torrent. Only one program runs per torrent per automation cycle. + +**Example:** If you have three rules with External Program actions that all match torrent XYZ, and the rules are sorted as: Rule A (position 1), Rule B (position 2), Rule C (position 3), then only Rule C's program will execute for that torrent. :::
268-270: Add guidance on configuring the executable allowlist.The warning about the allowlist is important for security, but users may not know where or how to configure it. Consider adding a reference to the relevant configuration documentation or settings location.
📝 Suggested addition
:::warning -The program's executable path must be present in the application's allowlist. Programs that are disabled or have forbidden paths will not run—attempts are rejected and logged in the activity log with the rule name and torrent details. +The program's executable path must be present in the application's allowlist (configured via `externalProgramAllowList` in your configuration file). Programs that are disabled or have forbidden paths will not run—attempts are rejected and logged in the activity log with the rule name and torrent details. :::
244-277: Consider documenting how torrent data is passed to external programs.The section explains when and how external programs are triggered by automations, but doesn't mention what torrent information (hash, name, path, etc.) is available to the executed programs. Users will likely want to know what data they can access in their scripts.
If this information is configured in the External Program settings themselves, consider adding a note like:
:::tip Torrent data (name, hash, paths, etc.) is passed to the program based on the argument template configured in Settings → External Programs. See [External Programs documentation](#) for details on available variables. :::
472-477: Consider simplifying the condition by removing potentially redundant check.The condition uses both
State is completedANDProgress = 100. If the "completed" state already implies 100% progress, the Progress check may be redundant. However, if there are edge cases where a torrent might be in "completed" state with Progress < 100 (e.g., during file checking after a client restart), the defensive condition is appropriate.If
State is completedreliably indicates 100% progress, consider simplifying to:### Post-Processing on Completion Run a script when torrents finish downloading: - Tracker: `*` -- Condition: `State is completed` AND `Progress = 100` +- Condition: `State is completed` - Action: External Program "post-process.sh"Otherwise, keep the explicit check for robustness.
web/src/components/instances/preferences/WorkflowDialog.tsx (1)
1787-1798: Empty-state copy may mislead when all programs are disabled.The query filters to enabled programs only (line 291), so this message shows when no enabled programs exist. If a user has configured programs but they're all disabled, "No external programs configured" is misleading.
Consider updating the copy:
💡 Suggested copy tweak
- <div className="text-sm text-muted-foreground p-2 border rounded-md bg-muted/50"> - No external programs configured.{" "} + <div className="text-sm text-muted-foreground p-2 border rounded-md bg-muted/50"> + No enabled external programs available.{" "} <a href="/settings?tab=external-programs" target="_blank" rel="noopener noreferrer" className="text-primary hover:underline" > - Configure in Settings + Manage in Settings </a> </div>web/src/components/instances/preferences/WorkflowsPanel.tsx (1)
457-471: Consider showing program name instead of ID in the tooltip.The tooltip displays
Program ID: {programId}which is less informative than showing the program name. However, this would require fetching the external programs list.For now, this is acceptable, but consider enhancing this in the future if the external programs data becomes available in this component's context.
0rkag
force-pushed
the
feat/execute-command-automation-action
branch
from
a891c5c to
b589897
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@documentation/docs/features/automations.md`:
- Around line 472-484: Update the Progress field documentation and examples to
use the normalized float range 0-1: change the descriptive text that currently
reads "(0-100%)" to "(0-1)" and update the example condition under
"Post-Processing on Completion" (currently "Progress = 100") to "Progress = 1";
ensure any other occurrences in the "Post-Processing on Completion" and "Notify
on Stalled Torrents" examples reference the Progress field using the 0-1 float
format instead of percent values so the docs match the float64 storage and
current behavior.
♻️ Duplicate comments (4)
web/src/components/instances/preferences/WorkflowDialog.tsx (1)
1787-1799: Empty-state copy may still be misleading when programs exist but are disabled.The past review comment flagged this issue and marked it as addressed, but the current code still shows "No external programs configured." Consider updating the copy to "No enabled external programs available." and the link text to "Enable one in Settings" to clarify that disabled programs may exist.
internal/services/automations/service.go (1)
1266-1268: Ensure FREE_SPACE context is initialized for external program conditions.The external program action can have conditions that use
FREE_SPACE, butrulesUseCondition(lines 2426-2452) doesn't checkExternalProgram. This means if only an external program action uses a FREE_SPACE condition, the free space state won't be prepared and the condition will evaluate incorrectly.Also applies to: 1568-1577
internal/api/handlers/external_programs.go (2)
89-92: Trim program names before validation/storage.Only
Pathis normalized;Namecan retain whitespace and create confusing duplicates. NormalizeNameonce and reuse the trimmed value through validation and persistence.🛠️ Proposed fix
// Validate and normalize input +req.Name = strings.TrimSpace(req.Name) req.Path = strings.TrimSpace(req.Path) if errMsg, status, ok := h.validateProgramInput(req.Name, req.Path); !ok {// Validate and normalize input +req.Name = strings.TrimSpace(req.Name) req.Path = strings.TrimSpace(req.Path) if errMsg, status, ok := h.validateProgramInput(req.Name, req.Path); !ok {Also applies to: 136-139
55-57: Return 500 when the service is missing (don’t mask misconfiguration as 403).A nil
externalProgramServiceis a server misconfiguration; responding with 403 makes this look like a policy failure rather than a backend error.🛠️ Proposed fix
-// Validate path against allowlist using the shared service (fail closed if service is nil) -if h.externalProgramService == nil || !h.externalProgramService.IsPathAllowed(path) { - return "Program path is not allowed", http.StatusForbidden, false -} +// Validate path against allowlist using the shared service +if h.externalProgramService == nil { + return "External program service not configured", http.StatusInternalServerError, false +} +if !h.externalProgramService.IsPathAllowed(path) { + return "Program path is not allowed", http.StatusForbidden, false +}
🧹 Nitpick comments (3)
web/src/components/instances/preferences/WorkflowsPanel.tsx (1)
457-471: Consider showing the program name instead of just the ID.The tooltip displays "Program ID: {programId}" which isn't particularly helpful to users. Other action badges show meaningful information (e.g., tags show actual tag names, category shows the category name). Consider fetching and displaying the program name for better UX, or at minimum showing both name and ID.
That said, this is consistent with the current implementation scope and can be improved in a follow-up.
web/src/components/settings/ExternalProgramsManager.tsx (1)
89-109: Consider adding defensive checks for the error structure.The error handling on line 103 assumes
error.status === 409anderror.data?.automationsexist. If the API client doesn't consistently structure errors this way, this could fail silently or throw. Consider adding defensive checks.💡 Suggested defensive check
onError: (error: any) => { // Check if this is a 409 conflict with automation references - if (error.status === 409 && error.data?.automations) { + if (error?.status === 409 && Array.isArray(error?.data?.automations)) { setDeleteConflict(error.data.automations) return } toast.error(`Failed to delete external program: ${error.message || "Unknown error"}`) },internal/services/automations/service_test.go (1)
1301-1316: Mock implementation uses brittle positional argument parsing.The
ExecContextmock extracts values using hardcoded argument positions (args[0],args[1], etc.) with direct type assertions that can panic. If the SQL schema or query changes, this will silently produce incorrect test results or panic.Consider adding defensive type assertion checks:
💡 Suggested safer type assertions
func (m *mockQuerier) ExecContext(_ context.Context, query string, args ...any) (sql.Result, error) { // Capture activity insertions if len(args) >= 10 && strings.Contains(query, "automation_activity") { + instanceID, _ := args[0].(int) + hash, _ := args[1].(string) + torrentName, _ := args[2].(string) + action, _ := args[4].(string) + ruleName, _ := args[6].(string) + outcome, _ := args[7].(string) + reason, _ := args[8].(string) activity := &models.AutomationActivity{ - InstanceID: args[0].(int), - Hash: args[1].(string), - TorrentName: args[2].(string), - Action: args[4].(string), - RuleName: args[6].(string), - Outcome: args[7].(string), - Reason: args[8].(string), + InstanceID: instanceID, + Hash: hash, + TorrentName: torrentName, + Action: action, + RuleName: ruleName, + Outcome: outcome, + Reason: reason, } m.activities = append(m.activities, activity) } return mockResult{}, nil }This is a minor concern since tests will fail loudly if there's a mismatch, but the ok-form assertions provide cleaner failure messages.
0rkag
force-pushed
the
feat/execute-command-automation-action
branch
from
6e769ac to
ed865fb
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@internal/api/handlers/external_programs.go`:
- Around line 188-224: The current force-delete is non-atomic: calling
automationStore.ClearExternalProgramAction(ctx, id) then
externalProgramStore.Delete(ctx, id) can leave automations cleared while the
program remains if Delete fails; implement an atomic operation in the storage
layer (e.g., a new method like
externalProgramStore.DeleteWithClearedReferences(ctx, id) or
automationStore.ClearAndDeleteProgram(ctx, id)) that performs clearing of
external program actions and deletion in a single transaction (or uses a DB
transaction/lock/mark-and-sweep approach) and update the handler to call that
single atomic method (replace ClearExternalProgramAction + Delete calls),
propagating and logging errors and returning appropriate HTTP responses on
failure.
In `@internal/models/automation.go`:
- Around line 379-425: Both FindByExternalProgramID and
ClearExternalProgramAction are currently filtering on json_extract(conditions,
'$.externalProgram.enabled') = 1 which leaves disabled automations' programId
intact; remove that enabled check from the WHERE clauses in both functions
(AutomationStore.FindByExternalProgramID and
AutomationStore.ClearExternalProgramAction) so the SQL only matches by
json_extract(conditions, '$.externalProgram.programId') = ? and thus cleans up
all references regardless of enabled state.
♻️ Duplicate comments (3)
internal/services/externalprograms/service_test.go (1)
1163-1167: Avoid assumingshexists on Windows.
This test still fails on Windows runners; make the command lookup OS-aware.💡 Suggested fix
t.Run("unknown terminal uses LookPath", func(t *testing.T) { - // Test with a command that definitely exists - result := isTerminalAvailable("sh") - assert.True(t, result, "sh should be available on all Unix systems") + if runtime.GOOS == "windows" { + result := isTerminalAvailable("cmd") + assert.True(t, result, "cmd should be available on Windows") + return + } + result := isTerminalAvailable("sh") + assert.True(t, result, "sh should be available on all Unix systems") })internal/services/externalprograms/service.go (1)
113-138: Guard against nil Service when Program is preloaded.
Calling(*Service)(nil).Executewithreq.Programstill panics inexecuteProgram.🛡️ Proposed fix
func (s *Service) Execute(ctx context.Context, req ExecuteRequest) ExecuteResult { + if s == nil { + return FailureResult(errors.New("external program service not initialized")) + } // Validate request first if err := req.Validate(); err != nil { return FailureResult(err) }web/src/components/instances/preferences/WorkflowDialog.tsx (1)
287-292: Enabled-only filtering can hide selected/disabled programs.
Because the query filters to enabled programs, editing a rule that references a disabled program can render an empty selection and the empty-state text becomes misleading. Consider fetching all programs and marking disabled options, or update the empty-state copy accordingly.Copy-only fallback (if you keep enabled-only filtering)
- No external programs configured.{" "} + No enabled external programs available.{" "} @@ - Configure in Settings + Enable one in SettingsAlso applies to: 1747-1799
🧹 Nitpick comments (2)
internal/services/automations/service.go (1)
2632-2658: Consider bounding concurrent external program launches.
This spawns one goroutine/process per match; large rules could spike resource usage. If that’s not intended, a small semaphore/worker‑pool cap (ideally configurable) would smooth bursts. Please confirm the expected concurrency behavior.♻️ Optional semaphore cap (illustrative)
+ const maxConcurrentExec = 8 // consider making configurable + sem := make(chan struct{}, maxConcurrentExec) for _, exec := range executions { + sem <- struct{}{} exec := exec go func() { + defer func() { <-sem }() result := s.externalProgramService.Execute(context.Background(), externalprograms.ExecuteRequest{ ProgramID: programID, Torrent: &torrent, InstanceID: instanceID, RuleID: &ruleID, RuleName: ruleName, }) if !result.Success { log.Error(). Err(result.Error). Int("programID", programID). Str("ruleName", ruleName). Str("torrentHash", torrent.Hash). Msg("automation: external program execution failed") } }() }internal/services/crossseed/service.go (1)
8362-8425: External program execution lacks explicit timeout enforcement.The
externalprograms.Service.Executemethod does not enforce a timeout. It usescontext.Background()for command execution (line 173 in service.go), meaning hung external commands or process lookups can block indefinitely. While this aligns with the codebase's intentional fire-and-forget pattern for background operations, consider adding an explicit timeout here (e.g.,context.WithTimeout) if long-running external programs are a concern.
0rkag
force-pushed
the
feat/execute-command-automation-action
branch
2 times, most recently
from
67534f2 to
4808700
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@internal/models/automation.go`:
- Around line 386-409: FindByExternalProgramID currently only returns
automations with json_extract(conditions, '$.externalProgram.enabled') = 1 which
causes the warning to omit disabled automations while ClearExternalProgramAction
removes all references; remove the enabled filter from the SQL WHERE clause in
FindByExternalProgramID so it selects any automation with
json_extract(conditions, '$.externalProgram.programId') = ? (keep the same
parameters and result mapping), and update the surrounding comment to note it
returns both enabled and disabled references so the warning fully reflects what
ClearExternalProgramAction will modify.
🧹 Nitpick comments (4)
internal/services/externalprograms/service_test.go (1)
53-73: Consider removing unused mock variables.The
storeandactivityStorevariables are created but only suppressed with blank identifiers. If they're not needed, removing them would clean up the test.♻️ Suggested cleanup
func TestNewService(t *testing.T) { t.Run("creates service with all dependencies", func(t *testing.T) { - store := &mockProgramStore{} - activityStore := &mockActivityStore{} config := &domain.Config{} // Note: NewService accepts the concrete types, not our mocks // This test validates the constructor pattern service := NewService(nil, nil, config) assert.NotNil(t, service) assert.Equal(t, config, service.config) // Test with nil config (should be allowed) service2 := NewService(nil, nil, nil) assert.NotNil(t, service2) assert.Nil(t, service2.config) - - _ = store - _ = activityStore }) }internal/services/crossseed/service.go (1)
8360-8428: Verify fire‑and‑forget context isn’t still deadline‑bound.
context.WithoutCancelpreserves deadlines; if the caller’s ctx is request-scoped, external programs could be killed early. If the intent is truly fire‑and‑forget, consider a background/explicit timeout context for execution.💡 Possible adjustment
- if ctx == nil { - ctx = context.Background() - } else { - ctx = context.WithoutCancel(ctx) - } + execCtx := context.Background() + if ctx != nil { + // Note: WithoutCancel preserves deadlines; use Background() if you want no deadline. + execCtx = context.WithoutCancel(ctx) + } // Execute in a separate goroutine to avoid blocking the cross-seed operation go func() { // Get torrent data from sync manager - targetTorrent, found, err := s.syncManager.HasTorrentByAnyHash(ctx, instanceID, []string{torrentHash}) + targetTorrent, found, err := s.syncManager.HasTorrentByAnyHash(execCtx, instanceID, []string{torrentHash}) if err != nil { log.Error().Err(err).Int("instanceId", instanceID).Str("torrentHash", torrentHash).Msg("Failed to get torrent for external program execution") return } ... - result := s.externalProgramService.Execute(ctx, externalprograms.ExecuteRequest{ + result := s.externalProgramService.Execute(execCtx, externalprograms.ExecuteRequest{ ProgramID: programID, Torrent: targetTorrent, InstanceID: instanceID, })internal/services/automations/service.go (1)
2577-2663: Consider bounding concurrent executions.
Fire‑and‑forget spawning can create unbounded concurrent processes; a small semaphore/worker pool with a configurable max would protect hosts under large match bursts.web/src/components/instances/preferences/WorkflowDialog.tsx (1)
1799-1809: Consider using a relative link or the app's routing for settings navigation.The hardcoded absolute path
/settings?tab=external-programsmay not work correctly if the app is deployed under a base path (e.g.,/qui/settings). Consider using the app's routing mechanism or thewithBasePathutility.🔧 Suggested fix using withBasePath
<a - href="/settings?tab=external-programs" + href={withBasePath("/settings?tab=external-programs")} target="_blank" rel="noopener noreferrer" className="text-primary hover:underline" > Configure in Settings </a>This would require importing
withBasePathfrom@/lib/base-url.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@internal/services/externalprograms/service_test.go`:
- Around line 593-633: The test currently treats any non-macOS case as Linux
(runtime.GOOS check in internal/services/externalprograms/service_test.go) and
asserts presence/absence of Linux terminals against the candidates slice; update
that logic so Windows is handled explicitly: change the else branch into if
runtime.GOOS == "linux" with the existing Linux assertions over candidates
(hasGnomeTerminal, hasXterm, NotEqual for macOS terminals), and add a new else
if runtime.GOOS == "windows" branch that either skips Linux-specific assertions
or checks Windows-specific expectations, ensuring the runtime.GOOS checks and
candidate name assertions (e.g., comparisons against "gnome-terminal", "xterm",
"iterm2", "apple-terminal") reference the correct platform.
♻️ Duplicate comments (1)
internal/models/automation.go (1)
386-394: Include disabled automations in external program reference lookup.
FindByExternalProgramIDstill filters onexternalProgram.enabled = 1, so disabled rules won’t be surfaced in warnings and won’t be cleaned consistently withClearExternalProgramAction.🛠️ Suggested adjustment
- WHERE json_extract(conditions, '$.externalProgram.enabled') = 1 - AND json_extract(conditions, '$.externalProgram.programId') = ? + WHERE json_extract(conditions, '$.externalProgram.programId') = ?
🧹 Nitpick comments (3)
internal/services/automations/service.go (1)
2586-2662: Consider capping concurrent external program executions.This spawns one goroutine per match with no limit; large batches can overwhelm the host. A simple semaphore/worker pool would add safety without blocking the main loop.
♻️ Example semaphore guard
func (s *Service) executeExternalProgramsFromAutomation(_ context.Context, instanceID int, executions []pendingProgramExec) { if len(executions) == 0 { return } @@ - for _, exec := range executions { + const maxConcurrent = 8 + sem := make(chan struct{}, maxConcurrent) + for _, exec := range executions { // Copy to avoid closure issues torrent := exec.torrent ruleID := exec.ruleID programID := exec.programID ruleName := exec.ruleName // Execute asynchronously - the service handles its own activity logging // Use context.Background() since parent context may be cancelled before execution completes go func() { + sem <- struct{}{} + defer func() { <-sem }() result := s.externalProgramService.Execute(context.Background(), externalprograms.ExecuteRequest{ ProgramID: programID, Torrent: &torrent, InstanceID: instanceID, RuleID: &ruleID, RuleName: ruleName, }) if !result.Success { log.Error(). Err(result.Error). Int("programID", programID). Str("ruleName", ruleName). Str("torrentHash", torrent.Hash). Msg("automation: external program execution failed") } }() } }internal/services/crossseed/service.go (1)
8360-8428: Consider detaching deadlines for fire‑and‑forget execution.
context.WithoutCancelkeeps the caller’s deadline; if the request times out quickly, the async execution can be canceled before it runs. Consider switching to a background context (optionally with a service‑level timeout) inside the goroutine to match the fire‑and‑forget intent.♻️ Suggested change
- if ctx == nil { - ctx = context.Background() - } else { - ctx = context.WithoutCancel(ctx) - } + // Detach deadlines for fire-and-forget execution (optionally wrap with timeout) + ctx = context.Background()internal/services/externalprograms/service.go (1)
119-143: **Minor robustness gap when pre-loaded program is provided.**Whenreq.Programis pre-loaded (not nil), the nil check at line 129 is bypassed. If a caller somehow has a nil*Serviceand callsExecutewith a pre-loaded program, "as long as the function doesn't attempt to dereference the receiver, it executes without issue" in Go. However,executeProgramcallss.logActivity(line 162) which accessess.activityStore—this would panic.In practice, the service is always properly initialized via
NewService, so this is a defensive programming concern rather than a critical bug. The past review marked this as addressed, but I note the check only covers the database lookup path.💡 Optional: Add nil guard at Execute start for completeness
func (s *Service) Execute(ctx context.Context, req ExecuteRequest) ExecuteResult { + if s == nil { + return FailureResult(errors.New("external program service not initialized")) + } // Validate request first if err := req.Validate(); err != nil { return FailureResult(err)
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/api/handlers/automations.go (1)
708-736: Missing regex validation for ExternalProgram condition.
collectConditionRegexErrorsvalidates regex patterns for all action conditions exceptExternalProgram. If a user configures anExternalProgramaction with an invalid regex pattern in its condition, the error won't be caught until runtime.🔧 Proposed fix
func collectConditionRegexErrors(conditions *models.ActionConditions) []RegexValidationError { if conditions == nil { return nil } var result []RegexValidationError if conditions.SpeedLimits != nil { validateConditionRegex(conditions.SpeedLimits.Condition, "/conditions/speedLimits/condition", &result) } if conditions.ShareLimits != nil { validateConditionRegex(conditions.ShareLimits.Condition, "/conditions/shareLimits/condition", &result) } if conditions.Pause != nil { validateConditionRegex(conditions.Pause.Condition, "/conditions/pause/condition", &result) } if conditions.Delete != nil { validateConditionRegex(conditions.Delete.Condition, "/conditions/delete/condition", &result) } if conditions.Tag != nil { validateConditionRegex(conditions.Tag.Condition, "/conditions/tag/condition", &result) } if conditions.Category != nil { validateConditionRegex(conditions.Category.Condition, "/conditions/category/condition", &result) } + if conditions.ExternalProgram != nil { + validateConditionRegex(conditions.ExternalProgram.Condition, "/conditions/externalProgram/condition", &result) + } return result }
🧹 Nitpick comments (2)
internal/api/handlers/automations.go (1)
355-371: Consider logging when store is nil to aid debugging.The existence check is silently skipped when
h.externalProgramStore == nil. While the wiring appears correct inserver.go, defensive logging would help diagnose misconfiguration.💡 Optional improvement
// Verify the referenced external program exists if payload.Conditions.ExternalProgram != nil && payload.Conditions.ExternalProgram.Enabled && payload.Conditions.ExternalProgram.ProgramID > 0 { if h.externalProgramStore != nil { _, err := h.externalProgramStore.GetByID(ctx, payload.Conditions.ExternalProgram.ProgramID) if err != nil { if errors.Is(err, models.ErrExternalProgramNotFound) { return http.StatusBadRequest, "Referenced external program does not exist", err } return http.StatusInternalServerError, "Failed to verify external program", err } + } else { + log.Warn().Msg("externalProgramStore is nil, skipping external program existence check") } }internal/api/server.go (1)
251-251: Remove commented-out code or add a TODO comment explaining the intent.The global logger middleware is commented out while logging is applied specifically to API routes at line 324. If this is intentional, consider removing the commented line to avoid confusion.
0rkag
force-pushed
the
feat/execute-command-automation-action
branch
from
7180c1e to
cbc115e
Compare
|
@s0up4200 conflicts should be fixed. Can you also add the automations tag to this PR? |
s0up4200
added
enhancement
…s and fix Windows execution error handling
…inals - Add macOS native terminal support (iTerm2, Terminal.app) via AppleScript - Add cross-platform terminals (WezTerm, Hyper) for all platforms - Detect user's current terminal via TERM_PROGRAM env var (highest priority) - Refactor terminal detection into testable helper functions: - getTerminalCandidates(): returns terminals by platform/priority - detectTerminalFromEnv(): checks TERM_PROGRAM - buildTerminalArgs(): constructs terminal-specific commands - isTerminalAvailable(): checks terminal availability - escapeAppleScript(): safely escapes strings for AppleScript - Add comprehensive tests (109 new test cases) - Update documentation with supported terminal list Terminal priority order: 1. TERM_PROGRAM (user's active terminal) 2. Cross-platform CLI (WezTerm, Hyper, Kitty, Alacritty) 3. Linux terminals (gnome-terminal, konsole, etc.) 4. macOS native (iTerm2, Terminal.app) 5. Fallback (sh -c)
The conditionsUseField function was missing the ExternalProgram action, which meant FREE_SPACE conditions on ExternalProgram actions were not being detected during validation. - Add ExternalProgram check to conditionsUseField() - Add test cases for ExternalProgram with FREE_SPACE conditions
…delete - Add activity logging when externalProgramService is nil so users can see why programs aren't running (new "external_program_skipped" action) - Fix documentation to correctly describe "last rule wins" behavior for external program actions - Add frontend error handling for external programs query in WorkflowDialog - Return proper HTTP status codes (400/403) when program is disabled or path not allowed - Add integration tests for executeExternalProgramsFromAutomation - Implement cascade delete with warning: when deleting an external program referenced by automations, warn user and allow force delete which removes the action from all referencing automations
- Replace extractErrorMessage with extractErrorData in importBackupManifest and exportTorrent - Use error.data?.automations instead of error.automations for type-safe access
- ClearExternalProgramAction now clears all references regardless of action enabled state - Show disabled programs in automation UI with visual indicator - Add activity logging when program execution is skipped due to disabled state - Add WARNING docs about process limits to Execute and caller functions - Add Windows and Unix command building tests - Remove incorrect terminal availability test
- FindByExternalProgramID now returns all automations referencing a program, regardless of whether the action is enabled (consistent with ClearExternalProgramAction) - Add validation to verify referenced external program exists when creating/updating automation - Add externalProgramStore to AutomationHandler for program existence validation
0rkag
force-pushed
the
feat/execute-command-automation-action
branch
from
bdd0054 to
751f1e6
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/api/server.go (1)
86-98: AddExternalProgramServiceto test dependencies.The test helper
newTestDependenciesomitsExternalProgramService, leaving it nil while production code incmd/qui/main.go:692properly assigns it. Although handlers include nil checks to prevent crashes, test Dependencies should be complete to avoid this defensive coding pattern and ensure tests reflect production behavior.Add the service to
server_test.goaround line 140:ExternalProgramService: &externalprograms.Service{},internal/services/crossseed/service.go (1)
8372-8440: External program execution lacks timeout enforcement.The
externalprograms.Service.Executemethod intentionally usescontext.Background()(line 180-181 inexecuteProgram) to detach from the caller's cancellation context. Combined with fire-and-forget execution via goroutine, this means external processes can run indefinitely with no deadline. No timeout configuration exists in theExternalProgrammodel, and processes are only cleaned up when they naturally exit viacmd.Wait(). Given the "no rate limiting" note in the original warning comment, rapid injections risk spawning unbounded concurrent processes that may never terminate.Recommend adding a configurable timeout (or system-wide default) and enforcing it via
context.WithTimeouton the background context before passing it tobuildCommand.
🧹 Nitpick comments (3)
internal/services/automations/service_test.go (1)
1301-1316: Mock relies on hardcoded positional indices — consider documenting the expected SQL signature.The mock captures activity insertions by checking
len(args) >= 10and accessing specific indices (e.g.,args[0].(int),args[1].(string)). If the SQL parameter order changes, this test will silently extract wrong values or panic on type assertions.Consider adding a comment documenting the expected parameter order, or extracting named constants for the indices.
📝 Suggested documentation
func (m *mockQuerier) ExecContext(_ context.Context, query string, args ...any) (sql.Result, error) { // Capture activity insertions + // Expected parameter order for automation_activity INSERT: + // 0: instanceID, 1: hash, 2: torrentName, 3: size, 4: action, + // 5: details, 6: ruleName, 7: outcome, 8: reason, 9: timestamp if len(args) >= 10 && strings.Contains(query, "automation_activity") {internal/services/externalprograms/service_test.go (1)
53-73: Unused mock variables in TestNewService.The
storeandactivityStorevariables are created but only referenced with blank identifiers. SinceNewServiceaccepts concrete types, these mocks can't be used directly. Consider removing them or adding a comment explaining they document the expected interface.♻️ Suggested cleanup
func TestNewService(t *testing.T) { t.Run("creates service with all dependencies", func(t *testing.T) { - store := &mockProgramStore{} - activityStore := &mockActivityStore{} config := &domain.Config{} - // Note: NewService accepts the concrete types, not our mocks - // This test validates the constructor pattern + // NewService accepts concrete store types; this test validates the constructor pattern service := NewService(nil, nil, config) assert.NotNil(t, service) assert.Equal(t, config, service.config) // Test with nil config (should be allowed) service2 := NewService(nil, nil, nil) assert.NotNil(t, service2) assert.Nil(t, service2.config) - - _ = store - _ = activityStore }) }internal/api/handlers/external_programs.go (1)
43-61: Defensive redundancy in path trimming.The
pathparameter is trimmed again at line 50, but callers at lines 90 and 137 already trimreq.Pathbefore calling this method. The redundancy is harmless but unnecessary.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@web/src/components/instances/preferences/WorkflowDialog.tsx`:
- Around line 309-313: The component currently uses useQuery to fetch external
programs but only checks for error and data, causing the empty-state to render
during fetch; update the useQuery call to destructure isLoading (const { data:
allExternalPrograms, isError: externalProgramsError, isLoading:
externalProgramsLoading } = useQuery(...)) and in the render logic of
WorkflowDialog show a loading state (Spinner/Skeleton) when
externalProgramsLoading is true, and only render the empty-state when
externalProgramsLoading is false and allExternalPrograms is empty; apply the
same change to the other useQuery usage for external programs elsewhere in this
file that currently lacks a loading branch.
| const { data: allExternalPrograms, isError: externalProgramsError } = useQuery({ | ||
| queryKey: ["externalPrograms"], | ||
| queryFn: () => api.listExternalPrograms(), | ||
| enabled: open, | ||
| }) |
There was a problem hiding this comment.
Show a loading state while external programs are fetching.
Without a loading branch, the empty-state renders during fetch, which is misleading.
🛠️ Suggested fix
- const { data: allExternalPrograms, isError: externalProgramsError } = useQuery({
+ const { data: allExternalPrograms, isError: externalProgramsError, isLoading: externalProgramsLoading } = useQuery({
queryKey: ["externalPrograms"],
queryFn: () => api.listExternalPrograms(),
enabled: open,
})- {externalProgramsError ? (
+ {externalProgramsLoading ? (
+ <div className="text-sm text-muted-foreground p-2 border rounded-md bg-muted/50">
+ Loading external programs...
+ </div>
+ ) : externalProgramsError ? (
<div className="text-sm text-destructive p-2 border border-destructive/50 rounded-md bg-destructive/10">
Failed to load external programs. Please try again.
</div>Also applies to: 1798-1839
🤖 Prompt for AI Agents
In `@web/src/components/instances/preferences/WorkflowDialog.tsx` around lines 309
- 313, The component currently uses useQuery to fetch external programs but only
checks for error and data, causing the empty-state to render during fetch;
update the useQuery call to destructure isLoading (const { data:
allExternalPrograms, isError: externalProgramsError, isLoading:
externalProgramsLoading } = useQuery(...)) and in the render logic of
WorkflowDialog show a loading state (Spinner/Skeleton) when
externalProgramsLoading is true, and only render the empty-state when
externalProgramsLoading is false and allExternalPrograms is empty; apply the
same change to the other useQuery usage for external programs elsewhere in this
file that currently lacks a loading branch.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/src/components/instances/preferences/WorkflowsOverview.tsx (1)
1140-1149:⚠️ Potential issue | 🟡 MinorOutcome badge text is misleading for external program runs.
“Removed” reads like a delete action; for external_program it should say “Executed” (or similar) to avoid confusion.
Proposed fix
- {event.outcome === "success" ? "Removed" : "Failed"} + {event.action === "external_program" + ? (event.outcome === "success" ? "Executed" : "Failed") + : (event.outcome === "success" ? "Removed" : "Failed")}internal/api/handlers/automations.go (1)
275-288:⚠️ Potential issue | 🟠 MajorBlock ExternalProgram when Delete is enabled.
Delete is documented as standalone, but ExternalProgram isn’t treated as an “other action,” so users can configure delete+external program and the program will be silently skipped. Please include ExternalProgram in the standalone check.🛠️ Proposed fix
- hasOtherAction := (payload.Conditions.SpeedLimits != nil && payload.Conditions.SpeedLimits.Enabled) || - (payload.Conditions.ShareLimits != nil && payload.Conditions.ShareLimits.Enabled) || - (payload.Conditions.Pause != nil && payload.Conditions.Pause.Enabled) || - (payload.Conditions.Tag != nil && payload.Conditions.Tag.Enabled) || - (payload.Conditions.Category != nil && payload.Conditions.Category.Enabled) + hasOtherAction := (payload.Conditions.SpeedLimits != nil && payload.Conditions.SpeedLimits.Enabled) || + (payload.Conditions.ShareLimits != nil && payload.Conditions.ShareLimits.Enabled) || + (payload.Conditions.Pause != nil && payload.Conditions.Pause.Enabled) || + (payload.Conditions.Tag != nil && payload.Conditions.Tag.Enabled) || + (payload.Conditions.Category != nil && payload.Conditions.Category.Enabled) || + (payload.Conditions.ExternalProgram != nil && payload.Conditions.ExternalProgram.Enabled)Also applies to: 355-373
|
LGTM! Thank you @0rkag ❤️ |
|
Hello @s0up4200: I'm trying unsuccessfully to “Run post-processing scripts when torrents complete” with my automations. Can you give me an example of conditions to make this work? I'm using pr-1395 |
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [ghcr.io/autobrr/qui](https://github.com/autobrr/qui) | minor | `v1.13.1` → `v1.14.0` | --- ### Release Notes <details> <summary>autobrr/qui (ghcr.io/autobrr/qui)</summary> ### [`v1.14.0`](https://github.com/autobrr/qui/releases/tag/v1.14.0) [Compare Source](autobrr/qui@v1.13.1...v1.14.0) #### Changelog ##### New Features - [`6f8e6ed`](autobrr/qui@6f8e6ed): feat(api): add torrent field endpoint for select all copy ([#​1477](autobrr/qui#1477)) ([@​jabloink](https://github.com/jabloink)) - [`2d9b4c7`](autobrr/qui@2d9b4c7): feat(automation): trigger external programs automatically via automation rules ([#​1284](autobrr/qui#1284)) ([@​0rkag](https://github.com/0rkag)) - [`32692a4`](autobrr/qui@32692a4): feat(automations): Add the ability to define the move automation with a templated path ([#​1376](autobrr/qui#1376)) ([@​ColinHebert](https://github.com/ColinHebert)) - [`61bbeb1`](autobrr/qui@61bbeb1): feat(automations): add Resume action to Automations ([#​1350](autobrr/qui#1350)) ([@​cy1der](https://github.com/cy1der)) - [`450b98f`](autobrr/qui@450b98f): feat(automations): grouping + release fields ([#​1467](autobrr/qui#1467)) ([@​s0up4200](https://github.com/s0up4200)) - [`18d4a64`](autobrr/qui@18d4a64): feat(automations): match tracker conditions by display name ([#​1420](autobrr/qui#1420)) ([@​s0up4200](https://github.com/s0up4200)) - [`7c67b82`](autobrr/qui@7c67b82): feat(automations): show activity run details ([#​1385](autobrr/qui#1385)) ([@​s0up4200](https://github.com/s0up4200)) - [`177ef4d`](autobrr/qui@177ef4d): feat(crossseed): Multiple hard/reflink dirs ([#​1289](autobrr/qui#1289)) ([@​rybertm](https://github.com/rybertm)) - [`a72b673`](autobrr/qui@a72b673): feat(crossseed): gazelle-only OPS/RED ([#​1436](autobrr/qui#1436)) ([@​s0up4200](https://github.com/s0up4200)) - [`6a29384`](autobrr/qui@6a29384): feat(crossseed): match bit depth ([#​1427](autobrr/qui#1427)) ([@​s0up4200](https://github.com/s0up4200)) - [`c7fd5aa`](autobrr/qui@c7fd5aa): feat(dirscan): add max searchee age filter ([#​1486](autobrr/qui#1486)) ([@​s0up4200](https://github.com/s0up4200)) - [`d595a55`](autobrr/qui@d595a55): feat(documentation): add AI doc actions and llms discoverability ([#​1451](autobrr/qui#1451)) ([@​s0up4200](https://github.com/s0up4200)) - [`562ab3f`](autobrr/qui@562ab3f): feat(metrics): add tracker metrics ([#​1073](autobrr/qui#1073)) ([@​Winter](https://github.com/Winter)) - [`1b9aa9d`](autobrr/qui@1b9aa9d): feat(notifications): add shoutrrr and notifiarr ([#​1371](autobrr/qui#1371)) ([@​s0up4200](https://github.com/s0up4200)) - [`6d1dac7`](autobrr/qui@6d1dac7): feat(pwa): add protocol and file handlers for magnet links and torrent files ([#​783](autobrr/qui#783)) ([@​s0up4200](https://github.com/s0up4200)) - [`42fa501`](autobrr/qui@42fa501): feat(torrents): add unified cross-instance torrent table ([#​1481](autobrr/qui#1481)) ([@​s0up4200](https://github.com/s0up4200)) - [`498eaca`](autobrr/qui@498eaca): feat(ui): show speeds in page title ([#​1292](autobrr/qui#1292)) ([@​NoLife141](https://github.com/NoLife141)) - [`94a506e`](autobrr/qui@94a506e): feat(unregistered): nem talalhato ([#​1483](autobrr/qui#1483)) ([@​KyleSanderson](https://github.com/KyleSanderson)) - [`8bf366c`](autobrr/qui@8bf366c): feat(web): add logs nav ([#​1458](autobrr/qui#1458)) ([@​s0up4200](https://github.com/s0up4200)) - [`babc88d`](autobrr/qui@babc88d): feat(web): add responsive popover with mobile drawer support ([#​1398](autobrr/qui#1398)) ([@​jabloink](https://github.com/jabloink)) - [`06d341b`](autobrr/qui@06d341b): feat(web): add torrent table selection quick wins ([#​1455](autobrr/qui#1455)) ([@​s0up4200](https://github.com/s0up4200)) - [`56fbbec`](autobrr/qui@56fbbec): feat(web): hide selection column ([#​1460](autobrr/qui#1460)) ([@​s0up4200](https://github.com/s0up4200)) - [`46814aa`](autobrr/qui@46814aa): feat(web): qBittorrent autorun preferences ([#​1430](autobrr/qui#1430)) ([@​s0up4200](https://github.com/s0up4200)) - [`342643e`](autobrr/qui@342643e): feat(web): unify instance settings & qbit options dialog ([#​1257](autobrr/qui#1257)) ([@​0rkag](https://github.com/0rkag)) - [`e634d01`](autobrr/qui@e634d01): feat: add cross-seed blocklist ([#​1391](autobrr/qui#1391)) ([@​s0up4200](https://github.com/s0up4200)) - [`13aaac8`](autobrr/qui@13aaac8): feat: add dry-run workflows ([#​1395](autobrr/qui#1395)) ([@​s0up4200](https://github.com/s0up4200)) - [`f01101d`](autobrr/qui@f01101d): feat: add option to disable built-in authentication ([#​1464](autobrr/qui#1464)) ([@​libussa](https://github.com/libussa)) - [`6d1da50`](autobrr/qui@6d1da50): feat: download individual content files from context menu ([#​1465](autobrr/qui#1465)) ([@​libussa](https://github.com/libussa)) - [`77e9abf`](autobrr/qui@77e9abf): feat: migrate to dodopayments ([#​1407](autobrr/qui#1407)) ([@​s0up4200](https://github.com/s0up4200)) - [`9f6c856`](autobrr/qui@9f6c856): feat: support basic auth for ARR and Torznab ([#​1442](autobrr/qui#1442)) ([@​s0up4200](https://github.com/s0up4200)) ##### Bug Fixes - [`8a06d4b`](autobrr/qui@8a06d4b): fix(api): correct add-torrent OpenAPI param names and add missing fields ([#​1426](autobrr/qui#1426)) ([@​s0up4200](https://github.com/s0up4200)) - [`b9a687c`](autobrr/qui@b9a687c): fix(api): honor explicit basic auth clear from URL userinfo ([@​s0up4200](https://github.com/s0up4200)) - [`948ca67`](autobrr/qui@948ca67): fix(api): tighten CORS/auth routing and base URL joins ([#​1325](autobrr/qui#1325)) ([@​s0up4200](https://github.com/s0up4200)) - [`12bea13`](autobrr/qui@12bea13): fix(automations): improve applied action summaries ([#​1478](autobrr/qui#1478)) ([@​s0up4200](https://github.com/s0up4200)) - [`8fe658b`](autobrr/qui@8fe658b): fix(automations): negate regex match for NotContains/NotEqual operators ([#​1441](autobrr/qui#1441)) ([@​andresatierf](https://github.com/andresatierf)) - [`8a808eb`](autobrr/qui@8a808eb): fix(automations): respect remove-only tag conditions ([#​1444](autobrr/qui#1444)) ([@​s0up4200](https://github.com/s0up4200)) - [`a72715e`](autobrr/qui@a72715e): fix(backups): add failure cooldown and export throttling ([#​1214](autobrr/qui#1214)) ([@​s0up4200](https://github.com/s0up4200)) - [`2e75c14`](autobrr/qui@2e75c14): fix(backups): skip exports missing metadata ([#​1362](autobrr/qui#1362)) ([@​s0up4200](https://github.com/s0up4200)) - [`5658421`](autobrr/qui@5658421): fix(config): update commented log settings in place ([#​1402](autobrr/qui#1402)) ([@​s0up4200](https://github.com/s0up4200)) - [`62c50c0`](autobrr/qui@62c50c0): fix(crossseed): tighten TV title matching ([#​1445](autobrr/qui#1445)) ([@​s0up4200](https://github.com/s0up4200)) - [`e7cc489`](autobrr/qui@e7cc489): fix(dirscan): prevent immediate requeue after cancel ([#​1446](autobrr/qui#1446)) ([@​s0up4200](https://github.com/s0up4200)) - [`36cbfcf`](autobrr/qui@36cbfcf): fix(docs): avoid mdx jsx parse error ([@​s0up4200](https://github.com/s0up4200)) - [`d8d6f62`](autobrr/qui@d8d6f62): fix(filters): stabilize dense sidebar layout ([#​1384](autobrr/qui#1384)) ([@​s0up4200](https://github.com/s0up4200)) - [`b959fc6`](autobrr/qui@b959fc6): fix(orphanscan): NFC-normalize paths to avoid false orphans ([#​1422](autobrr/qui#1422)) ([@​s0up4200](https://github.com/s0up4200)) - [`598e994`](autobrr/qui@598e994): fix(reflink): retry EAGAIN clones ([#​1360](autobrr/qui#1360)) ([@​s0up4200](https://github.com/s0up4200)) - [`aaa5ee0`](autobrr/qui@aaa5ee0): fix(reflinktree): retry transient FICLONE EINVAL and add diagnostics ([#​1487](autobrr/qui#1487)) ([@​s0up4200](https://github.com/s0up4200)) - [`647af31`](autobrr/qui@647af31): fix(rss): enable rules list scrolling ([#​1359](autobrr/qui#1359)) ([@​s0up4200](https://github.com/s0up4200)) - [`c356a6f`](autobrr/qui@c356a6f): fix(sync): Optimize torrent sorting and reference management ([#​1474](autobrr/qui#1474)) ([@​KyleSanderson](https://github.com/KyleSanderson)) - [`cf4310e`](autobrr/qui@cf4310e): fix(ui): update placeholder text in ArrInstanceForm based on instance type ([#​1375](autobrr/qui#1375)) ([@​pashioya](https://github.com/pashioya)) - [`92b6748`](autobrr/qui@92b6748): fix(web): format IPv6 peer addresses and copy IP without port ([#​1417](autobrr/qui#1417)) ([@​sleepm](https://github.com/sleepm)) - [`25039bc`](autobrr/qui@25039bc): fix(web): handle SSO session expiry behind Cloudflare Access and other proxies ([#​1438](autobrr/qui#1438)) ([@​nitrobass24](https://github.com/nitrobass24)) - [`77fe310`](autobrr/qui@77fe310): fix(web): prevent category submenu re-render ([#​1357](autobrr/qui#1357)) ([@​jabloink](https://github.com/jabloink)) - [`a42ab1e`](autobrr/qui@a42ab1e): fix(web): raise instance preferences max value from 999 to 99999 ([#​1311](autobrr/qui#1311)) ([@​s0up4200](https://github.com/s0up4200)) - [`540168c`](autobrr/qui@540168c): fix(web): raise virtualization threshold ([#​1355](autobrr/qui#1355)) ([@​jabloink](https://github.com/jabloink)) - [`8547dc6`](autobrr/qui@8547dc6): fix(web): remove column filters when column is hidden ([#​1418](autobrr/qui#1418)) ([@​jabloink](https://github.com/jabloink)) - [`6b09b8d`](autobrr/qui@6b09b8d): fix(web): remove panel size bounds ([@​s0up4200](https://github.com/s0up4200)) - [`db4cdc4`](autobrr/qui@db4cdc4): fix(web): show piece size in torrent details ([#​1365](autobrr/qui#1365)) ([@​s0up4200](https://github.com/s0up4200)) - [`1f94a06`](autobrr/qui@1f94a06): fix(web): use absolute for scroll-to-top on desktop ([#​1419](autobrr/qui#1419)) ([@​jabloink](https://github.com/jabloink)) - [`e31fe3a`](autobrr/qui@e31fe3a): fix: detect tracker health support after qBit upgrade ([#​909](autobrr/qui#909)) ([@​s0up4200](https://github.com/s0up4200)) - [`52f01da`](autobrr/qui@52f01da): fix: disable update indicators when update checks are off ([#​1364](autobrr/qui#1364)) ([@​s0up4200](https://github.com/s0up4200)) - [`f7e3fed`](autobrr/qui@f7e3fed): fix: normalize DD+ and DDP file keys ([#​1456](autobrr/qui#1456)) ([@​s0up4200](https://github.com/s0up4200)) ##### Other Changes - [`d914301`](autobrr/qui@d914301): chore(ci): fire Blacksmith (my wallet screamed) ([#​1408](autobrr/qui#1408)) ([@​s0up4200](https://github.com/s0up4200)) - [`b43327d`](autobrr/qui@b43327d): chore(deps): bump the golang group with 2 updates ([#​1378](autobrr/qui#1378)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`57747bd`](autobrr/qui@57747bd): chore(deps): bump the npm group across 1 directory with 27 updates ([#​1379](autobrr/qui#1379)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`a43850d`](autobrr/qui@a43850d): chore(docs): add BIMI SVG logo ([@​s0up4200](https://github.com/s0up4200)) - [`914bede`](autobrr/qui@914bede): chore(funding): add Patreon to FUNDING.yml ([@​s0up4200](https://github.com/s0up4200)) - [`8b76f1e`](autobrr/qui@8b76f1e): docs(automations): clarify tag matching examples ([#​1457](autobrr/qui#1457)) ([@​s0up4200](https://github.com/s0up4200)) - [`2994054`](autobrr/qui@2994054): docs(readme): restore concise README ([#​1452](autobrr/qui#1452)) ([@​s0up4200](https://github.com/s0up4200)) - [`51237d4`](autobrr/qui@51237d4): docs: Add configuration reference ([#​1440](autobrr/qui#1440)) ([@​s0up4200](https://github.com/s0up4200)) - [`741462c`](autobrr/qui@741462c): docs: add Windows installation guide ([#​1463](autobrr/qui#1463)) ([@​soggy-cr0uton](https://github.com/soggy-cr0uton)) - [`6a11430`](autobrr/qui@6a11430): docs: clarify autobrr filter + apply troubleshooting ([#​1459](autobrr/qui#1459)) ([@​s0up4200](https://github.com/s0up4200)) - [`5a2edc2`](autobrr/qui@5a2edc2): docs: update 2 documentation files ([#​1454](autobrr/qui#1454)) ([@​s0up4200](https://github.com/s0up4200)) - [`139ada9`](autobrr/qui@139ada9): docs: update contributing.md ([#​1470](autobrr/qui#1470)) ([@​s0up4200](https://github.com/s0up4200)) - [`3909aa1`](autobrr/qui@3909aa1): docs: update docs/features/automations.md ([#​1447](autobrr/qui#1447)) ([@​s0up4200](https://github.com/s0up4200)) - [`5dc57ca`](autobrr/qui@5dc57ca): docs: update intro.md ([#​1453](autobrr/qui#1453)) ([@​s0up4200](https://github.com/s0up4200)) - [`5d9e986`](autobrr/qui@5d9e986): perf(web): memoize useDateTimeFormatters ([#​1403](autobrr/qui#1403)) ([@​jabloink](https://github.com/jabloink)) **Full Changelog**: <autobrr/qui@v1.13.1...v1.14.0> #### Docker images - `docker pull ghcr.io/autobrr/qui:v1.14.0` - `docker pull ghcr.io/autobrr/qui:latest` #### What to do next? - Join our [Discord server](https://discord.autobrr.com/qui) Thank you for using qui! </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yNS43IiwidXBkYXRlZEluVmVyIjoiNDMuMjUuNyIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiaW1hZ2UiXX0=--> Reviewed-on: https://gitea.alexlebens.dev/alexlebens/infrastructure/pulls/4154 Co-authored-by: Renovate Bot <renovate-bot@alexlebens.net> Co-committed-by: Renovate Bot <renovate-bot@alexlebens.net>
5 participants
Summary
Changes
New Features
Refactoring
internal/services/externalprograms/service.go- a unified service that handles execution for all consumers (automations, cross-seed, API)Security & Reliability
externalProgramAllowListconfigexec.CommandContextfor better process lifecycle managementDocumentation
Test plan
Summary by CodeRabbit
New Features
Validation
API
Refactor
Documentation
Tests