feat: add post-download actions (on-complete and on-error shell hooks)

[mvanhorn]

Summary

Add configurable shell commands that run after a download completes or fails. Commands support template variables for flexible automation.

Why this matters

aria2 has --on-download-complete hooks. wget has --execute for post-processing. IDM and JDownloader have post-download actions. Surge currently has no way to automate what happens after a download finishes.

With Surge's daemon architecture, this is especially useful for headless servers and Raspberry Pi setups where users want downloads to auto-extract, move to media directories, or trigger notifications.

Changes

• Add PostDownloadActions config struct with on_complete_command and on_error_command fields to settings.json
• Add RunPostActions() in internal/processing/post_actions.go with template variable substitution ({filename}, {filepath}, {size}, {speed}, {duration}, {id}, {error})
• Wire into StartEventWorker for both DownloadCompleteMsg and DownloadErrorMsg events
• Actions run in goroutines so they never block the download lifecycle
• Add "Post-Download" settings tab in the TUI
• Cross-platform: uses sh -c on Unix, cmd /C on Windows

Testing

All tests pass including new tests for template expansion and command execution:

go test ./... -count=1   # All 18 packages pass

This contribution was developed with AI assistance (Codex + Claude Code).

Greptile Summary

This PR adds configurable post-download shell hooks (on_complete_command / on_error_command) with template variable substitution, wired into the StartEventWorker event loop via goroutines. Prior review concerns about shell injection and error-routing via empty string have been resolved with shellEscape and an explicit isError bool parameter.

• P1: In the DownloadErrorMsg handler, RunPostActions receives m.DestPath (the raw event field) instead of the locally-resolved destPath variable, so {filepath} is empty whenever the download path is recovered from the DB — the exact scenario the complete handler explicitly guards against.

Confidence Score: 4/5

One P1 bug in the error path should be fixed before merging; all other findings are P2.

The m.DestPath vs destPath mismatch in the error handler is a present defect that silently delivers an empty {filepath} to users' error hooks. Everything else — injection hardening, error routing, config struct, and tests — is solid. Fix that one line and this is merge-ready.

internal/processing/events.go — specifically the DownloadErrorMsg RunPostActions call at line 347.

Important Files Changed

Filename	Overview
internal/processing/post_actions.go	New file implementing shell hook execution with shellEscape for safe template variable substitution and explicit isError bool routing — prior injection and error-routing concerns are resolved.
internal/processing/events.go	P1 bug: error handler passes m.DestPath (raw message value) instead of the locally-resolved destPath to RunPostActions, making {filepath} empty when the path is recovered from DB; also has a redundant OnErrorCommand != "" pre-check inconsistent with the complete handler.
internal/config/settings.go	Adds PostDownloadActions struct and wires it into GeneralSettings; metadata and category order updated correctly, no issues.
internal/processing/post_actions_test.go	Covers happy path and empty-command cases; missing edge-case tests for shellEscape with special characters (quotes, semicolons, spaces) that are the primary security guarantee of this feature.
internal/config/settings_test.go	Comprehensive existing test suite updated to include Post-Download category; all category-order and metadata tests pass correctly.

Sequence Diagram

sequenceDiagram
    participant Engine
    participant EventWorker as StartEventWorker
    participant DB as state (DB)
    participant PostActions as RunPostActions
    participant Shell

    Engine->>EventWorker: DownloadCompleteMsg
    EventWorker->>DB: GetDownload(id) → destPath, filename
    EventWorker->>DB: AddToMasterList(completed)
    EventWorker->>PostActions: go RunPostActions(settings, ctx{destPath}, isError=false)
    PostActions->>PostActions: expandTemplate → shellEscape vars
    PostActions->>Shell: sh -c on_complete_command
    Shell-->>PostActions: output / error (logged)

    Engine->>EventWorker: DownloadErrorMsg
    EventWorker->>DB: GetDownload(id) → existing
    EventWorker->>DB: AddToMasterList(error)
    EventWorker->>PostActions: go RunPostActions(settings, ctx{m.DestPath ⚠️}, isError=true)
    PostActions->>PostActions: expandTemplate → shellEscape vars
    PostActions->>Shell: sh -c on_error_command
    Shell-->>PostActions: output / error (logged)

Loading

Comments Outside Diff (1)

1. internal/tui/view_settings.go, line 247-283 (link)

   "Post-Download" tab is silently non-functional in the TUI

   getSettingsValues has no "Post-Download" case, so the tab always renders empty values. setSettingValue (line 298) and resetSettingToDefault (line 631) also lack this case, meaning every edit the user types is silently discarded — the feature is only configurable by hand-editing settings.json.

   Three switch blocks all need a "Post-Download" arm. For example in getSettingsValues:

   case "Post-Download":
       values["on_complete_command"] = m.Settings.General.PostDownload.OnCompleteCommand
       values["on_error_command"] = m.Settings.General.PostDownload.OnErrorCommand

   And in setSettingValue, route to a new helper:

   case "Post-Download":
       if key == "on_complete_command" {
           m.Settings.General.PostDownload.OnCompleteCommand = value
       } else if key == "on_error_command" {
           m.Settings.General.PostDownload.OnErrorCommand = value
       }

   And in resetSettingToDefault:

   case "Post-Download":
       switch key {
       case "on_complete_command":
           m.Settings.General.PostDownload.OnCompleteCommand = defaults.General.PostDownload.OnCompleteCommand
       case "on_error_command":
           m.Settings.General.PostDownload.OnErrorCommand = defaults.General.PostDownload.OnErrorCommand
       }

   Rule Used: What: Enforce separation of concerns by keeping bu... (source)

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: internal/tui/view_settings.go
   Line: 247-283
   
   Comment:
   **"Post-Download" tab is silently non-functional in the TUI**
   
   `getSettingsValues` has no `"Post-Download"` case, so the tab always renders empty values. `setSettingValue` (line 298) and `resetSettingToDefault` (line 631) also lack this case, meaning every edit the user types is silently discarded — the feature is only configurable by hand-editing `settings.json`.
   
   Three switch blocks all need a `"Post-Download"` arm. For example in `getSettingsValues`:
   
   ```go
   case "Post-Download":
       values["on_complete_command"] = m.Settings.General.PostDownload.OnCompleteCommand
       values["on_error_command"] = m.Settings.General.PostDownload.OnErrorCommand
   ```
   
   And in `setSettingValue`, route to a new helper:
   
   ```go
   case "Post-Download":
       if key == "on_complete_command" {
           m.Settings.General.PostDownload.OnCompleteCommand = value
       } else if key == "on_error_command" {
           m.Settings.General.PostDownload.OnErrorCommand = value
       }
   ```
   
   And in `resetSettingToDefault`:
   
   ```go
   case "Post-Download":
       switch key {
       case "on_complete_command":
           m.Settings.General.PostDownload.OnCompleteCommand = defaults.General.PostDownload.OnCompleteCommand
       case "on_error_command":
           m.Settings.General.PostDownload.OnErrorCommand = defaults.General.PostDownload.OnErrorCommand
       }
   ```
   
   **Rule Used:** What: Enforce separation of concerns by keeping bu... ([source](https://app.greptile.com/review/custom-context?memory=c1138776-52c4-4b49-81a0-1cd68fe194dc))
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: internal/processing/events.go
Line: 347

Comment:
**`m.DestPath` instead of resolved `destPath` in error hook**

The error handler builds a corrected `destPath` by falling back to `existing.DestPath` when `m.DestPath` is empty (lines 308–323), and then correctly uses that variable for `RemoveIncompleteFile`. The `RunPostActions` call on line 347 bypasses that fix and passes `m.DestPath` directly, so `{filepath}` will be an empty string in error hooks whenever the message carries no path but the DB does — the same scenario the complete handler explicitly documents ("DownloadCompleteMsg does not carry destPath, so we recover the stable final location from the DB entry").

```suggestion
			FilePath: destPath,
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: internal/processing/events.go
Line: 339

Comment:
**Redundant `OnErrorCommand` pre-check**

`RunPostActions` already returns immediately when the selected command is empty (post_actions.go:60–62), so the `OnErrorCommand != ""` guard here is redundant — and creates an asymmetry with the `DownloadCompleteMsg` case (line 292), which calls `RunPostActions` unconditionally once `settings != nil`. Dropping the pre-check would make both paths consistent.

```suggestion
			if settings := mgr.GetSettings(); settings != nil {
```

**Rule Used:** What: Eliminate duplicate logic, functions, or cod... ([source](https://app.greptile.com/review/custom-context?memory=f0a796a9-684f-4dfb-a5cf-8c99c16b63a1))

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: internal/processing/post_actions_test.go
Line: 11-46

Comment:
**Missing `shellEscape` edge-case tests**

`TestExpandTemplate` only uses clean filenames (`"test.zip"`, `"abc123"`) that exercise neither branch of `shellEscape`. Given that the entire security posture of this feature relies on correct quoting, the test suite should include filenames with embedded single quotes, spaces, and semicolons (Unix) and double-quote characters (Windows) to confirm the escaping actually prevents injection.

```go
{"filename with single quote", "echo {filename}", "echo " + shellEscape("file'name.zip")},
{"filename with space",        "echo {filename}", "echo " + shellEscape("my file.zip")},
{"filename with semicolon",    "echo {filename}", "echo " + shellEscape("file;rm -rf .zip")},
```

**Rule Used:** What: All code changes must include tests for edge... ([source](https://app.greptile.com/review/custom-context?memory=2b22782d-3452-4d55-b059-e631b2540ce8))

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: internal/processing/post_actions.go
Line: 67-78

Comment:
**No timeout on command execution**

`c.CombinedOutput()` blocks the goroutine until the child process exits. For a headless daemon running many downloads in parallel, a hanging hook accumulates goroutines indefinitely with no bound. Consider wrapping the exec in a `context.WithTimeout` so runaway hooks are bounded:

```go
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
defer cancel()
var c *exec.Cmd
if runtime.GOOS == "windows" {
    c = exec.CommandContext(ctx, "cmd", "/C", expanded)
} else {
    c = exec.CommandContext(ctx, "sh", "-c", expanded)
}
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (4): Last reviewed commit: "test(post-actions): derive expected quot..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

[github-actions]

Binary Size Analysis

⚠️ Size Increased

Version	Human Readable	Raw Bytes
Main	18.91 MB	19829028
PR	18.92 MB	19837220
Difference	8.00 KB	8192

[mvanhorn]

Fixed the shell injection in expandTemplate - template variables like {filename} are now shell-escaped before substitution (single-quoted on Unix, double-quoted on Windows). Pushed in 7de5a0d.

The P1 (empty error routing to OnComplete) is a theoretical edge case since error messages are always non-empty in practice. The P2 about redundant GetSettings() is incorrect - the variable is used at both line 275 and 292.

[mvanhorn]

Fixed the Windows CI failures. TestExpandTemplate had single-quoted expected values hardcoded, but shellEscape uses double quotes on Windows per the runtime check. Rebuilt the expected strings by calling shellEscape directly so the test validates template substitution without pinning a quoting style. Passing locally on Unix and should cover Windows now (57e2b7a).

[greptile-apps]

m.DestPath instead of resolved destPath in error hook

The error handler builds a corrected destPath by falling back to existing.DestPath when m.DestPath is empty (lines 308–323), and then correctly uses that variable for RemoveIncompleteFile. The RunPostActions call on line 347 bypasses that fix and passes m.DestPath directly, so {filepath} will be an empty string in error hooks whenever the message carries no path but the DB does — the same scenario the complete handler explicitly documents ("DownloadCompleteMsg does not carry destPath, so we recover the stable final location from the DB entry").

Suggested change

	ID: m.DownloadID,
	FilePath: destPath,

Prompt To Fix With AI

This is a comment left during a code review.
Path: internal/processing/events.go
Line: 347

Comment:
**`m.DestPath` instead of resolved `destPath` in error hook**

The error handler builds a corrected `destPath` by falling back to `existing.DestPath` when `m.DestPath` is empty (lines 308–323), and then correctly uses that variable for `RemoveIncompleteFile`. The `RunPostActions` call on line 347 bypasses that fix and passes `m.DestPath` directly, so `{filepath}` will be an empty string in error hooks whenever the message carries no path but the DB does — the same scenario the complete handler explicitly documents ("DownloadCompleteMsg does not carry destPath, so we recover the stable final location from the DB entry").

```suggestion
			FilePath: destPath,
```

How can I resolve this? If you propose a fix, please make it concise.