From 9ecda7bca03655f7d92c5fae2026bd1bac910c47 Mon Sep 17 00:00:00 2001
From: garethgeorge <garethgeorge97@gmail.com>
Date: Tue, 25 Jun 2024 20:05:10 -0700
Subject: [PATCH 01/15] start refactoring hooks

---
 internal/hook/handler.go                          | 11 +++++++++++
 internal/hook/hook.go                             | 12 ++++--------
 internal/hook/hook_test.go                        |  6 +++---
 internal/orchestrator/logging/logging.go          |  2 +-
 internal/orchestrator/taskrunnerimpl.go           |  6 +++++-
 internal/{hook => orchestrator/tasks}/hookvars.go |  2 +-
 internal/orchestrator/tasks/task.go               |  3 +--
 internal/orchestrator/tasks/taskbackup.go         |  4 ++--
 internal/orchestrator/tasks/taskcheck.go          |  6 +++---
 internal/orchestrator/tasks/taskforget.go         |  3 +--
 internal/orchestrator/tasks/taskforgetsnapshot.go |  3 +--
 internal/orchestrator/tasks/taskindexsnapshots.go |  3 +--
 internal/orchestrator/tasks/taskprune.go          |  6 +++---
 internal/orchestrator/tasks/taskrestore.go        |  3 +--
 internal/orchestrator/tasks/taskstats.go          |  3 +--
 15 files changed, 39 insertions(+), 34 deletions(-)
 create mode 100644 internal/hook/handler.go
 rename internal/{hook => orchestrator/tasks}/hookvars.go (99%)

diff --git a/internal/hook/handler.go b/internal/hook/handler.go
new file mode 100644
index 00000000..403fd452
--- /dev/null
+++ b/internal/hook/handler.go
@@ -0,0 +1,11 @@
+package hook
+
+import (
+	v1 "github.com/garethgeorge/backrest/gen/go/v1"
+	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
+)
+
+type Handler interface {
+	ShouldHandle(hook *v1.Hook) bool
+	Execute(hook *v1.Hook, runner tasks.TaskRunner) error
+}
diff --git a/internal/hook/hook.go b/internal/hook/hook.go
index 2bf73411..48205c61 100644
--- a/internal/hook/hook.go
+++ b/internal/hook/hook.go
@@ -37,7 +37,7 @@ func NewHookExecutor(config *v1.Config, oplog *oplog.OpLog, bigOutputStore *rota
 
 // ExecuteHooks schedules tasks for the hooks subscribed to the given event. The vars map is used to substitute variables
 // Hooks are pulled both from the provided plan and from the repo config.
-func (e *HookExecutor) ExecuteHooks(flowID int64, repo *v1.Repo, plan *v1.Plan, events []v1.Hook_Condition, vars HookVars) error {
+func (e *HookExecutor) ExecuteHooks(flowID int64, repo *v1.Repo, plan *v1.Plan, events []v1.Hook_Condition, vars interface{}) error {
 	planId := plan.GetId()
 	if planId == "" {
 		planId = "_system_" // TODO: clean this up when refactoring hook execution
@@ -51,10 +51,6 @@ func (e *HookExecutor) ExecuteHooks(flowID int64, repo *v1.Repo, plan *v1.Plan,
 		FlowId:     flowID,
 	}
 
-	vars.Repo = repo
-	vars.Plan = plan
-	vars.CurTime = time.Now()
-
 	for idx, hook := range repo.GetHooks() {
 		h := (*Hook)(hook)
 		event := firstMatchingCondition(h, events)
@@ -119,7 +115,7 @@ func firstMatchingCondition(hook *Hook, events []v1.Hook_Condition) v1.Hook_Cond
 	return v1.Hook_CONDITION_UNKNOWN
 }
 
-func (e *HookExecutor) executeHook(op *v1.Operation, hook *Hook, event v1.Hook_Condition, vars HookVars) error {
+func (e *HookExecutor) executeHook(op *v1.Operation, hook *Hook, event v1.Hook_Condition, vars interface{}) error {
 	if err := e.oplog.Add(op); err != nil {
 		zap.S().Errorf("execute hook: add operation: %v", err)
 		return errors.New("couldn't create operation")
@@ -164,12 +160,12 @@ func curTimeMs() int64 {
 
 type Hook v1.Hook
 
-func (h *Hook) Do(event v1.Hook_Condition, vars HookVars, output io.Writer) error {
+func (h *Hook) Do(event v1.Hook_Condition, vars interface{}, output io.Writer) error {
 	if !slices.Contains(h.Conditions, event) {
 		return nil
 	}
 
-	vars.Event = event
+	vars.Event = event // TODO: add .Event to HookVars
 
 	switch action := h.Action.(type) {
 	case *v1.Hook_ActionCommand:
diff --git a/internal/hook/hook_test.go b/internal/hook/hook_test.go
index 79f6fe96..3f467578 100644
--- a/internal/hook/hook_test.go
+++ b/internal/hook/hook_test.go
@@ -20,7 +20,7 @@ func TestHookCommandInDefaultShell(t *testing.T) {
 		},
 	})
 
-	err := hook.Do(v1.Hook_CONDITION_SNAPSHOT_START, HookVars{}, &bytes.Buffer{})
+	err := hook.Do(v1.Hook_CONDITION_SNAPSHOT_START, struct{}{}, &bytes.Buffer{})
 	if err == nil {
 		t.Fatal("expected error")
 	}
@@ -49,7 +49,7 @@ exit $counter`,
 		},
 	})
 
-	err := hook.Do(v1.Hook_CONDITION_SNAPSHOT_START, HookVars{}, &bytes.Buffer{})
+	err := hook.Do(v1.Hook_CONDITION_SNAPSHOT_START, struct{}{}, &bytes.Buffer{})
 	if err == nil {
 		t.Fatal("expected error")
 	}
@@ -75,7 +75,7 @@ func TestCommandHookErrorHandling(t *testing.T) {
 		OnError: v1.Hook_ON_ERROR_CANCEL,
 	})
 
-	err := applyHookErrorPolicy(hook.OnError, hook.Do(v1.Hook_CONDITION_SNAPSHOT_START, HookVars{}, &bytes.Buffer{}))
+	err := applyHookErrorPolicy(hook.OnError, hook.Do(v1.Hook_CONDITION_SNAPSHOT_START, struct{}{}, &bytes.Buffer{}))
 	if err == nil {
 		t.Fatal("expected error")
 	}
diff --git a/internal/orchestrator/logging/logging.go b/internal/orchestrator/logging/logging.go
index 03c98f60..7554bf11 100644
--- a/internal/orchestrator/logging/logging.go
+++ b/internal/orchestrator/logging/logging.go
@@ -39,7 +39,7 @@ func Logger(ctx context.Context) *zap.Logger {
 	fe := zapcore.NewConsoleEncoder(p)
 	l := zap.New(zapcore.NewTee(
 		zap.L().Core(),
-		zapcore.NewCore(fe, zapcore.AddSync(&ioutil.LinePrefixer{W: writer, Prefix: []byte("[tasklog]")}), zapcore.DebugLevel),
+		zapcore.NewCore(fe, zapcore.AddSync(&ioutil.LinePrefixer{W: writer, Prefix: []byte("[tasklog] ")}), zapcore.DebugLevel),
 	))
 	return l
 }
diff --git a/internal/orchestrator/taskrunnerimpl.go b/internal/orchestrator/taskrunnerimpl.go
index 396aa143..b65c4c68 100644
--- a/internal/orchestrator/taskrunnerimpl.go
+++ b/internal/orchestrator/taskrunnerimpl.go
@@ -69,12 +69,14 @@ func (t *taskRunnerImpl) OpLog() *oplog.OpLog {
 	return t.orchestrator.OpLog
 }
 
-func (t *taskRunnerImpl) ExecuteHooks(events []v1.Hook_Condition, vars hook.HookVars) error {
+func (t *taskRunnerImpl) ExecuteHooks(events []v1.Hook_Condition, vars tasks.HookVars) error {
 	vars.Task = t.t.Name()
 	if t.op != nil {
 		vars.Duration = time.Since(time.UnixMilli(t.op.UnixTimeStartMs))
 	}
 
+	vars.CurTime = time.Now()
+
 	repoID := t.t.RepoID()
 	planID := t.t.PlanID()
 	var repo *v1.Repo
@@ -85,9 +87,11 @@ func (t *taskRunnerImpl) ExecuteHooks(events []v1.Hook_Condition, vars hook.Hook
 		if err != nil {
 			return err
 		}
+		vars.Repo = repo
 	}
 	if planID != "" {
 		plan, _ = t.FindPlan()
+		vars.Plan = plan
 	}
 	var flowID int64
 	if t.op != nil {
diff --git a/internal/hook/hookvars.go b/internal/orchestrator/tasks/hookvars.go
similarity index 99%
rename from internal/hook/hookvars.go
rename to internal/orchestrator/tasks/hookvars.go
index f6f0b5a7..cc80d982 100644
--- a/internal/hook/hookvars.go
+++ b/internal/orchestrator/tasks/hookvars.go
@@ -1,4 +1,4 @@
-package hook
+package tasks
 
 import (
 	"bytes"
diff --git a/internal/orchestrator/tasks/task.go b/internal/orchestrator/tasks/task.go
index c3b904cd..fa0d3e79 100644
--- a/internal/orchestrator/tasks/task.go
+++ b/internal/orchestrator/tasks/task.go
@@ -5,7 +5,6 @@ import (
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-	"github.com/garethgeorge/backrest/internal/hook"
 	"github.com/garethgeorge/backrest/internal/oplog"
 	"github.com/garethgeorge/backrest/internal/orchestrator/repo"
 	"go.uber.org/zap"
@@ -35,7 +34,7 @@ type TaskRunner interface {
 	// UpdateOperation updates the operation in storage. It must be called after CreateOperation.
 	UpdateOperation(*v1.Operation) error
 	// ExecuteHooks
-	ExecuteHooks(events []v1.Hook_Condition, vars hook.HookVars) error
+	ExecuteHooks(events []v1.Hook_Condition, vars HookVars) error
 	// OpLog returns the oplog for the operations.
 	OpLog() *oplog.OpLog
 	// GetRepo returns the repo with the given ID.
diff --git a/internal/orchestrator/tasks/taskbackup.go b/internal/orchestrator/tasks/taskbackup.go
index bd44c0f5..dda8d72d 100644
--- a/internal/orchestrator/tasks/taskbackup.go
+++ b/internal/orchestrator/tasks/taskbackup.go
@@ -108,7 +108,7 @@ func (t *BackupTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunne
 
 	if err := runner.ExecuteHooks([]v1.Hook_Condition{
 		v1.Hook_CONDITION_SNAPSHOT_START,
-	}, hook.HookVars{}); err != nil {
+	}, HookVars{}); err != nil {
 		var cancelErr *hook.HookErrorRequestCancel
 		if errors.As(err, &cancelErr) {
 			op.Status = v1.OperationStatus_STATUS_USER_CANCELLED // user visible cancelled status
@@ -169,7 +169,7 @@ func (t *BackupTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunne
 		summary = &restic.BackupProgressEntry{}
 	}
 
-	vars := hook.HookVars{
+	vars := HookVars{
 		Task:          t.Name(),
 		SnapshotStats: summary,
 		SnapshotId:    summary.SnapshotId,
diff --git a/internal/orchestrator/tasks/taskcheck.go b/internal/orchestrator/tasks/taskcheck.go
index e0243c02..d793acbf 100644
--- a/internal/orchestrator/tasks/taskcheck.go
+++ b/internal/orchestrator/tasks/taskcheck.go
@@ -103,7 +103,7 @@ func (t *CheckTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunner
 
 	if err := runner.ExecuteHooks([]v1.Hook_Condition{
 		v1.Hook_CONDITION_CHECK_START,
-	}, hook.HookVars{}); err != nil {
+	}, HookVars{}); err != nil {
 		// TODO: generalize this logic
 		op.DisplayMessage = err.Error()
 		var cancelErr *hook.HookErrorRequestCancel
@@ -160,7 +160,7 @@ func (t *CheckTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunner
 		runner.ExecuteHooks([]v1.Hook_Condition{
 			v1.Hook_CONDITION_CHECK_ERROR,
 			v1.Hook_CONDITION_ANY_ERROR,
-		}, hook.HookVars{
+		}, HookVars{
 			Error: err.Error(),
 		})
 
@@ -178,7 +178,7 @@ func (t *CheckTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunner
 
 	if err := runner.ExecuteHooks([]v1.Hook_Condition{
 		v1.Hook_CONDITION_CHECK_SUCCESS,
-	}, hook.HookVars{}); err != nil {
+	}, HookVars{}); err != nil {
 		return fmt.Errorf("execute prune success hooks: %w", err)
 	}
 
diff --git a/internal/orchestrator/tasks/taskforget.go b/internal/orchestrator/tasks/taskforget.go
index bee1cfdd..0e1b8015 100644
--- a/internal/orchestrator/tasks/taskforget.go
+++ b/internal/orchestrator/tasks/taskforget.go
@@ -6,7 +6,6 @@ import (
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-	"github.com/garethgeorge/backrest/internal/hook"
 	"github.com/garethgeorge/backrest/internal/oplog"
 	"github.com/garethgeorge/backrest/internal/oplog/indexutil"
 	"github.com/garethgeorge/backrest/internal/orchestrator/repo"
@@ -38,7 +37,7 @@ func NewOneoffForgetTask(repoID, planID string, flowID int64, at time.Time) Task
 			if err := forgetHelper(ctx, st, taskRunner); err != nil {
 				taskRunner.ExecuteHooks([]v1.Hook_Condition{
 					v1.Hook_CONDITION_ANY_ERROR,
-				}, hook.HookVars{
+				}, HookVars{
 					Error: err.Error(),
 				})
 				return err
diff --git a/internal/orchestrator/tasks/taskforgetsnapshot.go b/internal/orchestrator/tasks/taskforgetsnapshot.go
index 8d565c9b..5e81d0b6 100644
--- a/internal/orchestrator/tasks/taskforgetsnapshot.go
+++ b/internal/orchestrator/tasks/taskforgetsnapshot.go
@@ -6,7 +6,6 @@ import (
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-	"github.com/garethgeorge/backrest/internal/hook"
 )
 
 func NewOneoffForgetSnapshotTask(repoID, planID string, flowID int64, at time.Time, snapshotID string) Task {
@@ -33,7 +32,7 @@ func NewOneoffForgetSnapshotTask(repoID, planID string, flowID int64, at time.Ti
 			if err := forgetSnapshotHelper(ctx, st, taskRunner, snapshotID); err != nil {
 				taskRunner.ExecuteHooks([]v1.Hook_Condition{
 					v1.Hook_CONDITION_ANY_ERROR,
-				}, hook.HookVars{
+				}, HookVars{
 					Error: err.Error(),
 				})
 				return err
diff --git a/internal/orchestrator/tasks/taskindexsnapshots.go b/internal/orchestrator/tasks/taskindexsnapshots.go
index 40a80d53..2a07f209 100644
--- a/internal/orchestrator/tasks/taskindexsnapshots.go
+++ b/internal/orchestrator/tasks/taskindexsnapshots.go
@@ -8,7 +8,6 @@ import (
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-	"github.com/garethgeorge/backrest/internal/hook"
 	"github.com/garethgeorge/backrest/internal/oplog"
 	"github.com/garethgeorge/backrest/internal/oplog/indexutil"
 	"github.com/garethgeorge/backrest/internal/orchestrator/repo"
@@ -31,7 +30,7 @@ func NewOneoffIndexSnapshotsTask(repoID string, at time.Time) Task {
 			if err := indexSnapshotsHelper(ctx, st, taskRunner); err != nil {
 				taskRunner.ExecuteHooks([]v1.Hook_Condition{
 					v1.Hook_CONDITION_ANY_ERROR,
-				}, hook.HookVars{
+				}, HookVars{
 					Task:  st.Task.Name(),
 					Error: err.Error(),
 				})
diff --git a/internal/orchestrator/tasks/taskprune.go b/internal/orchestrator/tasks/taskprune.go
index 526aca99..39f079d0 100644
--- a/internal/orchestrator/tasks/taskprune.go
+++ b/internal/orchestrator/tasks/taskprune.go
@@ -103,7 +103,7 @@ func (t *PruneTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunner
 
 	if err := runner.ExecuteHooks([]v1.Hook_Condition{
 		v1.Hook_CONDITION_PRUNE_START,
-	}, hook.HookVars{}); err != nil {
+	}, HookVars{}); err != nil {
 		op.DisplayMessage = err.Error()
 		// TODO: generalize this logic
 		var cancelErr *hook.HookErrorRequestCancel
@@ -159,7 +159,7 @@ func (t *PruneTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunner
 
 		runner.ExecuteHooks([]v1.Hook_Condition{
 			v1.Hook_CONDITION_ANY_ERROR,
-		}, hook.HookVars{
+		}, HookVars{
 			Error: err.Error(),
 		})
 
@@ -177,7 +177,7 @@ func (t *PruneTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunner
 
 	if err := runner.ExecuteHooks([]v1.Hook_Condition{
 		v1.Hook_CONDITION_PRUNE_SUCCESS,
-	}, hook.HookVars{}); err != nil {
+	}, HookVars{}); err != nil {
 		return fmt.Errorf("execute prune end hooks: %w", err)
 	}
 
diff --git a/internal/orchestrator/tasks/taskrestore.go b/internal/orchestrator/tasks/taskrestore.go
index 478eb89f..f55f95b7 100644
--- a/internal/orchestrator/tasks/taskrestore.go
+++ b/internal/orchestrator/tasks/taskrestore.go
@@ -8,7 +8,6 @@ import (
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-	"github.com/garethgeorge/backrest/internal/hook"
 	"go.uber.org/zap"
 )
 
@@ -36,7 +35,7 @@ func NewOneoffRestoreTask(repoID, planID string, flowID int64, at time.Time, sna
 			if err := restoreHelper(ctx, st, taskRunner, snapshotID, path, target); err != nil {
 				taskRunner.ExecuteHooks([]v1.Hook_Condition{
 					v1.Hook_CONDITION_ANY_ERROR,
-				}, hook.HookVars{
+				}, HookVars{
 					Task:  st.Task.Name(),
 					Error: err.Error(),
 				})
diff --git a/internal/orchestrator/tasks/taskstats.go b/internal/orchestrator/tasks/taskstats.go
index 3dcb405f..0372fe31 100644
--- a/internal/orchestrator/tasks/taskstats.go
+++ b/internal/orchestrator/tasks/taskstats.go
@@ -6,7 +6,6 @@ import (
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-	"github.com/garethgeorge/backrest/internal/hook"
 	"github.com/garethgeorge/backrest/internal/oplog"
 	"github.com/garethgeorge/backrest/internal/oplog/indexutil"
 )
@@ -72,7 +71,7 @@ func (t *StatsTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunner
 	if err := statsHelper(ctx, st, runner); err != nil {
 		runner.ExecuteHooks([]v1.Hook_Condition{
 			v1.Hook_CONDITION_ANY_ERROR,
-		}, hook.HookVars{
+		}, HookVars{
 			Task:  st.Task.Name(),
 			Error: err.Error(),
 		})

From 9325fbcb4727290f02731633e7c80e11ab364f82 Mon Sep 17 00:00:00 2001
From: garethgeorge <garethgeorge97@gmail.com>
Date: Fri, 28 Jun 2024 20:59:49 -0700
Subject: [PATCH 02/15] cont'd hook refactoring

---
 internal/hook/hook.go                     |  7 ++++++-
 internal/orchestrator/orchestrator.go     |  5 ++---
 internal/orchestrator/taskrunnerimpl.go   | 10 +++++++++-
 internal/orchestrator/tasks/errors.go     |  8 ++++++++
 internal/orchestrator/tasks/taskbackup.go |  9 +--------
 webui/src/components/OperationTree.tsx    |  4 ++--
 6 files changed, 28 insertions(+), 15 deletions(-)
 create mode 100644 internal/orchestrator/tasks/errors.go

diff --git a/internal/hook/hook.go b/internal/hook/hook.go
index 48205c61..246e845b 100644
--- a/internal/hook/hook.go
+++ b/internal/hook/hook.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"reflect"
 	"slices"
 	"strings"
 	"text/template"
@@ -165,7 +166,11 @@ func (h *Hook) Do(event v1.Hook_Condition, vars interface{}, output io.Writer) e
 		return nil
 	}
 
-	vars.Event = event // TODO: add .Event to HookVars
+	// if vars has a .Event key set it to the event
+	// this is a bit of a hack to allow the event to be used in the template
+	if eventField := reflect.ValueOf(vars).FieldByName("Event"); eventField.IsValid() {
+		eventField.Set(reflect.ValueOf(event))
+	}
 
 	switch action := h.Action.(type) {
 	case *v1.Hook_ActionCommand:
diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go
index d62af28d..dc64b277 100644
--- a/internal/orchestrator/orchestrator.go
+++ b/internal/orchestrator/orchestrator.go
@@ -341,12 +341,11 @@ func (o *Orchestrator) Run(ctx context.Context) {
 					op.Logref = ref
 				}
 			}
-
 			if err != nil {
-				if taskCtx.Err() != nil {
+				if taskCtx.Err() != nil || errors.Is(err, tasks.ErrTaskCancelled) {
 					// task was cancelled
 					op.Status = v1.OperationStatus_STATUS_USER_CANCELLED
-				} else {
+				} else if errors.Is(err, tasks.ErrTaskFailed) {
 					op.Status = v1.OperationStatus_STATUS_ERROR
 				}
 				op.DisplayMessage = err.Error()
diff --git a/internal/orchestrator/taskrunnerimpl.go b/internal/orchestrator/taskrunnerimpl.go
index b65c4c68..e4173f92 100644
--- a/internal/orchestrator/taskrunnerimpl.go
+++ b/internal/orchestrator/taskrunnerimpl.go
@@ -2,6 +2,8 @@ package orchestrator
 
 import (
 	"context"
+	"errors"
+	"fmt"
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
@@ -97,8 +99,14 @@ func (t *taskRunnerImpl) ExecuteHooks(events []v1.Hook_Condition, vars tasks.Hoo
 	if t.op != nil {
 		flowID = t.op.FlowId
 	}
+
 	executor := hook.NewHookExecutor(t.Config(), t.orchestrator.OpLog, t.orchestrator.logStore)
-	return executor.ExecuteHooks(flowID, repo, plan, events, vars)
+	err := executor.ExecuteHooks(flowID, repo, plan, events, vars)
+	var cancelErr *hook.HookErrorRequestCancel
+	if errors.As(err, &cancelErr) {
+		return fmt.Errorf("%w: %w", tasks.ErrTaskCancelled, err)
+	}
+	return err
 }
 
 func (t *taskRunnerImpl) GetRepo(repoID string) (*v1.Repo, error) {
diff --git a/internal/orchestrator/tasks/errors.go b/internal/orchestrator/tasks/errors.go
new file mode 100644
index 00000000..ca02b9f4
--- /dev/null
+++ b/internal/orchestrator/tasks/errors.go
@@ -0,0 +1,8 @@
+package tasks
+
+import "errors"
+
+// ErrTaskCancelled signals that the task is beign cancelled gracefully.
+// This error is handled by marking the task as user cancelled.
+// By default a task returning an error will be marked as failed otherwise.
+var ErrTaskCancelled = errors.New("cancel task")
diff --git a/internal/orchestrator/tasks/taskbackup.go b/internal/orchestrator/tasks/taskbackup.go
index dda8d72d..5d126e69 100644
--- a/internal/orchestrator/tasks/taskbackup.go
+++ b/internal/orchestrator/tasks/taskbackup.go
@@ -9,7 +9,6 @@ import (
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-	"github.com/garethgeorge/backrest/internal/hook"
 	"github.com/garethgeorge/backrest/internal/protoutil"
 	"github.com/garethgeorge/backrest/pkg/restic"
 	"go.uber.org/zap"
@@ -109,13 +108,7 @@ func (t *BackupTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunne
 	if err := runner.ExecuteHooks([]v1.Hook_Condition{
 		v1.Hook_CONDITION_SNAPSHOT_START,
 	}, HookVars{}); err != nil {
-		var cancelErr *hook.HookErrorRequestCancel
-		if errors.As(err, &cancelErr) {
-			op.Status = v1.OperationStatus_STATUS_USER_CANCELLED // user visible cancelled status
-			op.DisplayMessage = err.Error()
-			return nil
-		}
-		return fmt.Errorf("hook failed: %w", err)
+		return fmt.Errorf("snapshot start hook failed: %w", err)
 	}
 
 	var sendWg sync.WaitGroup
diff --git a/webui/src/components/OperationTree.tsx b/webui/src/components/OperationTree.tsx
index 521c6520..b3001a67 100644
--- a/webui/src/components/OperationTree.tsx
+++ b/webui/src/components/OperationTree.tsx
@@ -128,10 +128,10 @@ export const OperationTree = ({
         }
       }}
       titleRender={(node: OpTreeNode): React.ReactNode => {
-        if (node.title) {
+        if (node.title !== undefined) {
           return <>{node.title}</>;
         }
-        if (node.backup) {
+        if (node.backup !== undefined) {
           const b = node.backup;
           const details: string[] = [];
 

From b13f25f7539bc4ae068b67c767e0febd73f15e90 Mon Sep 17 00:00:00 2001
From: garethgeorge <garethgeorge97@gmail.com>
Date: Fri, 28 Jun 2024 21:26:21 -0700
Subject: [PATCH 03/15] fixed build

---
 internal/hook/hook.go                     |  4 ++--
 internal/hook/hookcommand.go              |  2 +-
 internal/hook/hookdiscord.go              |  2 +-
 internal/hook/hookgotify.go               |  2 +-
 internal/hook/hookshoutrrr.go             |  2 +-
 internal/hook/hookslack.go                |  2 +-
 internal/orchestrator/orchestrator.go     |  2 +-
 internal/orchestrator/tasks/taskbackup.go |  2 +-
 internal/orchestrator/tasks/taskcheck.go  | 11 +----------
 internal/orchestrator/tasks/taskprune.go  | 11 +----------
 10 files changed, 11 insertions(+), 29 deletions(-)

diff --git a/internal/hook/hook.go b/internal/hook/hook.go
index 246e845b..e2d8721d 100644
--- a/internal/hook/hook.go
+++ b/internal/hook/hook.go
@@ -188,7 +188,7 @@ func (h *Hook) Do(event v1.Hook_Condition, vars interface{}, output io.Writer) e
 	}
 }
 
-func (h *Hook) renderTemplate(text string, vars HookVars) (string, error) {
+func (h *Hook) renderTemplate(text string, vars interface{}) (string, error) {
 	template, err := template.New("template").Parse(text)
 	if err != nil {
 		return "", fmt.Errorf("parse template: %w", err)
@@ -202,7 +202,7 @@ func (h *Hook) renderTemplate(text string, vars HookVars) (string, error) {
 	return buf.String(), nil
 }
 
-func (h *Hook) renderTemplateOrDefault(template string, defaultTmpl string, vars HookVars) (string, error) {
+func (h *Hook) renderTemplateOrDefault(template string, defaultTmpl string, vars interface{}) (string, error) {
 	if strings.Trim(template, " ") == "" {
 		return h.renderTemplate(defaultTmpl, vars)
 	}
diff --git a/internal/hook/hookcommand.go b/internal/hook/hookcommand.go
index e677a9de..96a67793 100644
--- a/internal/hook/hookcommand.go
+++ b/internal/hook/hookcommand.go
@@ -9,7 +9,7 @@ import (
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
 )
 
-func (h *Hook) doCommand(cmd *v1.Hook_ActionCommand, vars HookVars, output io.Writer) error {
+func (h *Hook) doCommand(cmd *v1.Hook_ActionCommand, vars interface{}, output io.Writer) error {
 	command, err := h.renderTemplate(cmd.ActionCommand.Command, vars)
 	if err != nil {
 		return fmt.Errorf("template rendering: %w", err)
diff --git a/internal/hook/hookdiscord.go b/internal/hook/hookdiscord.go
index a53e7c11..7afd4161 100644
--- a/internal/hook/hookdiscord.go
+++ b/internal/hook/hookdiscord.go
@@ -9,7 +9,7 @@ import (
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
 )
 
-func (h *Hook) doDiscord(cmd *v1.Hook_ActionDiscord, vars HookVars, output io.Writer) error {
+func (h *Hook) doDiscord(cmd *v1.Hook_ActionDiscord, vars interface{}, output io.Writer) error {
 	payload, err := h.renderTemplateOrDefault(cmd.ActionDiscord.GetTemplate(), defaultTemplate, vars)
 	if err != nil {
 		return fmt.Errorf("template rendering: %w", err)
diff --git a/internal/hook/hookgotify.go b/internal/hook/hookgotify.go
index bf8f2808..e7784403 100644
--- a/internal/hook/hookgotify.go
+++ b/internal/hook/hookgotify.go
@@ -11,7 +11,7 @@ import (
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
 )
 
-func (h *Hook) doGotify(cmd *v1.Hook_ActionGotify, vars HookVars, output io.Writer) error {
+func (h *Hook) doGotify(cmd *v1.Hook_ActionGotify, vars interface{}, output io.Writer) error {
 	payload, err := h.renderTemplateOrDefault(cmd.ActionGotify.GetTemplate(), defaultTemplate, vars)
 	if err != nil {
 		return fmt.Errorf("template rendering: %w", err)
diff --git a/internal/hook/hookshoutrrr.go b/internal/hook/hookshoutrrr.go
index 36f20af1..d55b4479 100644
--- a/internal/hook/hookshoutrrr.go
+++ b/internal/hook/hookshoutrrr.go
@@ -8,7 +8,7 @@ import (
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
 )
 
-func (h *Hook) doShoutrrr(cmd *v1.Hook_ActionShoutrrr, vars HookVars, output io.Writer) error {
+func (h *Hook) doShoutrrr(cmd *v1.Hook_ActionShoutrrr, vars interface{}, output io.Writer) error {
 	payload, err := h.renderTemplateOrDefault(cmd.ActionShoutrrr.GetTemplate(), defaultTemplate, vars)
 	if err != nil {
 		return fmt.Errorf("template rendering: %w", err)
diff --git a/internal/hook/hookslack.go b/internal/hook/hookslack.go
index 075115f0..dc6dec70 100644
--- a/internal/hook/hookslack.go
+++ b/internal/hook/hookslack.go
@@ -9,7 +9,7 @@ import (
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
 )
 
-func (h *Hook) doSlack(cmd *v1.Hook_ActionSlack, vars HookVars, output io.Writer) error {
+func (h *Hook) doSlack(cmd *v1.Hook_ActionSlack, vars interface{}, output io.Writer) error {
 	payload, err := h.renderTemplateOrDefault(cmd.ActionSlack.GetTemplate(), defaultTemplate, vars)
 	if err != nil {
 		return fmt.Errorf("template rendering: %w", err)
diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go
index dc64b277..c5082001 100644
--- a/internal/orchestrator/orchestrator.go
+++ b/internal/orchestrator/orchestrator.go
@@ -345,7 +345,7 @@ func (o *Orchestrator) Run(ctx context.Context) {
 				if taskCtx.Err() != nil || errors.Is(err, tasks.ErrTaskCancelled) {
 					// task was cancelled
 					op.Status = v1.OperationStatus_STATUS_USER_CANCELLED
-				} else if errors.Is(err, tasks.ErrTaskFailed) {
+				} else if err != nil {
 					op.Status = v1.OperationStatus_STATUS_ERROR
 				}
 				op.DisplayMessage = err.Error()
diff --git a/internal/orchestrator/tasks/taskbackup.go b/internal/orchestrator/tasks/taskbackup.go
index 5d126e69..0429bb03 100644
--- a/internal/orchestrator/tasks/taskbackup.go
+++ b/internal/orchestrator/tasks/taskbackup.go
@@ -108,7 +108,7 @@ func (t *BackupTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunne
 	if err := runner.ExecuteHooks([]v1.Hook_Condition{
 		v1.Hook_CONDITION_SNAPSHOT_START,
 	}, HookVars{}); err != nil {
-		return fmt.Errorf("snapshot start hook failed: %w", err)
+		return fmt.Errorf("snapshot start hook: %w", err)
 	}
 
 	var sendWg sync.WaitGroup
diff --git a/internal/orchestrator/tasks/taskcheck.go b/internal/orchestrator/tasks/taskcheck.go
index d793acbf..cd300635 100644
--- a/internal/orchestrator/tasks/taskcheck.go
+++ b/internal/orchestrator/tasks/taskcheck.go
@@ -9,7 +9,6 @@ import (
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-	"github.com/garethgeorge/backrest/internal/hook"
 	"github.com/garethgeorge/backrest/internal/ioutil"
 	"github.com/garethgeorge/backrest/internal/oplog"
 	"github.com/garethgeorge/backrest/internal/oplog/indexutil"
@@ -104,15 +103,7 @@ func (t *CheckTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunner
 	if err := runner.ExecuteHooks([]v1.Hook_Condition{
 		v1.Hook_CONDITION_CHECK_START,
 	}, HookVars{}); err != nil {
-		// TODO: generalize this logic
-		op.DisplayMessage = err.Error()
-		var cancelErr *hook.HookErrorRequestCancel
-		if errors.As(err, &cancelErr) {
-			op.Status = v1.OperationStatus_STATUS_USER_CANCELLED // user visible cancelled status
-			return nil
-		}
-		op.Status = v1.OperationStatus_STATUS_ERROR
-		return fmt.Errorf("execute check start hooks: %w", err)
+		return fmt.Errorf("check start hook: %w", err)
 	}
 
 	err = repo.UnlockIfAutoEnabled(ctx)
diff --git a/internal/orchestrator/tasks/taskprune.go b/internal/orchestrator/tasks/taskprune.go
index 39f079d0..3f16c729 100644
--- a/internal/orchestrator/tasks/taskprune.go
+++ b/internal/orchestrator/tasks/taskprune.go
@@ -9,7 +9,6 @@ import (
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-	"github.com/garethgeorge/backrest/internal/hook"
 	"github.com/garethgeorge/backrest/internal/ioutil"
 	"github.com/garethgeorge/backrest/internal/oplog"
 	"github.com/garethgeorge/backrest/internal/oplog/indexutil"
@@ -104,15 +103,7 @@ func (t *PruneTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunner
 	if err := runner.ExecuteHooks([]v1.Hook_Condition{
 		v1.Hook_CONDITION_PRUNE_START,
 	}, HookVars{}); err != nil {
-		op.DisplayMessage = err.Error()
-		// TODO: generalize this logic
-		var cancelErr *hook.HookErrorRequestCancel
-		if errors.As(err, &cancelErr) {
-			op.Status = v1.OperationStatus_STATUS_USER_CANCELLED // user visible cancelled status
-			return nil
-		}
-		op.Status = v1.OperationStatus_STATUS_ERROR
-		return fmt.Errorf("execute prune start hooks: %w", err)
+		return fmt.Errorf("prune start hook: %w", err)
 	}
 
 	err = repo.UnlockIfAutoEnabled(ctx)

From 76fb64aa68a929ac657a0097c6292cde54d530f0 Mon Sep 17 00:00:00 2001
From: garethgeorge <garethgeorge97@gmail.com>
Date: Fri, 28 Jun 2024 22:02:41 -0700
Subject: [PATCH 04/15] migrate handlers to new interface

---
 internal/hook/handler.go                      | 36 ++++++++++-
 internal/hook/hook.go                         | 39 +-----------
 internal/hook/hookcommand.go                  | 40 ------------
 internal/hook/hookdiscord.go                  | 33 ----------
 internal/hook/hookshoutrrr.go                 | 24 --------
 internal/hook/hookslack.go                    | 33 ----------
 internal/hook/httputil.go                     |  2 +-
 internal/hook/templateutil.go                 | 29 +++++++++
 internal/hook/types/command.go                | 61 +++++++++++++++++++
 internal/hook/types/discord.go                | 46 ++++++++++++++
 .../hook/{hookgotify.go => types/gotify.go}   | 29 ++++++---
 internal/hook/types/shoutrrr.go               | 39 ++++++++++++
 internal/hook/types/slack.go                  | 47 ++++++++++++++
 internal/orchestrator/taskrunnerimpl.go       |  5 ++
 internal/orchestrator/tasks/task.go           |  3 +
 15 files changed, 287 insertions(+), 179 deletions(-)
 delete mode 100644 internal/hook/hookcommand.go
 delete mode 100644 internal/hook/hookdiscord.go
 delete mode 100644 internal/hook/hookshoutrrr.go
 delete mode 100644 internal/hook/hookslack.go
 create mode 100644 internal/hook/templateutil.go
 create mode 100644 internal/hook/types/command.go
 create mode 100644 internal/hook/types/discord.go
 rename internal/hook/{hookgotify.go => types/gotify.go} (52%)
 create mode 100644 internal/hook/types/shoutrrr.go
 create mode 100644 internal/hook/types/slack.go

diff --git a/internal/hook/handler.go b/internal/hook/handler.go
index 403fd452..d3f2c525 100644
--- a/internal/hook/handler.go
+++ b/internal/hook/handler.go
@@ -1,11 +1,43 @@
 package hook
 
 import (
+	"context"
+	"errors"
+	"reflect"
+
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
 	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
 )
 
+var ErrHandlerNotFound = errors.New("handler not found")
+
+// defaultRegistry is the default handler registry.
+var defaultRegistry = &HandlerRegistry{
+	actionHandlers: make(map[reflect.Type]Handler),
+}
+
+func DefaultRegistry() *HandlerRegistry {
+	return defaultRegistry
+}
+
+type HandlerRegistry struct {
+	actionHandlers map[reflect.Type]Handler
+}
+
+// RegisterHandler registers a handler with the default registry.
+func (r *HandlerRegistry) RegisterHandler(handler Handler) {
+	r.actionHandlers[handler.ActionType()] = handler
+}
+
+func (r *HandlerRegistry) GetHandler(hook *v1.Hook) (Handler, error) {
+	handler, ok := r.actionHandlers[reflect.TypeOf(hook.Action)]
+	if !ok {
+		return nil, ErrHandlerNotFound
+	}
+	return handler, nil
+}
+
 type Handler interface {
-	ShouldHandle(hook *v1.Hook) bool
-	Execute(hook *v1.Hook, runner tasks.TaskRunner) error
+	Execute(ctx context.Context, hook *v1.Hook, vars interface{}, runner tasks.TaskRunner) error
+	ActionType() reflect.Type
 }
diff --git a/internal/hook/hook.go b/internal/hook/hook.go
index e2d8721d..228490ff 100644
--- a/internal/hook/hook.go
+++ b/internal/hook/hook.go
@@ -7,8 +7,6 @@ import (
 	"io"
 	"reflect"
 	"slices"
-	"strings"
-	"text/template"
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
@@ -19,7 +17,7 @@ import (
 )
 
 var (
-	defaultTemplate = `{{ .Summary }}`
+	DefaultTemplate = `{{ .Summary }}`
 )
 
 type HookExecutor struct {
@@ -172,41 +170,6 @@ func (h *Hook) Do(event v1.Hook_Condition, vars interface{}, output io.Writer) e
 		eventField.Set(reflect.ValueOf(event))
 	}
 
-	switch action := h.Action.(type) {
-	case *v1.Hook_ActionCommand:
-		return h.doCommand(action, vars, output)
-	case *v1.Hook_ActionDiscord:
-		return h.doDiscord(action, vars, output)
-	case *v1.Hook_ActionGotify:
-		return h.doGotify(action, vars, output)
-	case *v1.Hook_ActionSlack:
-		return h.doSlack(action, vars, output)
-	case *v1.Hook_ActionShoutrrr:
-		return h.doShoutrrr(action, vars, output)
-	default:
-		return fmt.Errorf("unknown hook action: %v", action)
-	}
-}
-
-func (h *Hook) renderTemplate(text string, vars interface{}) (string, error) {
-	template, err := template.New("template").Parse(text)
-	if err != nil {
-		return "", fmt.Errorf("parse template: %w", err)
-	}
-
-	buf := &bytes.Buffer{}
-	if err := template.Execute(buf, vars); err != nil {
-		return "", fmt.Errorf("execute template: %w", err)
-	}
-
-	return buf.String(), nil
-}
-
-func (h *Hook) renderTemplateOrDefault(template string, defaultTmpl string, vars interface{}) (string, error) {
-	if strings.Trim(template, " ") == "" {
-		return h.renderTemplate(defaultTmpl, vars)
-	}
-	return h.renderTemplate(template, vars)
 }
 
 func applyHookErrorPolicy(onError v1.Hook_OnError, err error) error {
diff --git a/internal/hook/hookcommand.go b/internal/hook/hookcommand.go
deleted file mode 100644
index 96a67793..00000000
--- a/internal/hook/hookcommand.go
+++ /dev/null
@@ -1,40 +0,0 @@
-package hook
-
-import (
-	"fmt"
-	"io"
-	"os/exec"
-	"strings"
-
-	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-)
-
-func (h *Hook) doCommand(cmd *v1.Hook_ActionCommand, vars interface{}, output io.Writer) error {
-	command, err := h.renderTemplate(cmd.ActionCommand.Command, vars)
-	if err != nil {
-		return fmt.Errorf("template rendering: %w", err)
-	}
-
-	// Parse out the shell to use if a #! prefix is present
-	shell := "sh"
-	if len(command) > 2 && command[0:2] == "#!" {
-		nextLine := strings.Index(command, "\n")
-		if nextLine == -1 {
-			nextLine = len(command)
-		}
-		shell = strings.Trim(command[2:nextLine], " ")
-		command = command[nextLine+1:]
-	}
-
-	output.Write([]byte(fmt.Sprintf("------- script -------\n#! %v\n%v\n", shell, command)))
-	output.Write([]byte("------- output -------\n"))
-
-	// Run the command in the specified shell
-	execCmd := exec.Command(shell)
-	execCmd.Stdin = strings.NewReader(command)
-
-	execCmd.Stderr = output
-	execCmd.Stdout = output
-
-	return execCmd.Run()
-}
diff --git a/internal/hook/hookdiscord.go b/internal/hook/hookdiscord.go
deleted file mode 100644
index 7afd4161..00000000
--- a/internal/hook/hookdiscord.go
+++ /dev/null
@@ -1,33 +0,0 @@
-package hook
-
-import (
-	"bytes"
-	"encoding/json"
-	"fmt"
-	"io"
-
-	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-)
-
-func (h *Hook) doDiscord(cmd *v1.Hook_ActionDiscord, vars interface{}, output io.Writer) error {
-	payload, err := h.renderTemplateOrDefault(cmd.ActionDiscord.GetTemplate(), defaultTemplate, vars)
-	if err != nil {
-		return fmt.Errorf("template rendering: %w", err)
-	}
-
-	type Message struct {
-		Content string `json:"content"`
-	}
-
-	request := Message{
-		Content: payload, // leading newline looks better in discord.
-	}
-
-	requestBytes, _ := json.Marshal(request)
-
-	fmt.Fprintf(output, "Sending Discord message to %s\n---- payload ----\n", cmd.ActionDiscord.GetWebhookUrl())
-	output.Write(requestBytes)
-
-	_, err = post(cmd.ActionDiscord.GetWebhookUrl(), "application/json", bytes.NewReader(requestBytes))
-	return err
-}
diff --git a/internal/hook/hookshoutrrr.go b/internal/hook/hookshoutrrr.go
deleted file mode 100644
index d55b4479..00000000
--- a/internal/hook/hookshoutrrr.go
+++ /dev/null
@@ -1,24 +0,0 @@
-package hook
-
-import (
-	"fmt"
-	"io"
-
-	"github.com/containrrr/shoutrrr"
-	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-)
-
-func (h *Hook) doShoutrrr(cmd *v1.Hook_ActionShoutrrr, vars interface{}, output io.Writer) error {
-	payload, err := h.renderTemplateOrDefault(cmd.ActionShoutrrr.GetTemplate(), defaultTemplate, vars)
-	if err != nil {
-		return fmt.Errorf("template rendering: %w", err)
-	}
-
-	fmt.Fprintf(output, "Sending notification to %s\nContents:\n", cmd.ActionShoutrrr.GetShoutrrrUrl())
-	output.Write([]byte(payload))
-
-	if err := shoutrrr.Send(cmd.ActionShoutrrr.GetShoutrrrUrl(), payload); err != nil {
-		return fmt.Errorf("send notification to %q: %w", cmd.ActionShoutrrr.GetShoutrrrUrl(), err)
-	}
-	return nil
-}
diff --git a/internal/hook/hookslack.go b/internal/hook/hookslack.go
deleted file mode 100644
index dc6dec70..00000000
--- a/internal/hook/hookslack.go
+++ /dev/null
@@ -1,33 +0,0 @@
-package hook
-
-import (
-	"bytes"
-	"encoding/json"
-	"fmt"
-	"io"
-
-	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-)
-
-func (h *Hook) doSlack(cmd *v1.Hook_ActionSlack, vars interface{}, output io.Writer) error {
-	payload, err := h.renderTemplateOrDefault(cmd.ActionSlack.GetTemplate(), defaultTemplate, vars)
-	if err != nil {
-		return fmt.Errorf("template rendering: %w", err)
-	}
-
-	type Message struct {
-		Text string `json:"text"`
-	}
-
-	request := Message{
-		Text: "Backrest Notification\n" + payload, // leading newline looks better in discord.
-	}
-
-	requestBytes, _ := json.Marshal(request)
-
-	fmt.Fprintf(output, "Sending Slack message to %s\n---- payload ----\n", cmd.ActionSlack.GetWebhookUrl())
-	output.Write(requestBytes)
-
-	_, err = post(cmd.ActionSlack.GetWebhookUrl(), "application/json", bytes.NewReader(requestBytes))
-	return err
-}
diff --git a/internal/hook/httputil.go b/internal/hook/httputil.go
index 0ee68850..fd4123b1 100644
--- a/internal/hook/httputil.go
+++ b/internal/hook/httputil.go
@@ -6,7 +6,7 @@ import (
 	"net/http"
 )
 
-func post(url string, contentType string, body io.Reader) (string, error) {
+func PostRequest(url string, contentType string, body io.Reader) (string, error) {
 	r, err := http.Post(url, contentType, body)
 	if err != nil {
 		return "", fmt.Errorf("send request %v: %w", url, err)
diff --git a/internal/hook/templateutil.go b/internal/hook/templateutil.go
new file mode 100644
index 00000000..9c0fe4ce
--- /dev/null
+++ b/internal/hook/templateutil.go
@@ -0,0 +1,29 @@
+package hook
+
+import (
+	"bytes"
+	"fmt"
+	"strings"
+	"text/template"
+)
+
+func RenderTemplate(text string, vars interface{}) (string, error) {
+	template, err := template.New("template").Parse(text)
+	if err != nil {
+		return "", fmt.Errorf("parse template: %w", err)
+	}
+
+	buf := &bytes.Buffer{}
+	if err := template.Execute(buf, vars); err != nil {
+		return "", fmt.Errorf("execute template: %w", err)
+	}
+
+	return buf.String(), nil
+}
+
+func RenderTemplateOrDefault(template string, defaultTmpl string, vars interface{}) (string, error) {
+	if strings.Trim(template, " ") == "" {
+		return RenderTemplate(defaultTmpl, vars)
+	}
+	return RenderTemplate(template, vars)
+}
diff --git a/internal/hook/types/command.go b/internal/hook/types/command.go
new file mode 100644
index 00000000..1ad0696a
--- /dev/null
+++ b/internal/hook/types/command.go
@@ -0,0 +1,61 @@
+package types
+
+import (
+	"context"
+	"fmt"
+	"os/exec"
+	"reflect"
+	"strings"
+
+	v1 "github.com/garethgeorge/backrest/gen/go/v1"
+	"github.com/garethgeorge/backrest/internal/hook"
+	"github.com/garethgeorge/backrest/internal/ioutil"
+	"github.com/garethgeorge/backrest/internal/orchestrator/logging"
+	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
+)
+
+type commandHandler struct{}
+
+func (commandHandler) Execute(ctx context.Context, h *v1.Hook, vars interface{}, runner tasks.TaskRunner) error {
+	command, err := hook.RenderTemplate(h.GetActionCommand().GetCommand(), vars)
+	if err != nil {
+		return fmt.Errorf("template rendering: %w", err)
+	}
+
+	writer := logging.WriterFromContext(ctx)
+
+	// Parse out the shell to use if a #! prefix is present
+	shell := "sh"
+	if len(command) > 2 && command[0:2] == "#!" {
+		nextLine := strings.Index(command, "\n")
+		if nextLine == -1 {
+			nextLine = len(command)
+		}
+		shell = strings.Trim(command[2:nextLine], " ")
+		command = command[nextLine+1:]
+	}
+
+	scriptWriter := &ioutil.LinePrefixer{W: writer, Prefix: []byte("[script] ")}
+	defer scriptWriter.Close()
+	outputWriter := &ioutil.LinePrefixer{W: writer, Prefix: []byte("[output] ")}
+	defer outputWriter.Close()
+	fmt.Fprintf(scriptWriter, "------- script -------\n#! %v\n%v\n", shell, command)
+
+	// Run the command in the specified shell
+	execCmd := exec.Command(shell)
+	execCmd.Stdin = strings.NewReader(command)
+
+	stdout := &ioutil.SynchronizedWriter{W: outputWriter}
+	execCmd.Stderr = stdout
+	execCmd.Stdout = stdout
+
+	return execCmd.Run()
+}
+
+func (commandHandler) ActionType() reflect.Type {
+	return reflect.TypeOf(&v1.Hook_ActionCommand{})
+}
+
+func init() {
+	hook.DefaultRegistry().RegisterHandler(&commandHandler{})
+}
diff --git a/internal/hook/types/discord.go b/internal/hook/types/discord.go
new file mode 100644
index 00000000..7a6b13cf
--- /dev/null
+++ b/internal/hook/types/discord.go
@@ -0,0 +1,46 @@
+package types
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"fmt"
+	"reflect"
+
+	v1 "github.com/garethgeorge/backrest/gen/go/v1"
+	"github.com/garethgeorge/backrest/internal/hook"
+	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
+)
+
+type discordHandler struct{}
+
+func (discordHandler) Execute(ctx context.Context, h *v1.Hook, vars interface{}, runner tasks.TaskRunner) error {
+	payload, err := hook.RenderTemplateOrDefault(h.GetActionDiscord().GetTemplate(), hook.DefaultTemplate, vars)
+	if err != nil {
+		return fmt.Errorf("template rendering: %w", err)
+	}
+
+	writer := runner.RawLogWriter(ctx)
+	fmt.Fprintf(writer, "Sending discord message to %s\n", h.GetActionDiscord().GetWebhookUrl())
+	fmt.Fprintf(writer, "---- payload ----\n%s\n", payload)
+
+	type Message struct {
+		Content string `json:"content"`
+	}
+
+	request := Message{
+		Content: payload, // leading newline looks better in discord.
+	}
+
+	requestBytes, _ := json.Marshal(request)
+	_, err = hook.PostRequest(h.GetActionDiscord().GetWebhookUrl(), "application/json", bytes.NewReader(requestBytes))
+	return err
+}
+
+func (discordHandler) ActionType() reflect.Type {
+	return reflect.TypeOf(&v1.Hook_ActionDiscord{})
+}
+
+func init() {
+	hook.DefaultRegistry().RegisterHandler(&discordHandler{})
+}
diff --git a/internal/hook/hookgotify.go b/internal/hook/types/gotify.go
similarity index 52%
rename from internal/hook/hookgotify.go
rename to internal/hook/types/gotify.go
index e7784403..b687ca67 100644
--- a/internal/hook/hookgotify.go
+++ b/internal/hook/types/gotify.go
@@ -1,27 +1,36 @@
-package hook
+package types
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
-	"io"
 	"net/url"
+	"reflect"
 	"strings"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
+	"github.com/garethgeorge/backrest/internal/hook"
+	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
 )
 
-func (h *Hook) doGotify(cmd *v1.Hook_ActionGotify, vars interface{}, output io.Writer) error {
-	payload, err := h.renderTemplateOrDefault(cmd.ActionGotify.GetTemplate(), defaultTemplate, vars)
+type gotifyHandler struct{}
+
+func (gotifyHandler) Execute(ctx context.Context, h *v1.Hook, vars interface{}, runner tasks.TaskRunner) error {
+	g := h.GetActionGotify()
+
+	payload, err := hook.RenderTemplateOrDefault(g.GetTemplate(), hook.DefaultTemplate, vars)
 	if err != nil {
 		return fmt.Errorf("template rendering: %w", err)
 	}
 
-	title, err := h.renderTemplateOrDefault(cmd.ActionGotify.GetTitleTemplate(), "Backrest Event", vars)
+	title, err := hook.RenderTemplateOrDefault(g.GetTitleTemplate(), "Backrest Event", vars)
 	if err != nil {
 		return fmt.Errorf("title template rendering: %w", err)
 	}
 
+	output := runner.RawLogWriter(ctx)
+
 	message := struct {
 		Message  string `json:"message"`
 		Title    string `json:"title"`
@@ -37,18 +46,18 @@ func (h *Hook) doGotify(cmd *v1.Hook_ActionGotify, vars interface{}, output io.W
 		return fmt.Errorf("json marshal: %w", err)
 	}
 
-	baseUrl := strings.Trim(cmd.ActionGotify.GetBaseUrl(), "/")
+	baseUrl := strings.Trim(g.GetBaseUrl(), "/")
 
 	postUrl := fmt.Sprintf(
 		"%s/message?token=%s",
 		baseUrl,
-		url.QueryEscape(cmd.ActionGotify.GetToken()))
+		url.QueryEscape(g.GetToken()))
 
 	fmt.Fprintf(output, "Sending gotify message to %s\n", postUrl)
 	fmt.Fprintf(output, "---- payload ----\n")
 	output.Write(b)
 
-	body, err := post(postUrl, "application/json", bytes.NewReader(b))
+	body, err := hook.PostRequest(postUrl, "application/json", bytes.NewReader(b))
 
 	if err != nil {
 		return fmt.Errorf("send gotify message: %w", err)
@@ -60,3 +69,7 @@ func (h *Hook) doGotify(cmd *v1.Hook_ActionGotify, vars interface{}, output io.W
 
 	return nil
 }
+
+func (gotifyHandler) ActionType() reflect.Type {
+	return reflect.TypeOf(&v1.Hook_ActionGotify{})
+}
diff --git a/internal/hook/types/shoutrrr.go b/internal/hook/types/shoutrrr.go
new file mode 100644
index 00000000..39cbd799
--- /dev/null
+++ b/internal/hook/types/shoutrrr.go
@@ -0,0 +1,39 @@
+package types
+
+import (
+	"context"
+	"fmt"
+	"reflect"
+
+	"github.com/containrrr/shoutrrr"
+	v1 "github.com/garethgeorge/backrest/gen/go/v1"
+	"github.com/garethgeorge/backrest/internal/hook"
+	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
+)
+
+type shoutrrrHandler struct{}
+
+func (shoutrrrHandler) Execute(ctx context.Context, h *v1.Hook, vars interface{}, runner tasks.TaskRunner) error {
+	payload, err := hook.RenderTemplateOrDefault(h.GetActionShoutrrr().GetTemplate(), hook.DefaultTemplate, vars)
+	if err != nil {
+		return fmt.Errorf("template rendering: %w", err)
+	}
+
+	writer := runner.RawLogWriter(ctx)
+	fmt.Fprintf(writer, "Sending shoutrrr message to %s\n", h.GetActionShoutrrr().GetShoutrrrUrl())
+	fmt.Fprintf(writer, "---- payload ----\n%s\n", payload)
+
+	if err := shoutrrr.Send(h.GetActionShoutrrr().GetShoutrrrUrl(), payload); err != nil {
+		return fmt.Errorf("sending shoutrrr message to %q: %w", h.GetActionShoutrrr().GetShoutrrrUrl(), err)
+	}
+
+	return nil
+}
+
+func (shoutrrrHandler) ActionType() reflect.Type {
+	return reflect.TypeOf(&v1.Hook_ActionShoutrrr{})
+}
+
+func init() {
+	hook.DefaultRegistry().RegisterHandler(&shoutrrrHandler{})
+}
diff --git a/internal/hook/types/slack.go b/internal/hook/types/slack.go
new file mode 100644
index 00000000..689e7e65
--- /dev/null
+++ b/internal/hook/types/slack.go
@@ -0,0 +1,47 @@
+package types
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"fmt"
+	"reflect"
+
+	v1 "github.com/garethgeorge/backrest/gen/go/v1"
+	"github.com/garethgeorge/backrest/internal/hook"
+	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
+)
+
+type slackHandler struct{}
+
+func (slackHandler) Execute(ctx context.Context, cmd *v1.Hook, vars interface{}, runner tasks.TaskRunner) error {
+	payload, err := hook.RenderTemplateOrDefault(cmd.GetActionSlack().GetTemplate(), hook.DefaultTemplate, vars)
+	if err != nil {
+		return fmt.Errorf("template rendering: %w", err)
+	}
+
+	writer := runner.RawLogWriter(ctx)
+	fmt.Fprintf(writer, "Sending slack message to %s\n", cmd.GetActionSlack().GetWebhookUrl())
+	fmt.Fprintf(writer, "---- payload ----\n%s\n", payload)
+
+	type Message struct {
+		Text string `json:"text"`
+	}
+
+	request := Message{
+		Text: "Backrest Notification\n" + payload, // leading newline looks better in discord.
+	}
+
+	requestBytes, _ := json.Marshal(request)
+
+	_, err = hook.PostRequest(cmd.GetActionSlack().GetWebhookUrl(), "application/json", bytes.NewReader(requestBytes))
+	return err
+}
+
+func (slackHandler) ActionType() reflect.Type {
+	return reflect.TypeOf(&v1.Hook_ActionSlack{})
+}
+
+func init() {
+	hook.DefaultRegistry().RegisterHandler(&slackHandler{})
+}
diff --git a/internal/orchestrator/taskrunnerimpl.go b/internal/orchestrator/taskrunnerimpl.go
index e4173f92..a74ce497 100644
--- a/internal/orchestrator/taskrunnerimpl.go
+++ b/internal/orchestrator/taskrunnerimpl.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"io"
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
@@ -136,3 +137,7 @@ func (t *taskRunnerImpl) Config() *v1.Config {
 func (t *taskRunnerImpl) Logger(ctx context.Context) *zap.Logger {
 	return logging.Logger(ctx).Named(t.t.Name())
 }
+
+func (t *taskRunnerImpl) RawLogWriter(ctx context.Context) io.Writer {
+	return logging.WriterFromContext(ctx)
+}
diff --git a/internal/orchestrator/tasks/task.go b/internal/orchestrator/tasks/task.go
index fa0d3e79..1b78b837 100644
--- a/internal/orchestrator/tasks/task.go
+++ b/internal/orchestrator/tasks/task.go
@@ -2,6 +2,7 @@ package tasks
 
 import (
 	"context"
+	"io"
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
@@ -49,6 +50,8 @@ type TaskRunner interface {
 	Config() *v1.Config
 	// Logger returns the logger.
 	Logger(ctx context.Context) *zap.Logger
+	// RawLogWriter returns a writer for raw logs.
+	RawLogWriter(ctx context.Context) io.Writer
 }
 
 // ScheduledTask is a task that is scheduled to run at a specific time.

From 96b87dc188130a84bc02613cf2753fd72de1bc35 Mon Sep 17 00:00:00 2001
From: garethgeorge <garethgeorge97@gmail.com>
Date: Mon, 1 Jul 2024 20:55:47 -0700
Subject: [PATCH 05/15] temp commit

---
 internal/hook/hook.go         | 14 +++++++-------
 internal/hook/types/gotify.go |  4 ++++
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/internal/hook/hook.go b/internal/hook/hook.go
index 228490ff..7d6c687d 100644
--- a/internal/hook/hook.go
+++ b/internal/hook/hook.go
@@ -51,8 +51,7 @@ func (e *HookExecutor) ExecuteHooks(flowID int64, repo *v1.Repo, plan *v1.Plan,
 	}
 
 	for idx, hook := range repo.GetHooks() {
-		h := (*Hook)(hook)
-		event := firstMatchingCondition(h, events)
+		event := firstMatchingCondition(hook, events)
 		if event == v1.Hook_CONDITION_UNKNOWN {
 			continue
 		}
@@ -68,7 +67,7 @@ func (e *HookExecutor) ExecuteHooks(flowID int64, repo *v1.Repo, plan *v1.Plan,
 			},
 		}
 		zap.L().Info("running hook", zap.String("plan", repo.Id), zap.Int64("opId", operation.Id), zap.String("hook", name))
-		if err := e.executeHook(operation, h, event, vars); err != nil {
+		if err := executeHook(operation, hook, event, vars); err != nil {
 			zap.S().Errorf("error on repo hook %v on condition %v: %v", idx, event.String(), err)
 			if isHaltingError(err) {
 				return fmt.Errorf("repo hook %v on condition %v: %w", idx, event.String(), err)
@@ -77,8 +76,7 @@ func (e *HookExecutor) ExecuteHooks(flowID int64, repo *v1.Repo, plan *v1.Plan,
 	}
 
 	for idx, hook := range plan.GetHooks() {
-		h := (*Hook)(hook)
-		event := firstMatchingCondition(h, events)
+		event := firstMatchingCondition(hook, events)
 		if event == v1.Hook_CONDITION_UNKNOWN {
 			continue
 		}
@@ -105,7 +103,7 @@ func (e *HookExecutor) ExecuteHooks(flowID int64, repo *v1.Repo, plan *v1.Plan,
 	return nil
 }
 
-func firstMatchingCondition(hook *Hook, events []v1.Hook_Condition) v1.Hook_Condition {
+func firstMatchingCondition(hook *v1.Hook, events []v1.Hook_Condition) v1.Hook_Condition {
 	for _, event := range events {
 		if slices.Contains(hook.Conditions, event) {
 			return event
@@ -114,12 +112,14 @@ func firstMatchingCondition(hook *Hook, events []v1.Hook_Condition) v1.Hook_Cond
 	return v1.Hook_CONDITION_UNKNOWN
 }
 
-func (e *HookExecutor) executeHook(op *v1.Operation, hook *Hook, event v1.Hook_Condition, vars interface{}) error {
+func (e *HookExecutor) executeHook(op *v1.Operation, hook *v1.Hook, event v1.Hook_Condition, vars interface{}) error {
 	if err := e.oplog.Add(op); err != nil {
 		zap.S().Errorf("execute hook: add operation: %v", err)
 		return errors.New("couldn't create operation")
 	}
 
+	// TODO: implement a task runner here
+
 	output := &bytes.Buffer{}
 	fmt.Fprintf(output, "triggering condition: %v\n", event.String())
 
diff --git a/internal/hook/types/gotify.go b/internal/hook/types/gotify.go
index b687ca67..c2a9b0f7 100644
--- a/internal/hook/types/gotify.go
+++ b/internal/hook/types/gotify.go
@@ -73,3 +73,7 @@ func (gotifyHandler) Execute(ctx context.Context, h *v1.Hook, vars interface{},
 func (gotifyHandler) ActionType() reflect.Type {
 	return reflect.TypeOf(&v1.Hook_ActionGotify{})
 }
+
+func init() {
+	hook.DefaultRegistry().RegisterHandler(&gotifyHandler{})
+}

From aa044d2344d815e4eb64984aa7e3e5be5af49333 Mon Sep 17 00:00:00 2001
From: garethgeorge <garethgeorge97@gmail.com>
Date: Sat, 6 Jul 2024 11:48:06 -0700
Subject: [PATCH 06/15] new hook interface builds

---
 internal/hook/handler.go                      |   3 +-
 internal/hook/hook.go                         | 183 ++++++++----------
 internal/oplog/oplog.go                       |   2 +-
 internal/orchestrator/orchestrator.go         | 155 +++++++--------
 internal/orchestrator/taskrunnerimpl.go       |  20 +-
 internal/orchestrator/tasks/task.go           |   6 +-
 internal/orchestrator/tasks/taskbackup.go     |   8 +-
 internal/orchestrator/tasks/taskcheck.go      |   6 +-
 internal/orchestrator/tasks/taskforget.go     |   2 +-
 .../orchestrator/tasks/taskforgetsnapshot.go  |   2 +-
 .../orchestrator/tasks/taskindexsnapshots.go  |   2 +-
 internal/orchestrator/tasks/taskprune.go      |   6 +-
 internal/orchestrator/tasks/taskrestore.go    |   2 +-
 internal/orchestrator/tasks/taskrunhook.go    |   1 +
 internal/orchestrator/tasks/taskstats.go      |   2 +-
 internal/queue/genheap.go                     |  16 +-
 internal/queue/genheap_test.go                |   4 +-
 internal/queue/timepriorityqueue.go           |   4 +-
 internal/queue/timequeue.go                   |   4 +-
 19 files changed, 210 insertions(+), 218 deletions(-)
 create mode 100644 internal/orchestrator/tasks/taskrunhook.go

diff --git a/internal/hook/handler.go b/internal/hook/handler.go
index d3f2c525..110a41b3 100644
--- a/internal/hook/handler.go
+++ b/internal/hook/handler.go
@@ -3,6 +3,7 @@ package hook
 import (
 	"context"
 	"errors"
+	"fmt"
 	"reflect"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
@@ -32,7 +33,7 @@ func (r *HandlerRegistry) RegisterHandler(handler Handler) {
 func (r *HandlerRegistry) GetHandler(hook *v1.Hook) (Handler, error) {
 	handler, ok := r.actionHandlers[reflect.TypeOf(hook.Action)]
 	if !ok {
-		return nil, ErrHandlerNotFound
+		return nil, fmt.Errorf("hook type %T: %w", hook.Action, ErrHandlerNotFound)
 	}
 	return handler, nil
 }
diff --git a/internal/hook/hook.go b/internal/hook/hook.go
index 7d6c687d..d6f9431e 100644
--- a/internal/hook/hook.go
+++ b/internal/hook/hook.go
@@ -1,18 +1,15 @@
 package hook
 
 import (
-	"bytes"
+	"context"
 	"errors"
 	"fmt"
-	"io"
 	"reflect"
 	"slices"
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-	"github.com/garethgeorge/backrest/internal/oplog"
-	"github.com/garethgeorge/backrest/internal/rotatinglog"
-	"go.uber.org/zap"
+	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
 	"google.golang.org/protobuf/proto"
 )
 
@@ -20,34 +17,48 @@ var (
 	DefaultTemplate = `{{ .Summary }}`
 )
 
-type HookExecutor struct {
-	config   *v1.Config
-	oplog    *oplog.OpLog
-	logStore *rotatinglog.RotatingLog
+func ExecuteHookTasks(ctx context.Context, runner tasks.ScheduledTaskExecutor, hookTasks []tasks.Task) error {
+	for _, task := range hookTasks {
+		err := runner.RunTask(ctx, tasks.ScheduledTask{Task: task, RunAt: time.Now()})
+		if isHaltingError(err) {
+			return fmt.Errorf("hook %v: %w", task.Name(), err)
+		}
+	}
+	return nil
 }
 
-func NewHookExecutor(config *v1.Config, oplog *oplog.OpLog, bigOutputStore *rotatinglog.RotatingLog) *HookExecutor {
-	return &HookExecutor{
-		config:   config,
-		oplog:    oplog,
-		logStore: bigOutputStore,
+func TasksTriggeredByEvent(config *v1.Config, flowID int64, planID string, repoID string, events []v1.Hook_Condition, vars interface{}) ([]tasks.Task, error) {
+	var taskSet []tasks.Task
+
+	var plan *v1.Plan
+	var repo *v1.Repo
+	if repoID != "" {
+		if repoIdx := slices.IndexFunc(config.Repos, func(r *v1.Repo) bool {
+			return r.GetId() == repoID
+		}); repoIdx != -1 {
+			repo = config.Repos[repoIdx]
+		} else {
+			return nil, fmt.Errorf("repo %v not found", repoID)
+		}
 	}
-}
 
-// ExecuteHooks schedules tasks for the hooks subscribed to the given event. The vars map is used to substitute variables
-// Hooks are pulled both from the provided plan and from the repo config.
-func (e *HookExecutor) ExecuteHooks(flowID int64, repo *v1.Repo, plan *v1.Plan, events []v1.Hook_Condition, vars interface{}) error {
-	planId := plan.GetId()
-	if planId == "" {
-		planId = "_system_" // TODO: clean this up when refactoring hook execution
+	if planID != "" {
+		if planIdx := slices.IndexFunc(config.Plans, func(p *v1.Plan) bool {
+			return p.GetId() == planID
+		}); planIdx != -1 {
+			plan = config.Plans[planIdx]
+		} else {
+			return nil, fmt.Errorf("plan %v not found", planID)
+		}
+	} else {
+		planID = tasks.PlanForSystemTasks
 	}
 
-	operationBase := v1.Operation{
-		Status:     v1.OperationStatus_STATUS_INPROGRESS,
-		PlanId:     planId,
-		RepoId:     repo.GetId(),
-		InstanceId: e.config.Instance,
-		FlowId:     flowID,
+	baseOp := v1.Operation{
+		Status:          v1.OperationStatus_STATUS_PENDING,
+		InstanceId:      config.Instance,
+		FlowId:          flowID,
+		UnixTimeStartMs: curTimeMs(),
 	}
 
 	for idx, hook := range repo.GetHooks() {
@@ -57,22 +68,16 @@ func (e *HookExecutor) ExecuteHooks(flowID int64, repo *v1.Repo, plan *v1.Plan,
 		}
 
 		name := fmt.Sprintf("repo/%v/hook/%v", repo.Id, idx)
-		operation := proto.Clone(&operationBase).(*v1.Operation)
-		operation.DisplayMessage = "running " + name
-		operation.UnixTimeStartMs = curTimeMs()
-		operation.Op = &v1.Operation_OperationRunHook{
+		op := proto.Clone(&baseOp).(*v1.Operation)
+		op.DisplayMessage = "running " + name
+		op.Op = &v1.Operation_OperationRunHook{
 			OperationRunHook: &v1.OperationRunHook{
 				Name:      name,
 				Condition: event,
 			},
 		}
-		zap.L().Info("running hook", zap.String("plan", repo.Id), zap.Int64("opId", operation.Id), zap.String("hook", name))
-		if err := executeHook(operation, hook, event, vars); err != nil {
-			zap.S().Errorf("error on repo hook %v on condition %v: %v", idx, event.String(), err)
-			if isHaltingError(err) {
-				return fmt.Errorf("repo hook %v on condition %v: %w", idx, event.String(), err)
-			}
-		}
+
+		taskSet = append(taskSet, newOneoffRunHookTask(name, repoID, planID, flowID, time.Now(), hook, event, vars))
 	}
 
 	for idx, hook := range plan.GetHooks() {
@@ -82,25 +87,51 @@ func (e *HookExecutor) ExecuteHooks(flowID int64, repo *v1.Repo, plan *v1.Plan,
 		}
 
 		name := fmt.Sprintf("plan/%v/hook/%v", plan.Id, idx)
-		operation := proto.Clone(&operationBase).(*v1.Operation)
-		operation.DisplayMessage = "running " + name
-		operation.UnixTimeStartMs = curTimeMs()
-		operation.Op = &v1.Operation_OperationRunHook{
+		op := proto.Clone(&baseOp).(*v1.Operation)
+		op.DisplayMessage = fmt.Sprintf("running %v triggered by %v", name, event.String())
+		op.Op = &v1.Operation_OperationRunHook{
 			OperationRunHook: &v1.OperationRunHook{
 				Name:      name,
 				Condition: event,
 			},
 		}
 
-		zap.L().Info("running hook", zap.String("plan", plan.Id), zap.Int64("opId", operation.Id), zap.String("hook", name))
-		if err := e.executeHook(operation, h, event, vars); err != nil {
-			zap.S().Errorf("error on plan hook %v on condition %v: %v", idx, event.String(), err)
-			if isHaltingError(err) {
-				return fmt.Errorf("plan hook %v on condition %v: %w", idx, event.String(), err)
+		taskSet = append(taskSet, newOneoffRunHookTask(name, repoID, planID, flowID, time.Now(), hook, event, vars))
+	}
+
+	return taskSet, nil
+}
+
+func newOneoffRunHookTask(title, repoID, planID string, flowID int64, at time.Time, hook *v1.Hook, event v1.Hook_Condition, vars interface{}) tasks.Task {
+	return &tasks.GenericOneoffTask{
+		BaseTask: tasks.BaseTask{
+			TaskName:   fmt.Sprintf("run hook %v", title),
+			TaskRepoID: repoID,
+			TaskPlanID: planID,
+		},
+		OneoffTask: tasks.OneoffTask{
+			FlowID: flowID,
+			RunAt:  at,
+			ProtoOp: &v1.Operation{
+				Op: &v1.Operation_OperationRunHook{},
+			},
+		},
+		Do: func(ctx context.Context, st tasks.ScheduledTask, taskRunner tasks.TaskRunner) error {
+			h, err := DefaultRegistry().GetHandler(hook)
+			if err != nil {
+				return err
 			}
-		}
+			if eventField := reflect.ValueOf(vars).FieldByName("Event"); eventField.IsValid() {
+				eventField.Set(reflect.ValueOf(event))
+			}
+
+			if err := h.Execute(ctx, hook, vars, taskRunner); err != nil {
+				err = applyHookErrorPolicy(hook.OnError, err)
+				return err
+			}
+			return nil
+		},
 	}
-	return nil
 }
 
 func firstMatchingCondition(hook *v1.Hook, events []v1.Hook_Condition) v1.Hook_Condition {
@@ -112,66 +143,12 @@ func firstMatchingCondition(hook *v1.Hook, events []v1.Hook_Condition) v1.Hook_C
 	return v1.Hook_CONDITION_UNKNOWN
 }
 
-func (e *HookExecutor) executeHook(op *v1.Operation, hook *v1.Hook, event v1.Hook_Condition, vars interface{}) error {
-	if err := e.oplog.Add(op); err != nil {
-		zap.S().Errorf("execute hook: add operation: %v", err)
-		return errors.New("couldn't create operation")
-	}
-
-	// TODO: implement a task runner here
-
-	output := &bytes.Buffer{}
-	fmt.Fprintf(output, "triggering condition: %v\n", event.String())
-
-	var retErr error
-	if err := hook.Do(event, vars, io.MultiWriter(output)); err != nil {
-		output.Write([]byte(fmt.Sprintf("Error: %v", err)))
-		err = applyHookErrorPolicy(hook.OnError, err)
-		var cancelErr *HookErrorRequestCancel
-		if errors.As(err, &cancelErr) {
-			// if it was a cancel then it successfully indicated it's intent to the caller
-			// no error should be displayed in the UI.
-			op.Status = v1.OperationStatus_STATUS_SUCCESS
-		} else {
-			op.Status = v1.OperationStatus_STATUS_ERROR
-		}
-		retErr = err
-	} else {
-		op.Status = v1.OperationStatus_STATUS_SUCCESS
-	}
-
-	outputRef, err := e.logStore.Write(output.Bytes())
-	if err != nil {
-		retErr = errors.Join(retErr, fmt.Errorf("write logstore: %w", err))
-	}
-	op.Logref = outputRef
-
-	op.UnixTimeEndMs = curTimeMs()
-	if err := e.oplog.Update(op); err != nil {
-		retErr = errors.Join(retErr, fmt.Errorf("update oplog: %w", err))
-	}
-	return retErr
-}
-
 func curTimeMs() int64 {
 	return time.Now().UnixNano() / 1000000
 }
 
 type Hook v1.Hook
 
-func (h *Hook) Do(event v1.Hook_Condition, vars interface{}, output io.Writer) error {
-	if !slices.Contains(h.Conditions, event) {
-		return nil
-	}
-
-	// if vars has a .Event key set it to the event
-	// this is a bit of a hack to allow the event to be used in the template
-	if eventField := reflect.ValueOf(vars).FieldByName("Event"); eventField.IsValid() {
-		eventField.Set(reflect.ValueOf(event))
-	}
-
-}
-
 func applyHookErrorPolicy(onError v1.Hook_OnError, err error) error {
 	if err == nil || errors.As(err, &HookErrorFatal{}) || errors.As(err, &HookErrorRequestCancel{}) {
 		return err
diff --git a/internal/oplog/oplog.go b/internal/oplog/oplog.go
index 36f94b2d..ead79e6d 100644
--- a/internal/oplog/oplog.go
+++ b/internal/oplog/oplog.go
@@ -222,7 +222,7 @@ func (o *OpLog) notifyHelper(old *v1.Operation, new *v1.Operation) {
 	o.subscribersMu.RUnlock()
 
 	for _, sub := range subscribers {
-		(*sub)(old, new)
+		go (*sub)(old, new)
 	}
 }
 
diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go
index c5082001..353a2c53 100644
--- a/internal/orchestrator/orchestrator.go
+++ b/internal/orchestrator/orchestrator.go
@@ -6,11 +6,11 @@ import (
 	"errors"
 	"fmt"
 	"slices"
+	"strings"
 	"sync"
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-	"github.com/garethgeorge/backrest/internal/hook"
 	"github.com/garethgeorge/backrest/internal/ioutil"
 	"github.com/garethgeorge/backrest/internal/oplog"
 	"github.com/garethgeorge/backrest/internal/orchestrator/logging"
@@ -28,13 +28,13 @@ var ErrPlanNotFound = errors.New("plan not found")
 
 // Orchestrator is responsible for managing repos and backups.
 type Orchestrator struct {
-	mu           sync.Mutex
-	config       *v1.Config
-	OpLog        *oplog.OpLog
-	repoPool     *resticRepoPool
-	taskQueue    *queue.TimePriorityQueue[stContainer]
-	hookExecutor *hook.HookExecutor
-	logStore     *rotatinglog.RotatingLog
+	mu              sync.Mutex
+	config          *v1.Config
+	OpLog           *oplog.OpLog
+	repoPool        *resticRepoPool
+	taskQueue       *queue.TimePriorityQueue[stContainer]
+	readyTaskQueues map[string]chan tasks.Task
+	logStore        *rotatinglog.RotatingLog
 
 	// cancelNotify is a list of channels that are notified when a task should be cancelled.
 	cancelNotify []chan int64
@@ -43,6 +43,8 @@ type Orchestrator struct {
 	now func() time.Time
 }
 
+var _ tasks.ScheduledTaskExecutor = &Orchestrator{}
+
 type stContainer struct {
 	tasks.ScheduledTask
 	configModno int32
@@ -285,12 +287,7 @@ func (o *Orchestrator) Run(ctx context.Context) {
 			continue
 		}
 
-		zap.L().Info("running task", zap.String("task", t.Task.Name()))
-
-		logs := bytes.NewBuffer(nil)
 		taskCtx, cancelTaskCtx := context.WithCancel(ctx)
-		taskCtx = logging.ContextWithWriter(taskCtx, &ioutil.SynchronizedWriter{W: logs})
-
 		go func() {
 			for {
 				select {
@@ -303,62 +300,10 @@ func (o *Orchestrator) Run(ctx context.Context) {
 				}
 			}
 		}()
-
 		start := time.Now()
-		runner := newTaskRunnerImpl(o, t.Task, t.Op)
-
-		op := t.Op
-		if op != nil {
-			op.UnixTimeStartMs = time.Now().UnixMilli()
-			if op.Status == v1.OperationStatus_STATUS_PENDING || op.Status == v1.OperationStatus_STATUS_UNKNOWN {
-				op.Status = v1.OperationStatus_STATUS_INPROGRESS
-			}
-			if op.Id != 0 {
-				if err := o.OpLog.Update(op); err != nil {
-					zap.S().Errorf("failed to add operation to oplog: %w", err)
-				}
-			} else {
-				if err := o.OpLog.Add(op); err != nil {
-					zap.S().Errorf("failed to add operation to oplog: %w", err)
-				}
-			}
-		}
-
-		err := t.Task.Run(taskCtx, t.ScheduledTask, runner)
-		if err != nil {
-			fmt.Fprintf(logs, "\ntask %q returned error: %v\n", t.Task.Name(), err)
-		} else {
-			fmt.Fprintf(logs, "\ntask %q completed successfully\n", t.Task.Name())
-		}
-
-		if op != nil {
-			// write logs to log storage for this task.
-			if logs.Len() > 0 {
-				ref, err := o.logStore.Write(logs.Bytes())
-				if err != nil {
-					zap.S().Errorf("failed to write logs for task %q to log store: %v", t.Task.Name(), err)
-				} else {
-					op.Logref = ref
-				}
-			}
-			if err != nil {
-				if taskCtx.Err() != nil || errors.Is(err, tasks.ErrTaskCancelled) {
-					// task was cancelled
-					op.Status = v1.OperationStatus_STATUS_USER_CANCELLED
-				} else if err != nil {
-					op.Status = v1.OperationStatus_STATUS_ERROR
-				}
-				op.DisplayMessage = err.Error()
-			}
-			op.UnixTimeEndMs = time.Now().UnixMilli()
-			if op.Status == v1.OperationStatus_STATUS_INPROGRESS {
-				op.Status = v1.OperationStatus_STATUS_SUCCESS
-			}
-			if e := o.OpLog.Update(op); e != nil {
-				zap.S().Errorf("failed to update operation in oplog: %v", e)
-			}
-		}
 
+		zap.L().Info("running task", zap.String("task", t.Task.Name()))
+		err := o.RunTask(taskCtx, t.ScheduledTask)
 		if err != nil {
 			zap.L().Error("task failed", zap.String("task", t.Task.Name()), zap.Error(err), zap.Duration("duration", time.Since(start)))
 		} else {
@@ -376,12 +321,75 @@ func (o *Orchestrator) Run(ctx context.Context) {
 		}
 		cancelTaskCtx()
 
-		go func() {
-			for _, cb := range t.callbacks {
-				cb(err)
+		for _, cb := range t.callbacks {
+			go cb(err)
+		}
+	}
+}
+
+func (o *Orchestrator) RunTask(ctx context.Context, st tasks.ScheduledTask) error {
+	logs := bytes.NewBuffer(nil)
+	ctx = logging.ContextWithWriter(ctx, &ioutil.SynchronizedWriter{W: logs})
+
+	runner := newTaskRunnerImpl(o, st.Task, st.Op)
+
+	op := st.Op
+	if op != nil {
+		op.UnixTimeStartMs = time.Now().UnixMilli()
+		if op.Status == v1.OperationStatus_STATUS_PENDING || op.Status == v1.OperationStatus_STATUS_UNKNOWN {
+			op.Status = v1.OperationStatus_STATUS_INPROGRESS
+		}
+		if op.Id != 0 {
+			if err := o.OpLog.Update(op); err != nil {
+				zap.S().Errorf("failed to add operation to oplog: %w", err)
 			}
-		}()
+		} else {
+			if err := o.OpLog.Add(op); err != nil {
+				zap.S().Errorf("failed to add operation to oplog: %w", err)
+			}
+		}
+	}
+
+	err := st.Task.Run(ctx, st, runner)
+	if err != nil {
+		fmt.Fprintf(logs, "\ntask %q returned error: %v\n", st.Task.Name(), err)
+	} else {
+		fmt.Fprintf(logs, "\ntask %q completed successfully\n", st.Task.Name())
+	}
+
+	if op != nil {
+		// write logs to log storage for this task.
+		if logs.Len() > 0 {
+			ref, err := o.logStore.Write(logs.Bytes())
+			if err != nil {
+				zap.S().Errorf("failed to write logs for task %q to log store: %v", st.Task.Name(), err)
+			} else {
+				op.Logref = ref
+			}
+		}
+		if err != nil {
+			if ctx.Err() != nil || errors.Is(err, tasks.ErrTaskCancelled) {
+				// task was cancelled
+				op.Status = v1.OperationStatus_STATUS_USER_CANCELLED
+			} else if err != nil {
+				op.Status = v1.OperationStatus_STATUS_ERROR
+			}
+
+			// append the error to the display message
+			if op.DisplayMessage != "" && !strings.HasSuffix(op.DisplayMessage, "\n") {
+				op.DisplayMessage += "\n"
+			}
+			op.DisplayMessage += err.Error()
+		}
+		op.UnixTimeEndMs = time.Now().UnixMilli()
+		if op.Status == v1.OperationStatus_STATUS_INPROGRESS {
+			op.Status = v1.OperationStatus_STATUS_SUCCESS
+		}
+		if e := o.OpLog.Update(op); e != nil {
+			zap.S().Errorf("failed to update operation in oplog: %v", e)
+		}
 	}
+	return err
 }
 
 // ScheduleTask schedules a task to run at the next available time.
@@ -473,8 +481,3 @@ func (rp *resticRepoPool) GetRepo(repoId string) (*repo.RepoOrchestrator, error)
 	rp.repos[repoId] = r
 	return r, nil
 }
-
-type taskExecutionInfo struct {
-	operationId int64
-	cancel      func()
-}
diff --git a/internal/orchestrator/taskrunnerimpl.go b/internal/orchestrator/taskrunnerimpl.go
index a74ce497..787f7e8b 100644
--- a/internal/orchestrator/taskrunnerimpl.go
+++ b/internal/orchestrator/taskrunnerimpl.go
@@ -72,7 +72,7 @@ func (t *taskRunnerImpl) OpLog() *oplog.OpLog {
 	return t.orchestrator.OpLog
 }
 
-func (t *taskRunnerImpl) ExecuteHooks(events []v1.Hook_Condition, vars tasks.HookVars) error {
+func (t *taskRunnerImpl) ExecuteHooks(ctx context.Context, events []v1.Hook_Condition, vars tasks.HookVars) error {
 	vars.Task = t.t.Name()
 	if t.op != nil {
 		vars.Duration = time.Since(time.UnixMilli(t.op.UnixTimeStartMs))
@@ -101,13 +101,19 @@ func (t *taskRunnerImpl) ExecuteHooks(events []v1.Hook_Condition, vars tasks.Hoo
 		flowID = t.op.FlowId
 	}
 
-	executor := hook.NewHookExecutor(t.Config(), t.orchestrator.OpLog, t.orchestrator.logStore)
-	err := executor.ExecuteHooks(flowID, repo, plan, events, vars)
-	var cancelErr *hook.HookErrorRequestCancel
-	if errors.As(err, &cancelErr) {
-		return fmt.Errorf("%w: %w", tasks.ErrTaskCancelled, err)
+	hookTasks, err := hook.TasksTriggeredByEvent(t.Config(), flowID, planID, repoID, events, vars)
+	if err != nil {
+		return err
 	}
-	return err
+
+	if err := hook.ExecuteHookTasks(ctx, t.orchestrator, hookTasks); err != nil {
+		var cancelErr hook.HookErrorRequestCancel
+		if errors.As(err, &cancelErr) {
+			return fmt.Errorf("%w: %w", tasks.ErrTaskCancelled, err)
+		}
+		return err
+	}
+	return nil
 }
 
 func (t *taskRunnerImpl) GetRepo(repoID string) (*v1.Repo, error) {
diff --git a/internal/orchestrator/tasks/task.go b/internal/orchestrator/tasks/task.go
index 1b78b837..5ce7101d 100644
--- a/internal/orchestrator/tasks/task.go
+++ b/internal/orchestrator/tasks/task.go
@@ -35,7 +35,7 @@ type TaskRunner interface {
 	// UpdateOperation updates the operation in storage. It must be called after CreateOperation.
 	UpdateOperation(*v1.Operation) error
 	// ExecuteHooks
-	ExecuteHooks(events []v1.Hook_Condition, vars HookVars) error
+	ExecuteHooks(ctx context.Context, events []v1.Hook_Condition, vars HookVars) error
 	// OpLog returns the oplog for the operations.
 	OpLog() *oplog.OpLog
 	// GetRepo returns the repo with the given ID.
@@ -54,6 +54,10 @@ type TaskRunner interface {
 	RawLogWriter(ctx context.Context) io.Writer
 }
 
+type ScheduledTaskExecutor interface {
+	RunTask(ctx context.Context, st ScheduledTask) error
+}
+
 // ScheduledTask is a task that is scheduled to run at a specific time.
 type ScheduledTask struct {
 	Task  Task          // the task to run
diff --git a/internal/orchestrator/tasks/taskbackup.go b/internal/orchestrator/tasks/taskbackup.go
index 0429bb03..5201de0a 100644
--- a/internal/orchestrator/tasks/taskbackup.go
+++ b/internal/orchestrator/tasks/taskbackup.go
@@ -105,7 +105,7 @@ func (t *BackupTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunne
 		return err
 	}
 
-	if err := runner.ExecuteHooks([]v1.Hook_Condition{
+	if err := runner.ExecuteHooks(ctx, []v1.Hook_Condition{
 		v1.Hook_CONDITION_SNAPSHOT_START,
 	}, HookVars{}); err != nil {
 		return fmt.Errorf("snapshot start hook: %w", err)
@@ -171,7 +171,7 @@ func (t *BackupTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunne
 	if err != nil {
 		vars.Error = err.Error()
 		if !errors.Is(err, restic.ErrPartialBackup) {
-			runner.ExecuteHooks([]v1.Hook_Condition{
+			runner.ExecuteHooks(ctx, []v1.Hook_Condition{
 				v1.Hook_CONDITION_SNAPSHOT_ERROR,
 				v1.Hook_CONDITION_ANY_ERROR,
 				v1.Hook_CONDITION_SNAPSHOT_END,
@@ -183,12 +183,12 @@ func (t *BackupTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunne
 		op.Status = v1.OperationStatus_STATUS_WARNING
 		op.DisplayMessage = "Partial backup, some files may not have been read completely."
 
-		runner.ExecuteHooks([]v1.Hook_Condition{
+		runner.ExecuteHooks(ctx, []v1.Hook_Condition{
 			v1.Hook_CONDITION_SNAPSHOT_WARNING,
 			v1.Hook_CONDITION_SNAPSHOT_END,
 		}, vars)
 	} else {
-		runner.ExecuteHooks([]v1.Hook_Condition{
+		runner.ExecuteHooks(ctx, []v1.Hook_Condition{
 			v1.Hook_CONDITION_SNAPSHOT_SUCCESS,
 			v1.Hook_CONDITION_SNAPSHOT_END,
 		}, vars)
diff --git a/internal/orchestrator/tasks/taskcheck.go b/internal/orchestrator/tasks/taskcheck.go
index cd300635..e17a38ba 100644
--- a/internal/orchestrator/tasks/taskcheck.go
+++ b/internal/orchestrator/tasks/taskcheck.go
@@ -100,7 +100,7 @@ func (t *CheckTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunner
 		return fmt.Errorf("couldn't get repo %q: %w", t.RepoID(), err)
 	}
 
-	if err := runner.ExecuteHooks([]v1.Hook_Condition{
+	if err := runner.ExecuteHooks(ctx, []v1.Hook_Condition{
 		v1.Hook_CONDITION_CHECK_START,
 	}, HookVars{}); err != nil {
 		return fmt.Errorf("check start hook: %w", err)
@@ -148,7 +148,7 @@ func (t *CheckTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunner
 	if err := repo.Check(ctx, bufWriter); err != nil {
 		cancel()
 
-		runner.ExecuteHooks([]v1.Hook_Condition{
+		runner.ExecuteHooks(ctx, []v1.Hook_Condition{
 			v1.Hook_CONDITION_CHECK_ERROR,
 			v1.Hook_CONDITION_ANY_ERROR,
 		}, HookVars{
@@ -167,7 +167,7 @@ func (t *CheckTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunner
 		zap.L().Error("schedule stats task", zap.Error(err))
 	}
 
-	if err := runner.ExecuteHooks([]v1.Hook_Condition{
+	if err := runner.ExecuteHooks(ctx, []v1.Hook_Condition{
 		v1.Hook_CONDITION_CHECK_SUCCESS,
 	}, HookVars{}); err != nil {
 		return fmt.Errorf("execute prune success hooks: %w", err)
diff --git a/internal/orchestrator/tasks/taskforget.go b/internal/orchestrator/tasks/taskforget.go
index 0e1b8015..834ba30c 100644
--- a/internal/orchestrator/tasks/taskforget.go
+++ b/internal/orchestrator/tasks/taskforget.go
@@ -35,7 +35,7 @@ func NewOneoffForgetTask(repoID, planID string, flowID int64, at time.Time) Task
 			}
 
 			if err := forgetHelper(ctx, st, taskRunner); err != nil {
-				taskRunner.ExecuteHooks([]v1.Hook_Condition{
+				taskRunner.ExecuteHooks(ctx, []v1.Hook_Condition{
 					v1.Hook_CONDITION_ANY_ERROR,
 				}, HookVars{
 					Error: err.Error(),
diff --git a/internal/orchestrator/tasks/taskforgetsnapshot.go b/internal/orchestrator/tasks/taskforgetsnapshot.go
index 5e81d0b6..3ef9c781 100644
--- a/internal/orchestrator/tasks/taskforgetsnapshot.go
+++ b/internal/orchestrator/tasks/taskforgetsnapshot.go
@@ -30,7 +30,7 @@ func NewOneoffForgetSnapshotTask(repoID, planID string, flowID int64, at time.Ti
 			}
 
 			if err := forgetSnapshotHelper(ctx, st, taskRunner, snapshotID); err != nil {
-				taskRunner.ExecuteHooks([]v1.Hook_Condition{
+				taskRunner.ExecuteHooks(ctx, []v1.Hook_Condition{
 					v1.Hook_CONDITION_ANY_ERROR,
 				}, HookVars{
 					Error: err.Error(),
diff --git a/internal/orchestrator/tasks/taskindexsnapshots.go b/internal/orchestrator/tasks/taskindexsnapshots.go
index 2a07f209..75eb0d62 100644
--- a/internal/orchestrator/tasks/taskindexsnapshots.go
+++ b/internal/orchestrator/tasks/taskindexsnapshots.go
@@ -28,7 +28,7 @@ func NewOneoffIndexSnapshotsTask(repoID string, at time.Time) Task {
 		},
 		Do: func(ctx context.Context, st ScheduledTask, taskRunner TaskRunner) error {
 			if err := indexSnapshotsHelper(ctx, st, taskRunner); err != nil {
-				taskRunner.ExecuteHooks([]v1.Hook_Condition{
+				taskRunner.ExecuteHooks(ctx, []v1.Hook_Condition{
 					v1.Hook_CONDITION_ANY_ERROR,
 				}, HookVars{
 					Task:  st.Task.Name(),
diff --git a/internal/orchestrator/tasks/taskprune.go b/internal/orchestrator/tasks/taskprune.go
index 3f16c729..c2c32f92 100644
--- a/internal/orchestrator/tasks/taskprune.go
+++ b/internal/orchestrator/tasks/taskprune.go
@@ -100,7 +100,7 @@ func (t *PruneTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunner
 		return fmt.Errorf("couldn't get repo %q: %w", t.RepoID(), err)
 	}
 
-	if err := runner.ExecuteHooks([]v1.Hook_Condition{
+	if err := runner.ExecuteHooks(ctx, []v1.Hook_Condition{
 		v1.Hook_CONDITION_PRUNE_START,
 	}, HookVars{}); err != nil {
 		return fmt.Errorf("prune start hook: %w", err)
@@ -148,7 +148,7 @@ func (t *PruneTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunner
 	if err := repo.Prune(ctx, bufWriter); err != nil {
 		cancel()
 
-		runner.ExecuteHooks([]v1.Hook_Condition{
+		runner.ExecuteHooks(ctx, []v1.Hook_Condition{
 			v1.Hook_CONDITION_ANY_ERROR,
 		}, HookVars{
 			Error: err.Error(),
@@ -166,7 +166,7 @@ func (t *PruneTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunner
 		zap.L().Error("schedule stats task", zap.Error(err))
 	}
 
-	if err := runner.ExecuteHooks([]v1.Hook_Condition{
+	if err := runner.ExecuteHooks(ctx, []v1.Hook_Condition{
 		v1.Hook_CONDITION_PRUNE_SUCCESS,
 	}, HookVars{}); err != nil {
 		return fmt.Errorf("execute prune end hooks: %w", err)
diff --git a/internal/orchestrator/tasks/taskrestore.go b/internal/orchestrator/tasks/taskrestore.go
index f55f95b7..e9d290a6 100644
--- a/internal/orchestrator/tasks/taskrestore.go
+++ b/internal/orchestrator/tasks/taskrestore.go
@@ -33,7 +33,7 @@ func NewOneoffRestoreTask(repoID, planID string, flowID int64, at time.Time, sna
 		},
 		Do: func(ctx context.Context, st ScheduledTask, taskRunner TaskRunner) error {
 			if err := restoreHelper(ctx, st, taskRunner, snapshotID, path, target); err != nil {
-				taskRunner.ExecuteHooks([]v1.Hook_Condition{
+				taskRunner.ExecuteHooks(ctx, []v1.Hook_Condition{
 					v1.Hook_CONDITION_ANY_ERROR,
 				}, HookVars{
 					Task:  st.Task.Name(),
diff --git a/internal/orchestrator/tasks/taskrunhook.go b/internal/orchestrator/tasks/taskrunhook.go
new file mode 100644
index 00000000..9b29ce4d
--- /dev/null
+++ b/internal/orchestrator/tasks/taskrunhook.go
@@ -0,0 +1 @@
+package tasks
diff --git a/internal/orchestrator/tasks/taskstats.go b/internal/orchestrator/tasks/taskstats.go
index 0372fe31..f1cd8aac 100644
--- a/internal/orchestrator/tasks/taskstats.go
+++ b/internal/orchestrator/tasks/taskstats.go
@@ -69,7 +69,7 @@ func (t *StatsTask) Next(now time.Time, runner TaskRunner) (ScheduledTask, error
 
 func (t *StatsTask) Run(ctx context.Context, st ScheduledTask, runner TaskRunner) error {
 	if err := statsHelper(ctx, st, runner); err != nil {
-		runner.ExecuteHooks([]v1.Hook_Condition{
+		runner.ExecuteHooks(ctx, []v1.Hook_Condition{
 			v1.Hook_CONDITION_ANY_ERROR,
 		}, HookVars{
 			Task:  st.Task.Name(),
diff --git a/internal/queue/genheap.go b/internal/queue/genheap.go
index dde1c23e..4e46e2e6 100644
--- a/internal/queue/genheap.go
+++ b/internal/queue/genheap.go
@@ -1,23 +1,23 @@
 package queue
 
 // genericHeap is a generic heap implementation that can be used with any type that satisfies the constraints.Ordered interface.
-type genericHeap[T comparable[T]] []T
+type GenericHeap[T Comparable[T]] []T
 
-func (h genericHeap[T]) Len() int {
+func (h GenericHeap[T]) Len() int {
 	return len(h)
 }
 
-func (h genericHeap[T]) Swap(i, j int) {
+func (h GenericHeap[T]) Swap(i, j int) {
 	h[i], h[j] = h[j], h[i]
 }
 
 // Push pushes an element onto the heap. Do not call directly, use heap.Push
-func (h *genericHeap[T]) Push(x interface{}) {
+func (h *GenericHeap[T]) Push(x interface{}) {
 	*h = append(*h, x.(T))
 }
 
 // Pop pops an element from the heap. Do not call directly, use heap.Pop
-func (h *genericHeap[T]) Pop() interface{} {
+func (h *GenericHeap[T]) Pop() interface{} {
 	old := *h
 	n := len(old)
 	x := old[n-1]
@@ -25,7 +25,7 @@ func (h *genericHeap[T]) Pop() interface{} {
 	return x
 }
 
-func (h genericHeap[T]) Peek() T {
+func (h GenericHeap[T]) Peek() T {
 	if len(h) == 0 {
 		var zero T
 		return zero
@@ -33,10 +33,10 @@ func (h genericHeap[T]) Peek() T {
 	return h[0]
 }
 
-func (h genericHeap[T]) Less(i, j int) bool {
+func (h GenericHeap[T]) Less(i, j int) bool {
 	return h[i].Less(h[j])
 }
 
-type comparable[T any] interface {
+type Comparable[T any] interface {
 	Less(other T) bool
 }
diff --git a/internal/queue/genheap_test.go b/internal/queue/genheap_test.go
index 61bee966..e7167a62 100644
--- a/internal/queue/genheap_test.go
+++ b/internal/queue/genheap_test.go
@@ -19,7 +19,7 @@ func (v val) Eq(other val) bool {
 
 func TestGenericHeapInit(t *testing.T) {
 	t.Parallel()
-	genHeap := genericHeap[val]{{v: 3}, {v: 2}, {v: 1}}
+	genHeap := GenericHeap[val]{{v: 3}, {v: 2}, {v: 1}}
 	heap.Init(&genHeap)
 
 	if genHeap.Len() != 3 {
@@ -36,7 +36,7 @@ func TestGenericHeapInit(t *testing.T) {
 
 func TestGenericHeapPushPop(t *testing.T) {
 	t.Parallel()
-	genHeap := genericHeap[val]{} // empty heap
+	genHeap := GenericHeap[val]{} // empty heap
 	heap.Push(&genHeap, val{v: 3})
 	heap.Push(&genHeap, val{v: 2})
 	heap.Push(&genHeap, val{v: 1})
diff --git a/internal/queue/timepriorityqueue.go b/internal/queue/timepriorityqueue.go
index de511ac0..a3f60fe7 100644
--- a/internal/queue/timepriorityqueue.go
+++ b/internal/queue/timepriorityqueue.go
@@ -9,13 +9,13 @@ import (
 // TimePriorityQueue is a priority queue that dequeues elements at (or after) a specified time, and prioritizes elements based on a priority value. It is safe for concurrent use.
 type TimePriorityQueue[T equals[T]] struct {
 	tqueue TimeQueue[priorityEntry[T]]
-	ready  genericHeap[priorityEntry[T]]
+	ready  GenericHeap[priorityEntry[T]]
 }
 
 func NewTimePriorityQueue[T equals[T]]() *TimePriorityQueue[T] {
 	return &TimePriorityQueue[T]{
 		tqueue: TimeQueue[priorityEntry[T]]{},
-		ready:  genericHeap[priorityEntry[T]]{},
+		ready:  GenericHeap[priorityEntry[T]]{},
 	}
 }
 
diff --git a/internal/queue/timequeue.go b/internal/queue/timequeue.go
index 0e9b409c..5674103b 100644
--- a/internal/queue/timequeue.go
+++ b/internal/queue/timequeue.go
@@ -10,7 +10,7 @@ import (
 
 // TimeQueue is a priority queue that dequeues elements at (or after) a specified time. It is safe for concurrent use.
 type TimeQueue[T equals[T]] struct {
-	heap genericHeap[timeQueueEntry[T]]
+	heap GenericHeap[timeQueueEntry[T]]
 
 	dequeueMu sync.Mutex
 	mu        sync.Mutex
@@ -19,7 +19,7 @@ type TimeQueue[T equals[T]] struct {
 
 func NewTimeQueue[T equals[T]]() *TimeQueue[T] {
 	return &TimeQueue[T]{
-		heap: genericHeap[timeQueueEntry[T]]{},
+		heap: GenericHeap[timeQueueEntry[T]]{},
 	}
 }
 

From 3ea8e7abc3daddea5c3dae268e68dcb6a1270c14 Mon Sep 17 00:00:00 2001
From: garethgeorge <garethgeorge97@gmail.com>
Date: Tue, 9 Jul 2024 22:09:37 -0700
Subject: [PATCH 07/15] more refactoring work

---
 internal/config/configutil.go                 | 21 ++++++
 internal/hook/hook.go                         | 71 +++++++------------
 internal/hook/{ => hookutil}/httputil.go      |  2 +-
 internal/hook/{ => hookutil}/templateutil.go  |  6 +-
 internal/hook/types/command.go                |  6 +-
 internal/hook/types/discord.go                |  8 +--
 internal/hook/types/gotify.go                 | 10 +--
 .../hook/{handler.go => types/registry.go}    |  2 +-
 internal/hook/types/shoutrrr.go               |  6 +-
 internal/hook/types/slack.go                  |  8 +--
 internal/orchestrator/orchestrator.go         | 42 +++++------
 internal/orchestrator/taskrunnerimpl.go       | 48 ++++++++-----
 internal/orchestrator/tasks/task.go           |  4 +-
 internal/orchestrator/tasks/taskcheck.go      |  2 -
 internal/orchestrator/tasks/taskforget.go     | 10 +--
 .../orchestrator/tasks/taskforgetsnapshot.go  | 10 +--
 .../orchestrator/tasks/taskindexsnapshots.go  |  8 +--
 internal/orchestrator/tasks/taskprune.go      |  2 -
 internal/orchestrator/tasks/taskrestore.go    | 10 +--
 internal/orchestrator/tasks/taskrunhook.go    |  1 -
 20 files changed, 143 insertions(+), 134 deletions(-)
 create mode 100644 internal/config/configutil.go
 rename internal/hook/{ => hookutil}/httputil.go (97%)
 rename internal/hook/{ => hookutil}/templateutil.go (91%)
 rename internal/hook/{handler.go => types/registry.go} (98%)
 delete mode 100644 internal/orchestrator/tasks/taskrunhook.go

diff --git a/internal/config/configutil.go b/internal/config/configutil.go
new file mode 100644
index 00000000..70eb9023
--- /dev/null
+++ b/internal/config/configutil.go
@@ -0,0 +1,21 @@
+package config
+
+import v1 "github.com/garethgeorge/backrest/gen/go/v1"
+
+func FindPlan(cfg *v1.Config, planID string) *v1.Plan {
+	for _, plan := range cfg.Plans {
+		if plan.Id == planID {
+			return plan
+		}
+	}
+	return nil
+}
+
+func FindRepo(cfg *v1.Config, repoID string) *v1.Repo {
+	for _, repo := range cfg.Repos {
+		if repo.Id == repoID {
+			return repo
+		}
+	}
+	return nil
+}
diff --git a/internal/hook/hook.go b/internal/hook/hook.go
index d6f9431e..dedc5cac 100644
--- a/internal/hook/hook.go
+++ b/internal/hook/hook.go
@@ -9,54 +9,29 @@ import (
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
+	cfg "github.com/garethgeorge/backrest/internal/config"
+	"github.com/garethgeorge/backrest/internal/hook/types"
 	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
 	"google.golang.org/protobuf/proto"
 )
 
-var (
-	DefaultTemplate = `{{ .Summary }}`
-)
-
-func ExecuteHookTasks(ctx context.Context, runner tasks.ScheduledTaskExecutor, hookTasks []tasks.Task) error {
-	for _, task := range hookTasks {
-		err := runner.RunTask(ctx, tasks.ScheduledTask{Task: task, RunAt: time.Now()})
-		if isHaltingError(err) {
-			return fmt.Errorf("hook %v: %w", task.Name(), err)
-		}
-	}
-	return nil
-}
-
 func TasksTriggeredByEvent(config *v1.Config, flowID int64, planID string, repoID string, events []v1.Hook_Condition, vars interface{}) ([]tasks.Task, error) {
 	var taskSet []tasks.Task
 
-	var plan *v1.Plan
-	var repo *v1.Repo
-	if repoID != "" {
-		if repoIdx := slices.IndexFunc(config.Repos, func(r *v1.Repo) bool {
-			return r.GetId() == repoID
-		}); repoIdx != -1 {
-			repo = config.Repos[repoIdx]
-		} else {
-			return nil, fmt.Errorf("repo %v not found", repoID)
-		}
+	repo := cfg.FindRepo(config, repoID)
+	if repo == nil {
+		return nil, fmt.Errorf("repo %v not found", repoID)
 	}
-
-	if planID != "" {
-		if planIdx := slices.IndexFunc(config.Plans, func(p *v1.Plan) bool {
-			return p.GetId() == planID
-		}); planIdx != -1 {
-			plan = config.Plans[planIdx]
-		} else {
-			return nil, fmt.Errorf("plan %v not found", planID)
-		}
-	} else {
-		planID = tasks.PlanForSystemTasks
+	plan := cfg.FindPlan(config, planID)
+	if plan == nil && planID != "" {
+		return nil, fmt.Errorf("plan %v not found", planID)
 	}
 
 	baseOp := v1.Operation{
 		Status:          v1.OperationStatus_STATUS_PENDING,
 		InstanceId:      config.Instance,
+		PlanId:          planID,
+		RepoId:          repoID,
 		FlowId:          flowID,
 		UnixTimeStartMs: curTimeMs(),
 	}
@@ -104,12 +79,12 @@ func TasksTriggeredByEvent(config *v1.Config, flowID int64, planID string, repoI
 
 func newOneoffRunHookTask(title, repoID, planID string, flowID int64, at time.Time, hook *v1.Hook, event v1.Hook_Condition, vars interface{}) tasks.Task {
 	return &tasks.GenericOneoffTask{
-		BaseTask: tasks.BaseTask{
-			TaskName:   fmt.Sprintf("run hook %v", title),
-			TaskRepoID: repoID,
-			TaskPlanID: planID,
-		},
 		OneoffTask: tasks.OneoffTask{
+			BaseTask: tasks.BaseTask{
+				TaskName:   fmt.Sprintf("run hook %v", title),
+				TaskRepoID: repoID,
+				TaskPlanID: planID,
+			},
 			FlowID: flowID,
 			RunAt:  at,
 			ProtoOp: &v1.Operation{
@@ -117,15 +92,19 @@ func newOneoffRunHookTask(title, repoID, planID string, flowID int64, at time.Ti
 			},
 		},
 		Do: func(ctx context.Context, st tasks.ScheduledTask, taskRunner tasks.TaskRunner) error {
-			h, err := DefaultRegistry().GetHandler(hook)
+			h, err := types.DefaultRegistry().GetHandler(hook)
 			if err != nil {
 				return err
 			}
-			if eventField := reflect.ValueOf(vars).FieldByName("Event"); eventField.IsValid() {
-				eventField.Set(reflect.ValueOf(event))
+
+			v := reflect.ValueOf(&vars).Elem()
+			clone := reflect.New(v.Elem().Type()).Elem()
+			clone.Set(v.Elem()) // copy vars to clone
+			if field := v.Elem().FieldByName("Event"); field.IsValid() {
+				clone.FieldByName("Event").Set(reflect.ValueOf(event))
 			}
 
-			if err := h.Execute(ctx, hook, vars, taskRunner); err != nil {
+			if err := h.Execute(ctx, hook, clone, taskRunner); err != nil {
 				err = applyHookErrorPolicy(hook.OnError, err)
 				return err
 			}
@@ -162,8 +141,8 @@ func applyHookErrorPolicy(onError v1.Hook_OnError, err error) error {
 	return err
 }
 
-// isHaltingError returns true if the error is a fatal error or a request to cancel the operation
-func isHaltingError(err error) bool {
+// IsHaltingError returns true if the error is a fatal error or a request to cancel the operation
+func IsHaltingError(err error) bool {
 	var fatalErr *HookErrorFatal
 	var cancelErr *HookErrorRequestCancel
 	return errors.As(err, &fatalErr) || errors.As(err, &cancelErr)
diff --git a/internal/hook/httputil.go b/internal/hook/hookutil/httputil.go
similarity index 97%
rename from internal/hook/httputil.go
rename to internal/hook/hookutil/httputil.go
index fd4123b1..9d006388 100644
--- a/internal/hook/httputil.go
+++ b/internal/hook/hookutil/httputil.go
@@ -1,4 +1,4 @@
-package hook
+package hookutil
 
 import (
 	"fmt"
diff --git a/internal/hook/templateutil.go b/internal/hook/hookutil/templateutil.go
similarity index 91%
rename from internal/hook/templateutil.go
rename to internal/hook/hookutil/templateutil.go
index 9c0fe4ce..eee8c81f 100644
--- a/internal/hook/templateutil.go
+++ b/internal/hook/hookutil/templateutil.go
@@ -1,4 +1,4 @@
-package hook
+package hookutil
 
 import (
 	"bytes"
@@ -7,6 +7,10 @@ import (
 	"text/template"
 )
 
+var (
+	DefaultTemplate = `{{ .Summary }}`
+)
+
 func RenderTemplate(text string, vars interface{}) (string, error) {
 	template, err := template.New("template").Parse(text)
 	if err != nil {
diff --git a/internal/hook/types/command.go b/internal/hook/types/command.go
index 1ad0696a..ba6b374e 100644
--- a/internal/hook/types/command.go
+++ b/internal/hook/types/command.go
@@ -8,7 +8,7 @@ import (
 	"strings"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-	"github.com/garethgeorge/backrest/internal/hook"
+	"github.com/garethgeorge/backrest/internal/hook/hookutil"
 	"github.com/garethgeorge/backrest/internal/ioutil"
 	"github.com/garethgeorge/backrest/internal/orchestrator/logging"
 	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
@@ -17,7 +17,7 @@ import (
 type commandHandler struct{}
 
 func (commandHandler) Execute(ctx context.Context, h *v1.Hook, vars interface{}, runner tasks.TaskRunner) error {
-	command, err := hook.RenderTemplate(h.GetActionCommand().GetCommand(), vars)
+	command, err := hookutil.RenderTemplate(h.GetActionCommand().GetCommand(), vars)
 	if err != nil {
 		return fmt.Errorf("template rendering: %w", err)
 	}
@@ -57,5 +57,5 @@ func (commandHandler) ActionType() reflect.Type {
 }
 
 func init() {
-	hook.DefaultRegistry().RegisterHandler(&commandHandler{})
+	DefaultRegistry().RegisterHandler(&commandHandler{})
 }
diff --git a/internal/hook/types/discord.go b/internal/hook/types/discord.go
index 7a6b13cf..f1677d54 100644
--- a/internal/hook/types/discord.go
+++ b/internal/hook/types/discord.go
@@ -8,14 +8,14 @@ import (
 	"reflect"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-	"github.com/garethgeorge/backrest/internal/hook"
+	"github.com/garethgeorge/backrest/internal/hook/hookutil"
 	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
 )
 
 type discordHandler struct{}
 
 func (discordHandler) Execute(ctx context.Context, h *v1.Hook, vars interface{}, runner tasks.TaskRunner) error {
-	payload, err := hook.RenderTemplateOrDefault(h.GetActionDiscord().GetTemplate(), hook.DefaultTemplate, vars)
+	payload, err := hookutil.RenderTemplateOrDefault(h.GetActionDiscord().GetTemplate(), hookutil.DefaultTemplate, vars)
 	if err != nil {
 		return fmt.Errorf("template rendering: %w", err)
 	}
@@ -33,7 +33,7 @@ func (discordHandler) Execute(ctx context.Context, h *v1.Hook, vars interface{},
 	}
 
 	requestBytes, _ := json.Marshal(request)
-	_, err = hook.PostRequest(h.GetActionDiscord().GetWebhookUrl(), "application/json", bytes.NewReader(requestBytes))
+	_, err = hookutil.PostRequest(h.GetActionDiscord().GetWebhookUrl(), "application/json", bytes.NewReader(requestBytes))
 	return err
 }
 
@@ -42,5 +42,5 @@ func (discordHandler) ActionType() reflect.Type {
 }
 
 func init() {
-	hook.DefaultRegistry().RegisterHandler(&discordHandler{})
+	DefaultRegistry().RegisterHandler(&discordHandler{})
 }
diff --git a/internal/hook/types/gotify.go b/internal/hook/types/gotify.go
index c2a9b0f7..a2d74b65 100644
--- a/internal/hook/types/gotify.go
+++ b/internal/hook/types/gotify.go
@@ -10,7 +10,7 @@ import (
 	"strings"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-	"github.com/garethgeorge/backrest/internal/hook"
+	"github.com/garethgeorge/backrest/internal/hook/hookutil"
 	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
 )
 
@@ -19,12 +19,12 @@ type gotifyHandler struct{}
 func (gotifyHandler) Execute(ctx context.Context, h *v1.Hook, vars interface{}, runner tasks.TaskRunner) error {
 	g := h.GetActionGotify()
 
-	payload, err := hook.RenderTemplateOrDefault(g.GetTemplate(), hook.DefaultTemplate, vars)
+	payload, err := hookutil.RenderTemplateOrDefault(g.GetTemplate(), hookutil.DefaultTemplate, vars)
 	if err != nil {
 		return fmt.Errorf("template rendering: %w", err)
 	}
 
-	title, err := hook.RenderTemplateOrDefault(g.GetTitleTemplate(), "Backrest Event", vars)
+	title, err := hookutil.RenderTemplateOrDefault(g.GetTitleTemplate(), "Backrest Event", vars)
 	if err != nil {
 		return fmt.Errorf("title template rendering: %w", err)
 	}
@@ -57,7 +57,7 @@ func (gotifyHandler) Execute(ctx context.Context, h *v1.Hook, vars interface{},
 	fmt.Fprintf(output, "---- payload ----\n")
 	output.Write(b)
 
-	body, err := hook.PostRequest(postUrl, "application/json", bytes.NewReader(b))
+	body, err := hookutil.PostRequest(postUrl, "application/json", bytes.NewReader(b))
 
 	if err != nil {
 		return fmt.Errorf("send gotify message: %w", err)
@@ -75,5 +75,5 @@ func (gotifyHandler) ActionType() reflect.Type {
 }
 
 func init() {
-	hook.DefaultRegistry().RegisterHandler(&gotifyHandler{})
+	DefaultRegistry().RegisterHandler(&gotifyHandler{})
 }
diff --git a/internal/hook/handler.go b/internal/hook/types/registry.go
similarity index 98%
rename from internal/hook/handler.go
rename to internal/hook/types/registry.go
index 110a41b3..cf6bbc03 100644
--- a/internal/hook/handler.go
+++ b/internal/hook/types/registry.go
@@ -1,4 +1,4 @@
-package hook
+package types
 
 import (
 	"context"
diff --git a/internal/hook/types/shoutrrr.go b/internal/hook/types/shoutrrr.go
index 39cbd799..65e8a44a 100644
--- a/internal/hook/types/shoutrrr.go
+++ b/internal/hook/types/shoutrrr.go
@@ -7,14 +7,14 @@ import (
 
 	"github.com/containrrr/shoutrrr"
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-	"github.com/garethgeorge/backrest/internal/hook"
+	"github.com/garethgeorge/backrest/internal/hook/hookutil"
 	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
 )
 
 type shoutrrrHandler struct{}
 
 func (shoutrrrHandler) Execute(ctx context.Context, h *v1.Hook, vars interface{}, runner tasks.TaskRunner) error {
-	payload, err := hook.RenderTemplateOrDefault(h.GetActionShoutrrr().GetTemplate(), hook.DefaultTemplate, vars)
+	payload, err := hookutil.RenderTemplateOrDefault(h.GetActionShoutrrr().GetTemplate(), hookutil.DefaultTemplate, vars)
 	if err != nil {
 		return fmt.Errorf("template rendering: %w", err)
 	}
@@ -35,5 +35,5 @@ func (shoutrrrHandler) ActionType() reflect.Type {
 }
 
 func init() {
-	hook.DefaultRegistry().RegisterHandler(&shoutrrrHandler{})
+	DefaultRegistry().RegisterHandler(&shoutrrrHandler{})
 }
diff --git a/internal/hook/types/slack.go b/internal/hook/types/slack.go
index 689e7e65..8887ec30 100644
--- a/internal/hook/types/slack.go
+++ b/internal/hook/types/slack.go
@@ -8,14 +8,14 @@ import (
 	"reflect"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-	"github.com/garethgeorge/backrest/internal/hook"
+	"github.com/garethgeorge/backrest/internal/hook/hookutil"
 	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
 )
 
 type slackHandler struct{}
 
 func (slackHandler) Execute(ctx context.Context, cmd *v1.Hook, vars interface{}, runner tasks.TaskRunner) error {
-	payload, err := hook.RenderTemplateOrDefault(cmd.GetActionSlack().GetTemplate(), hook.DefaultTemplate, vars)
+	payload, err := hookutil.RenderTemplateOrDefault(cmd.GetActionSlack().GetTemplate(), hookutil.DefaultTemplate, vars)
 	if err != nil {
 		return fmt.Errorf("template rendering: %w", err)
 	}
@@ -34,7 +34,7 @@ func (slackHandler) Execute(ctx context.Context, cmd *v1.Hook, vars interface{},
 
 	requestBytes, _ := json.Marshal(request)
 
-	_, err = hook.PostRequest(cmd.GetActionSlack().GetWebhookUrl(), "application/json", bytes.NewReader(requestBytes))
+	_, err = hookutil.PostRequest(cmd.GetActionSlack().GetWebhookUrl(), "application/json", bytes.NewReader(requestBytes))
 	return err
 }
 
@@ -43,5 +43,5 @@ func (slackHandler) ActionType() reflect.Type {
 }
 
 func init() {
-	hook.DefaultRegistry().RegisterHandler(&slackHandler{})
+	DefaultRegistry().RegisterHandler(&slackHandler{})
 }
diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go
index 353a2c53..dc764d03 100644
--- a/internal/orchestrator/orchestrator.go
+++ b/internal/orchestrator/orchestrator.go
@@ -11,6 +11,7 @@ import (
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
+	"github.com/garethgeorge/backrest/internal/config"
 	"github.com/garethgeorge/backrest/internal/ioutil"
 	"github.com/garethgeorge/backrest/internal/oplog"
 	"github.com/garethgeorge/backrest/internal/orchestrator/logging"
@@ -43,7 +44,7 @@ type Orchestrator struct {
 	now func() time.Time
 }
 
-var _ tasks.ScheduledTaskExecutor = &Orchestrator{}
+var _ tasks.TaskExecutor = &Orchestrator{}
 
 type stContainer struct {
 	tasks.ScheduledTask
@@ -193,30 +194,26 @@ func (o *Orchestrator) GetRepoOrchestrator(repoId string) (repo *repo.RepoOrches
 	return r, nil
 }
 
-func (o *Orchestrator) GetRepo(repoId string) (*v1.Repo, error) {
+func (o *Orchestrator) GetRepo(repoID string) (*v1.Repo, error) {
 	o.mu.Lock()
 	defer o.mu.Unlock()
 
-	for _, r := range o.config.Repos {
-		if r.GetId() == repoId {
-			return r, nil
-		}
+	repo := config.FindRepo(o.config, repoID)
+	if repo == nil {
+		return nil, fmt.Errorf("get repo %q: %w", repoID, ErrRepoNotFound)
 	}
-
-	return nil, fmt.Errorf("get repo %q: %w", repoId, ErrRepoNotFound)
+	return repo, nil
 }
 
-func (o *Orchestrator) GetPlan(planId string) (*v1.Plan, error) {
+func (o *Orchestrator) GetPlan(planID string) (*v1.Plan, error) {
 	o.mu.Lock()
 	defer o.mu.Unlock()
 
-	for _, p := range o.config.Plans {
-		if p.Id == planId {
-			return p, nil
-		}
+	plan := config.FindPlan(o.config, planID)
+	if plan == nil {
+		return nil, fmt.Errorf("get plan %q: %w", planID, ErrPlanNotFound)
 	}
-
-	return nil, fmt.Errorf("get plan %q: %w", planId, ErrPlanNotFound)
+	return plan, nil
 }
 
 func (o *Orchestrator) CancelOperation(operationId int64, status v1.OperationStatus) error {
@@ -300,15 +297,8 @@ func (o *Orchestrator) Run(ctx context.Context) {
 				}
 			}
 		}()
-		start := time.Now()
 
-		zap.L().Info("running task", zap.String("task", t.Task.Name()))
 		err := o.RunTask(taskCtx, t.ScheduledTask)
-		if err != nil {
-			zap.L().Error("task failed", zap.String("task", t.Task.Name()), zap.Error(err), zap.Duration("duration", time.Since(start)))
-		} else {
-			zap.L().Info("task finished", zap.String("task", t.Task.Name()), zap.Duration("duration", time.Since(start)))
-		}
 
 		o.mu.Lock()
 		curCfgModno := o.config.Modno
@@ -333,6 +323,8 @@ func (o *Orchestrator) RunTask(ctx context.Context, st tasks.ScheduledTask) erro
 
 	runner := newTaskRunnerImpl(o, st.Task, st.Op)
 
+	zap.L().Info("running task", zap.String("task", st.Task.Name()), zap.String("runAt", st.RunAt.Format(time.RFC3339)))
+
 	op := st.Op
 	if op != nil {
 		op.UnixTimeStartMs = time.Now().UnixMilli()
@@ -350,10 +342,13 @@ func (o *Orchestrator) RunTask(ctx context.Context, st tasks.ScheduledTask) erro
 		}
 	}
 
+	start := time.Now()
 	err := st.Task.Run(ctx, st, runner)
 	if err != nil {
+		zap.L().Error("task failed", zap.String("task", st.Task.Name()), zap.Error(err), zap.Duration("duration", time.Since(start)))
 		fmt.Fprintf(logs, "\ntask %q returned error: %v\n", st.Task.Name(), err)
 	} else {
+		zap.L().Info("task finished", zap.String("task", st.Task.Name()), zap.Duration("duration", time.Since(start)))
 		fmt.Fprintf(logs, "\ntask %q completed successfully\n", st.Task.Name())
 	}
 
@@ -389,6 +384,7 @@ func (o *Orchestrator) RunTask(ctx context.Context, st tasks.ScheduledTask) erro
 			zap.S().Errorf("failed to update operation in oplog: %v", e)
 		}
 	}
+
 	return err
 }
 
@@ -425,8 +421,8 @@ func (o *Orchestrator) scheduleTaskHelper(t tasks.Task, priority int, curTime ti
 		}
 	}
 
-	zap.L().Info("scheduling task", zap.String("task", t.Name()), zap.String("runAt", nextRun.RunAt.Format(time.RFC3339)))
 	o.taskQueue.Enqueue(nextRun.RunAt, priority, stc)
+	zap.L().Info("scheduled task", zap.String("task", t.Name()), zap.String("runAt", nextRun.RunAt.Format(time.RFC3339)))
 	return nil
 }
 
diff --git a/internal/orchestrator/taskrunnerimpl.go b/internal/orchestrator/taskrunnerimpl.go
index 787f7e8b..28da4b71 100644
--- a/internal/orchestrator/taskrunnerimpl.go
+++ b/internal/orchestrator/taskrunnerimpl.go
@@ -28,7 +28,15 @@ type taskRunnerImpl struct {
 
 var _ tasks.TaskRunner = &taskRunnerImpl{}
 
-func (t *taskRunnerImpl) FindRepo() (*v1.Repo, error) {
+func newTaskRunnerImpl(orchestrator *Orchestrator, task tasks.Task, op *v1.Operation) *taskRunnerImpl {
+	return &taskRunnerImpl{
+		orchestrator: orchestrator,
+		t:            task,
+		op:           op,
+	}
+}
+
+func (t *taskRunnerImpl) findRepo() (*v1.Repo, error) {
 	if t.repo != nil {
 		return t.repo, nil
 	}
@@ -37,7 +45,7 @@ func (t *taskRunnerImpl) FindRepo() (*v1.Repo, error) {
 	return t.repo, err
 }
 
-func (t *taskRunnerImpl) FindPlan() (*v1.Plan, error) {
+func (t *taskRunnerImpl) findPlan() (*v1.Plan, error) {
 	if t.plan != nil {
 		return t.plan, nil
 	}
@@ -46,14 +54,6 @@ func (t *taskRunnerImpl) FindPlan() (*v1.Plan, error) {
 	return t.plan, err
 }
 
-func newTaskRunnerImpl(orchestrator *Orchestrator, task tasks.Task, op *v1.Operation) *taskRunnerImpl {
-	return &taskRunnerImpl{
-		orchestrator: orchestrator,
-		t:            task,
-		op:           op,
-	}
-}
-
 func (t *taskRunnerImpl) CreateOperation(op *v1.Operation) error {
 	op.InstanceId = t.orchestrator.config.Instance
 	return t.orchestrator.OpLog.Add(op)
@@ -86,14 +86,14 @@ func (t *taskRunnerImpl) ExecuteHooks(ctx context.Context, events []v1.Hook_Cond
 	var plan *v1.Plan
 	if repoID != "" {
 		var err error
-		repo, err = t.FindRepo()
+		repo, err = t.findRepo()
 		if err != nil {
 			return err
 		}
 		vars.Repo = repo
 	}
 	if planID != "" {
-		plan, _ = t.FindPlan()
+		plan, _ = t.findPlan()
 		vars.Plan = plan
 	}
 	var flowID int64
@@ -106,21 +106,35 @@ func (t *taskRunnerImpl) ExecuteHooks(ctx context.Context, events []v1.Hook_Cond
 		return err
 	}
 
-	if err := hook.ExecuteHookTasks(ctx, t.orchestrator, hookTasks); err != nil {
-		var cancelErr hook.HookErrorRequestCancel
-		if errors.As(err, &cancelErr) {
-			return fmt.Errorf("%w: %w", tasks.ErrTaskCancelled, err)
+	for _, task := range hookTasks {
+		zap.L().Debug("running task", zap.Any("task", fmt.Sprintf("%+v", task)))
+		st, _ := task.Next(time.Now(), t)
+		st.Task = task
+		if err := t.OpLog().Add(st.Op); err != nil {
+			return fmt.Errorf("%v: %w", task.Name(), err)
+		}
+		if err := t.orchestrator.RunTask(ctx, st); hook.IsHaltingError(err) {
+			var cancelErr hook.HookErrorRequestCancel
+			if errors.As(err, &cancelErr) {
+				return fmt.Errorf("%w: %w", tasks.ErrTaskCancelled, err)
+			}
+			return fmt.Errorf("%v: %w", task.Name(), err)
 		}
-		return err
 	}
 	return nil
 }
 
 func (t *taskRunnerImpl) GetRepo(repoID string) (*v1.Repo, error) {
+	if repoID == t.t.RepoID() {
+		return t.findRepo() // optimization for the common case of the current repo
+	}
 	return t.orchestrator.GetRepo(repoID)
 }
 
 func (t *taskRunnerImpl) GetPlan(planID string) (*v1.Plan, error) {
+	if planID == t.t.PlanID() {
+		return t.findPlan() // optimization for the common case of the current plan
+	}
 	return t.orchestrator.GetPlan(planID)
 }
 
diff --git a/internal/orchestrator/tasks/task.go b/internal/orchestrator/tasks/task.go
index 5ce7101d..f165dbab 100644
--- a/internal/orchestrator/tasks/task.go
+++ b/internal/orchestrator/tasks/task.go
@@ -54,7 +54,7 @@ type TaskRunner interface {
 	RawLogWriter(ctx context.Context) io.Writer
 }
 
-type ScheduledTaskExecutor interface {
+type TaskExecutor interface {
 	RunTask(ctx context.Context, st ScheduledTask) error
 }
 
@@ -120,8 +120,8 @@ func (o *OneoffTask) Next(now time.Time, runner TaskRunner) (ScheduledTask, erro
 	var op *v1.Operation
 	if o.ProtoOp != nil {
 		op = proto.Clone(o.ProtoOp).(*v1.Operation)
-		op.PlanId = o.PlanID()
 		op.RepoId = o.RepoID()
+		op.PlanId = o.PlanID()
 		op.FlowId = o.FlowID
 		op.UnixTimeStartMs = timeToUnixMillis(o.RunAt) // TODO: this should be updated before Run is called.
 		op.Status = v1.OperationStatus_STATUS_PENDING
diff --git a/internal/orchestrator/tasks/taskcheck.go b/internal/orchestrator/tasks/taskcheck.go
index e17a38ba..b753bc1d 100644
--- a/internal/orchestrator/tasks/taskcheck.go
+++ b/internal/orchestrator/tasks/taskcheck.go
@@ -74,8 +74,6 @@ func (t *CheckTask) Next(now time.Time, runner TaskRunner) (ScheduledTask, error
 		lastRan = time.Now()
 	}
 
-	zap.L().Debug("last check time", zap.Time("time", lastRan), zap.String("repo", t.RepoID()))
-
 	runAt, err := protoutil.ResolveSchedule(repo.CheckPolicy.GetSchedule(), lastRan)
 	if errors.Is(err, protoutil.ErrScheduleDisabled) {
 		return NeverScheduledTask, nil
diff --git a/internal/orchestrator/tasks/taskforget.go b/internal/orchestrator/tasks/taskforget.go
index 834ba30c..9335726d 100644
--- a/internal/orchestrator/tasks/taskforget.go
+++ b/internal/orchestrator/tasks/taskforget.go
@@ -15,12 +15,12 @@ import (
 
 func NewOneoffForgetTask(repoID, planID string, flowID int64, at time.Time) Task {
 	return &GenericOneoffTask{
-		BaseTask: BaseTask{
-			TaskName:   fmt.Sprintf("forget for plan %q in repo %q", repoID, planID),
-			TaskRepoID: repoID,
-			TaskPlanID: planID,
-		},
 		OneoffTask: OneoffTask{
+			BaseTask: BaseTask{
+				TaskName:   fmt.Sprintf("forget for plan %q in repo %q", repoID, planID),
+				TaskRepoID: repoID,
+				TaskPlanID: planID,
+			},
 			FlowID: flowID,
 			RunAt:  at,
 			ProtoOp: &v1.Operation{
diff --git a/internal/orchestrator/tasks/taskforgetsnapshot.go b/internal/orchestrator/tasks/taskforgetsnapshot.go
index 3ef9c781..46c4351d 100644
--- a/internal/orchestrator/tasks/taskforgetsnapshot.go
+++ b/internal/orchestrator/tasks/taskforgetsnapshot.go
@@ -10,12 +10,12 @@ import (
 
 func NewOneoffForgetSnapshotTask(repoID, planID string, flowID int64, at time.Time, snapshotID string) Task {
 	return &GenericOneoffTask{
-		BaseTask: BaseTask{
-			TaskName:   fmt.Sprintf("forget snapshot %q for plan %q in repo %q", snapshotID, planID, repoID),
-			TaskRepoID: repoID,
-			TaskPlanID: planID,
-		},
 		OneoffTask: OneoffTask{
+			BaseTask: BaseTask{
+				TaskName:   fmt.Sprintf("forget snapshot %q for plan %q in repo %q", snapshotID, planID, repoID),
+				TaskRepoID: repoID,
+				TaskPlanID: planID,
+			},
 			FlowID: flowID,
 			RunAt:  at,
 			ProtoOp: &v1.Operation{
diff --git a/internal/orchestrator/tasks/taskindexsnapshots.go b/internal/orchestrator/tasks/taskindexsnapshots.go
index 75eb0d62..3c9ec155 100644
--- a/internal/orchestrator/tasks/taskindexsnapshots.go
+++ b/internal/orchestrator/tasks/taskindexsnapshots.go
@@ -18,11 +18,11 @@ import (
 
 func NewOneoffIndexSnapshotsTask(repoID string, at time.Time) Task {
 	return &GenericOneoffTask{
-		BaseTask: BaseTask{
-			TaskName:   fmt.Sprintf("index snapshots for repo %q", repoID),
-			TaskRepoID: repoID,
-		},
 		OneoffTask: OneoffTask{
+			BaseTask: BaseTask{
+				TaskName:   fmt.Sprintf("index snapshots for repo %q", repoID),
+				TaskRepoID: repoID,
+			},
 			RunAt:   at,
 			ProtoOp: nil,
 		},
diff --git a/internal/orchestrator/tasks/taskprune.go b/internal/orchestrator/tasks/taskprune.go
index c2c32f92..574a2320 100644
--- a/internal/orchestrator/tasks/taskprune.go
+++ b/internal/orchestrator/tasks/taskprune.go
@@ -74,8 +74,6 @@ func (t *PruneTask) Next(now time.Time, runner TaskRunner) (ScheduledTask, error
 		lastRan = time.Now()
 	}
 
-	zap.L().Debug("last prune time", zap.Time("time", lastRan), zap.String("repo", t.RepoID()))
-
 	runAt, err := protoutil.ResolveSchedule(repo.PrunePolicy.GetSchedule(), lastRan)
 	if errors.Is(err, protoutil.ErrScheduleDisabled) {
 		return NeverScheduledTask, nil
diff --git a/internal/orchestrator/tasks/taskrestore.go b/internal/orchestrator/tasks/taskrestore.go
index e9d290a6..a10c0b1d 100644
--- a/internal/orchestrator/tasks/taskrestore.go
+++ b/internal/orchestrator/tasks/taskrestore.go
@@ -13,12 +13,12 @@ import (
 
 func NewOneoffRestoreTask(repoID, planID string, flowID int64, at time.Time, snapshotID, path, target string) Task {
 	return &GenericOneoffTask{
-		BaseTask: BaseTask{
-			TaskName:   fmt.Sprintf("restore snapshot %q in repo %q", snapshotID, repoID),
-			TaskRepoID: repoID,
-			TaskPlanID: planID,
-		},
 		OneoffTask: OneoffTask{
+			BaseTask: BaseTask{
+				TaskName:   fmt.Sprintf("restore snapshot %q in repo %q", snapshotID, repoID),
+				TaskRepoID: repoID,
+				TaskPlanID: planID,
+			},
 			FlowID: flowID,
 			RunAt:  at,
 			ProtoOp: &v1.Operation{
diff --git a/internal/orchestrator/tasks/taskrunhook.go b/internal/orchestrator/tasks/taskrunhook.go
deleted file mode 100644
index 9b29ce4d..00000000
--- a/internal/orchestrator/tasks/taskrunhook.go
+++ /dev/null
@@ -1 +0,0 @@
-package tasks

From 3812228c04028b39a491cd5e9a11aa291773845c Mon Sep 17 00:00:00 2001
From: garethgeorge <garethgeorge97@gmail.com>
Date: Tue, 9 Jul 2024 23:06:27 -0700
Subject: [PATCH 08/15] UI bug fixes

---
 webui/src/components/OperationList.tsx |  2 --
 webui/src/components/OperationRow.tsx  |  3 ---
 webui/src/components/OperationTree.tsx | 15 +++---------
 webui/src/state/oplog.ts               | 32 +++++++++++---------------
 webui/src/views/App.tsx                |  7 ++++--
 webui/src/views/PlanView.tsx           |  1 -
 webui/src/views/RepoView.tsx           |  1 -
 7 files changed, 22 insertions(+), 39 deletions(-)

diff --git a/webui/src/components/OperationList.tsx b/webui/src/components/OperationList.tsx
index 66e8ba5c..20c32155 100644
--- a/webui/src/components/OperationList.tsx
+++ b/webui/src/components/OperationList.tsx
@@ -24,12 +24,10 @@ export const OperationList = ({
   req,
   useBackups,
   showPlan,
-  filter,
 }: React.PropsWithoutRef<{
   req?: GetOperationsRequest;
   useBackups?: BackupInfo[];
   showPlan?: boolean;
-  filter?: (op: Operation) => boolean; // if provided, only operations that pass this filter will be displayed.
 }>) => {
   const alertApi = useAlertApi();
 
diff --git a/webui/src/components/OperationRow.tsx b/webui/src/components/OperationRow.tsx
index e41aa639..09de251c 100644
--- a/webui/src/components/OperationRow.tsx
+++ b/webui/src/components/OperationRow.tsx
@@ -310,9 +310,6 @@ export const OperationRow = ({
     const triggeringCondition = proto3
       .getEnumType(Hook_Condition)
       .findNumber(hook.condition);
-    if (triggeringCondition !== undefined) {
-      displayMessage += "\ntriggered by condition: " + triggeringCondition.name;
-    }
   }
 
   const children = [];
diff --git a/webui/src/components/OperationTree.tsx b/webui/src/components/OperationTree.tsx
index b3001a67..973023b9 100644
--- a/webui/src/components/OperationTree.tsx
+++ b/webui/src/components/OperationTree.tsx
@@ -1,18 +1,13 @@
-import React, { useEffect, useMemo, useRef, useState } from "react";
+import React, { useEffect, useRef, useState } from "react";
 import {
   BackupInfo,
   BackupInfoCollector,
   colorForStatus,
   detailsForOperation,
   displayTypeToString,
-  getOperations,
   getTypeForDisplay,
-  matchSelector,
-  shouldHideOperation,
-  subscribeToOperations,
-  unsubscribeFromOperations,
 } from "../state/oplog";
-import { Button, Col, Divider, Empty, Modal, Row, Tooltip, Tree } from "antd";
+import { Col, Empty, Modal, Row, Tooltip, Tree } from "antd";
 import _ from "lodash";
 import { DataNode } from "antd/es/tree";
 import {
@@ -28,11 +23,7 @@ import {
   QuestionOutlined,
   SaveOutlined,
 } from "@ant-design/icons";
-import {
-  OperationEvent,
-  OperationEventType,
-  OperationStatus,
-} from "../../gen/ts/v1/operations_pb";
+import { OperationStatus } from "../../gen/ts/v1/operations_pb";
 import { useAlertApi } from "./Alerts";
 import { OperationList } from "./OperationList";
 import {
diff --git a/webui/src/state/oplog.ts b/webui/src/state/oplog.ts
index c06fe17c..93ff6e21 100644
--- a/webui/src/state/oplog.ts
+++ b/webui/src/state/oplog.ts
@@ -6,7 +6,7 @@ import {
 } from "../../gen/ts/v1/operations_pb";
 import { GetOperationsRequest, OpSelector } from "../../gen/ts/v1/service_pb";
 import { BackupProgressEntry, ResticSnapshot, RestoreProgressEntry } from "../../gen/ts/v1/restic_pb";
-import _ from "lodash";
+import _, { flow } from "lodash";
 import { formatDuration, formatTime } from "../lib/formatting";
 import { backrestService } from "../api";
 import {
@@ -63,7 +63,7 @@ export const unsubscribeFromOperations = (
 export const getStatusForSelector = async (sel: OpSelector) => {
   const req = new GetOperationsRequest({
     selector: sel,
-    lastN: BigInt(STATUS_OPERATION_HISTORY),
+    lastN: BigInt(20),
   });
   return await getStatus(req);
 };
@@ -74,25 +74,21 @@ const getStatus = async (req: GetOperationsRequest) => {
   ops.sort((a, b) => {
     return Number(b.unixTimeStartMs - a.unixTimeStartMs);
   });
-  if (ops.length === 0) {
-    return OperationStatus.STATUS_SUCCESS;
-  }
-  const flowId = ops.find((op) => op.status !== OperationStatus.STATUS_PENDING)?.flowId;
-  if (!flowId) {
-    return OperationStatus.STATUS_SUCCESS;
-  }
+
+  let flowID: BigInt | undefined = undefined;
   for (const op of ops) {
-    if (op.status === OperationStatus.STATUS_PENDING) {
+    if (op.status === OperationStatus.STATUS_PENDING || op.status === OperationStatus.STATUS_SYSTEM_CANCELLED) {
       continue;
     }
-    if (op.flowId !== flowId) {
+    if (op.status !== OperationStatus.STATUS_SUCCESS) {
+      return op.status;
+    }
+    if (!flowID) {
+      flowID = op.flowId;
+    } else if (flowID !== op.flowId) {
       break;
     }
-    if (
-      op.status !== OperationStatus.STATUS_SUCCESS &&
-      op.status !== OperationStatus.STATUS_USER_CANCELLED &&
-      op.status !== OperationStatus.STATUS_SYSTEM_CANCELLED
-    ) {
+    if (op.status !== OperationStatus.STATUS_SUCCESS) {
       return op.status;
     }
   }
@@ -437,7 +433,7 @@ export const colorForStatus = (status: OperationStatus) => {
     case OperationStatus.STATUS_SUCCESS:
       return "green";
     case OperationStatus.STATUS_USER_CANCELLED:
-      return "yellow";
+      return "orange";
     default:
       return "grey";
   }
@@ -481,7 +477,7 @@ export const detailsForOperation = (
       break;
     case OperationStatus.STATUS_USER_CANCELLED:
       state = "cancelled";
-      color = "yellow";
+      color = "orange";
       break;
     default:
       state = "";
diff --git a/webui/src/views/App.tsx b/webui/src/views/App.tsx
index ac476561..0110f86c 100644
--- a/webui/src/views/App.tsx
+++ b/webui/src/views/App.tsx
@@ -306,11 +306,14 @@ const IconForResource = ({
       setStatus(await getStatusForSelector(new OpSelector({ planId, repoId })));
     };
     load();
-    const refresh = _.debounce(load, 1000, { maxWait: 5000, trailing: true });
+    const refresh = _.debounce(load, 1000, { maxWait: 10000, trailing: true });
     const callback = (event?: OperationEvent, err?: Error) => {
       if (!event || !event.operation) return;
       const operation = event.operation;
-      if (operation.planId === planId || operation.repoId === repoId) {
+      if (
+        (planId && operation.planId === planId) ||
+        (repoId && operation.repoId === repoId)
+      ) {
         refresh();
       }
     };
diff --git a/webui/src/views/PlanView.tsx b/webui/src/views/PlanView.tsx
index 82c0f36c..c407e7a6 100644
--- a/webui/src/views/PlanView.tsx
+++ b/webui/src/views/PlanView.tsx
@@ -128,7 +128,6 @@ export const PlanView = ({ plan }: React.PropsWithChildren<{ plan: Plan }>) => {
                       lastN: BigInt(MAX_OPERATION_HISTORY),
                     })
                   }
-                  filter={(op) => !shouldHideStatus(op.status)}
                 />
               </>
             ),
diff --git a/webui/src/views/RepoView.tsx b/webui/src/views/RepoView.tsx
index 9c9a7fb5..db35b24f 100644
--- a/webui/src/views/RepoView.tsx
+++ b/webui/src/views/RepoView.tsx
@@ -141,7 +141,6 @@ export const RepoView = ({ repo }: React.PropsWithChildren<{ repo: Repo }>) => {
               })
             }
             showPlan={true}
-            filter={(op) => !shouldHideStatus(op.status)}
           />
         </>
       ),

From ac16fa8cc420c5a5c2ed54d0e55446cec693539b Mon Sep 17 00:00:00 2001
From: garethgeorge <garethgeorge97@gmail.com>
Date: Tue, 9 Jul 2024 23:06:38 -0700
Subject: [PATCH 09/15] bug fixes

---
 internal/hook/hook.go                   | 49 ++++++++-----------------
 internal/orchestrator/orchestrator.go   | 10 ++---
 internal/orchestrator/taskrunnerimpl.go |  6 ++-
 internal/orchestrator/tasks/task.go     |  1 -
 4 files changed, 25 insertions(+), 41 deletions(-)

diff --git a/internal/hook/hook.go b/internal/hook/hook.go
index dedc5cac..015591cc 100644
--- a/internal/hook/hook.go
+++ b/internal/hook/hook.go
@@ -12,10 +12,9 @@ import (
 	cfg "github.com/garethgeorge/backrest/internal/config"
 	"github.com/garethgeorge/backrest/internal/hook/types"
 	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
-	"google.golang.org/protobuf/proto"
 )
 
-func TasksTriggeredByEvent(config *v1.Config, flowID int64, planID string, repoID string, events []v1.Hook_Condition, vars interface{}) ([]tasks.Task, error) {
+func TasksTriggeredByEvent(config *v1.Config, repoID string, planID string, flowID int64, events []v1.Hook_Condition, vars interface{}) ([]tasks.Task, error) {
 	var taskSet []tasks.Task
 
 	repo := cfg.FindRepo(config, repoID)
@@ -27,15 +26,6 @@ func TasksTriggeredByEvent(config *v1.Config, flowID int64, planID string, repoI
 		return nil, fmt.Errorf("plan %v not found", planID)
 	}
 
-	baseOp := v1.Operation{
-		Status:          v1.OperationStatus_STATUS_PENDING,
-		InstanceId:      config.Instance,
-		PlanId:          planID,
-		RepoId:          repoID,
-		FlowId:          flowID,
-		UnixTimeStartMs: curTimeMs(),
-	}
-
 	for idx, hook := range repo.GetHooks() {
 		event := firstMatchingCondition(hook, events)
 		if event == v1.Hook_CONDITION_UNKNOWN {
@@ -43,16 +33,7 @@ func TasksTriggeredByEvent(config *v1.Config, flowID int64, planID string, repoI
 		}
 
 		name := fmt.Sprintf("repo/%v/hook/%v", repo.Id, idx)
-		op := proto.Clone(&baseOp).(*v1.Operation)
-		op.DisplayMessage = "running " + name
-		op.Op = &v1.Operation_OperationRunHook{
-			OperationRunHook: &v1.OperationRunHook{
-				Name:      name,
-				Condition: event,
-			},
-		}
-
-		taskSet = append(taskSet, newOneoffRunHookTask(name, repoID, planID, flowID, time.Now(), hook, event, vars))
+		taskSet = append(taskSet, newOneoffRunHookTask(name, config.Instance, repoID, planID, flowID, time.Now(), hook, event, vars))
 	}
 
 	for idx, hook := range plan.GetHooks() {
@@ -62,22 +43,13 @@ func TasksTriggeredByEvent(config *v1.Config, flowID int64, planID string, repoI
 		}
 
 		name := fmt.Sprintf("plan/%v/hook/%v", plan.Id, idx)
-		op := proto.Clone(&baseOp).(*v1.Operation)
-		op.DisplayMessage = fmt.Sprintf("running %v triggered by %v", name, event.String())
-		op.Op = &v1.Operation_OperationRunHook{
-			OperationRunHook: &v1.OperationRunHook{
-				Name:      name,
-				Condition: event,
-			},
-		}
-
-		taskSet = append(taskSet, newOneoffRunHookTask(name, repoID, planID, flowID, time.Now(), hook, event, vars))
+		taskSet = append(taskSet, newOneoffRunHookTask(name, config.Instance, repoID, planID, flowID, time.Now(), hook, event, vars))
 	}
 
 	return taskSet, nil
 }
 
-func newOneoffRunHookTask(title, repoID, planID string, flowID int64, at time.Time, hook *v1.Hook, event v1.Hook_Condition, vars interface{}) tasks.Task {
+func newOneoffRunHookTask(title, instanceID, repoID, planID string, flowID int64, at time.Time, hook *v1.Hook, event v1.Hook_Condition, vars interface{}) tasks.Task {
 	return &tasks.GenericOneoffTask{
 		OneoffTask: tasks.OneoffTask{
 			BaseTask: tasks.BaseTask{
@@ -88,7 +60,17 @@ func newOneoffRunHookTask(title, repoID, planID string, flowID int64, at time.Ti
 			FlowID: flowID,
 			RunAt:  at,
 			ProtoOp: &v1.Operation{
-				Op: &v1.Operation_OperationRunHook{},
+				InstanceId: instanceID,
+				RepoId:     repoID,
+				PlanId:     planID,
+
+				DisplayMessage: fmt.Sprintf("running %v triggered by %v", title, event.String()),
+				Op: &v1.Operation_OperationRunHook{
+					OperationRunHook: &v1.OperationRunHook{
+						Name:      title,
+						Condition: event,
+					},
+				},
 			},
 		},
 		Do: func(ctx context.Context, st tasks.ScheduledTask, taskRunner tasks.TaskRunner) error {
@@ -97,6 +79,7 @@ func newOneoffRunHookTask(title, repoID, planID string, flowID int64, at time.Ti
 				return err
 			}
 
+			// TODO: this is a hack to get around the fact that vars is an interface{} .
 			v := reflect.ValueOf(&vars).Elem()
 			clone := reflect.New(v.Elem().Type()).Elem()
 			clone.Set(v.Elem()) // copy vars to clone
diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go
index dc764d03..7ef36d92 100644
--- a/internal/orchestrator/orchestrator.go
+++ b/internal/orchestrator/orchestrator.go
@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"slices"
-	"strings"
 	"sync"
 	"time"
 
@@ -370,11 +369,12 @@ func (o *Orchestrator) RunTask(ctx context.Context, st tasks.ScheduledTask) erro
 				op.Status = v1.OperationStatus_STATUS_ERROR
 			}
 
-			// append the error to the display message
-			if op.DisplayMessage != "" && !strings.HasSuffix(op.DisplayMessage, "\n") {
-				op.DisplayMessage += "\n"
+			// prepend the error to the display message
+			if op.DisplayMessage != "" {
+				op.DisplayMessage = err.Error() + "\n\n" + op.DisplayMessage
+			} else {
+				op.DisplayMessage = err.Error()
 			}
-			op.DisplayMessage += err.Error()
 		}
 		op.UnixTimeEndMs = time.Now().UnixMilli()
 		if op.Status == v1.OperationStatus_STATUS_INPROGRESS {
diff --git a/internal/orchestrator/taskrunnerimpl.go b/internal/orchestrator/taskrunnerimpl.go
index 28da4b71..829a4434 100644
--- a/internal/orchestrator/taskrunnerimpl.go
+++ b/internal/orchestrator/taskrunnerimpl.go
@@ -98,10 +98,12 @@ func (t *taskRunnerImpl) ExecuteHooks(ctx context.Context, events []v1.Hook_Cond
 	}
 	var flowID int64
 	if t.op != nil {
-		flowID = t.op.FlowId
+		// a hook always belongs to a flow associated with the ID of the triggering task.
+		// this forms a tree.
+		flowID = t.op.Id
 	}
 
-	hookTasks, err := hook.TasksTriggeredByEvent(t.Config(), flowID, planID, repoID, events, vars)
+	hookTasks, err := hook.TasksTriggeredByEvent(t.Config(), repoID, planID, flowID, events, vars)
 	if err != nil {
 		return err
 	}
diff --git a/internal/orchestrator/tasks/task.go b/internal/orchestrator/tasks/task.go
index f165dbab..c7fe9c01 100644
--- a/internal/orchestrator/tasks/task.go
+++ b/internal/orchestrator/tasks/task.go
@@ -134,7 +134,6 @@ func (o *OneoffTask) Next(now time.Time, runner TaskRunner) (ScheduledTask, erro
 }
 
 type GenericOneoffTask struct {
-	BaseTask
 	OneoffTask
 	Do func(ctx context.Context, st ScheduledTask, runner TaskRunner) error
 }

From dcb27fa2813e162695acfe19928f9180ae1f544b Mon Sep 17 00:00:00 2001
From: garethgeorge <garethgeorge97@gmail.com>
Date: Tue, 9 Jul 2024 23:20:21 -0700
Subject: [PATCH 10/15] fix hook execution visibility in list view

---
 internal/orchestrator/taskrunnerimpl.go | 1 -
 webui/src/components/OperationList.tsx  | 5 ++++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/internal/orchestrator/taskrunnerimpl.go b/internal/orchestrator/taskrunnerimpl.go
index 829a4434..00015d58 100644
--- a/internal/orchestrator/taskrunnerimpl.go
+++ b/internal/orchestrator/taskrunnerimpl.go
@@ -109,7 +109,6 @@ func (t *taskRunnerImpl) ExecuteHooks(ctx context.Context, events []v1.Hook_Cond
 	}
 
 	for _, task := range hookTasks {
-		zap.L().Debug("running task", zap.Any("task", fmt.Sprintf("%+v", task)))
 		st, _ := task.Next(time.Now(), t)
 		st.Task = task
 		if err := t.OpLog().Add(st.Op); err != nil {
diff --git a/webui/src/components/OperationList.tsx b/webui/src/components/OperationList.tsx
index 20c32155..b22bc396 100644
--- a/webui/src/components/OperationList.tsx
+++ b/webui/src/components/OperationList.tsx
@@ -10,6 +10,7 @@ import {
   BackupInfoCollector,
   getOperations,
   matchSelector,
+  shouldHideStatus,
   subscribeToOperations,
   unsubscribeFromOperations,
 } from "../state/oplog";
@@ -38,7 +39,9 @@ export const OperationList = ({
 
     // track backups for this operation tree view.
     useEffect(() => {
-      const backupCollector = new BackupInfoCollector();
+      const backupCollector = new BackupInfoCollector(
+        (op) => !shouldHideStatus(op.status)
+      );
       backupCollector.subscribe(
         _.debounce(
           () => {

From db1c5f1b3ac3526b953aae001be626bce5f38ddf Mon Sep 17 00:00:00 2001
From: garethgeorge <garethgeorge97@gmail.com>
Date: Thu, 11 Jul 2024 00:53:01 -0700
Subject: [PATCH 11/15] improve grouping of hook executions with associated
 operations

---
 gen/go/v1/operations.pb.go              |  72 ++--
 internal/hook/hook.go                   |  12 +-
 internal/orchestrator/taskrunnerimpl.go |   8 +-
 proto/v1/operations.proto               |   5 +-
 webui/gen/ts/v1/operations_pb.ts        |  10 +-
 webui/src/components/OperationList.tsx  |  29 +-
 webui/src/components/OperationRow.tsx   | 456 ++++++++++++------------
 webui/src/components/OperationTree.tsx  |   2 +-
 webui/src/state/oplog.ts                |  11 +-
 9 files changed, 308 insertions(+), 297 deletions(-)

diff --git a/gen/go/v1/operations.pb.go b/gen/go/v1/operations.pb.go
index 172b80b2..9b88c83a 100644
--- a/gen/go/v1/operations.pb.go
+++ b/gen/go/v1/operations.pb.go
@@ -201,7 +201,7 @@ type Operation struct {
 	Status     OperationStatus `protobuf:"varint,4,opt,name=status,proto3,enum=v1.OperationStatus" json:"status,omitempty"`
 	// required, unix time in milliseconds of the operation's creation (ID is derived from this)
 	UnixTimeStartMs int64 `protobuf:"varint,5,opt,name=unix_time_start_ms,json=unixTimeStartMs,proto3" json:"unix_time_start_ms,omitempty"`
-	// optional, unix time in milliseconds of the operation's completion
+	// ptional, unix time in milliseconds of the operation's completion
 	UnixTimeEndMs int64 `protobuf:"varint,6,opt,name=unix_time_end_ms,json=unixTimeEndMs,proto3" json:"unix_time_end_ms,omitempty"`
 	// optional, human readable context message, typically an error message.
 	DisplayMessage string `protobuf:"bytes,7,opt,name=display_message,json=displayMessage,proto3" json:"display_message,omitempty"`
@@ -889,6 +889,7 @@ type OperationRunHook struct {
 	sizeCache     protoimpl.SizeCache
 	unknownFields protoimpl.UnknownFields
 
+	ParentOp     int64          `protobuf:"varint,4,opt,name=parent_op,json=parentOp,proto3" json:"parent_op,omitempty"`            // ID of the operation that ran the hook.
 	Name         string         `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"`                                     // description of the hook that was run. typically repo/hook_idx or plan/hook_idx.
 	OutputLogref string         `protobuf:"bytes,2,opt,name=output_logref,json=outputLogref,proto3" json:"output_logref,omitempty"` // logref of the hook's output. DEPRECATED.
 	Condition    Hook_Condition `protobuf:"varint,3,opt,name=condition,proto3,enum=v1.Hook_Condition" json:"condition,omitempty"`   // triggering condition of the hook.
@@ -926,6 +927,13 @@ func (*OperationRunHook) Descriptor() ([]byte, []int) {
 	return file_v1_operations_proto_rawDescGZIP(), []int{10}
 }
 
+func (x *OperationRunHook) GetParentOp() int64 {
+	if x != nil {
+		return x.ParentOp
+	}
+	return 0
+}
+
 func (x *OperationRunHook) GetName() string {
 	if x != nil {
 		return x.Name
@@ -1061,36 +1069,38 @@ var file_v1_operations_proto_rawDesc = []byte{
 	0x0e, 0x4f, 0x70, 0x65, 0x72, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x53, 0x74, 0x61, 0x74, 0x73, 0x12,
 	0x23, 0x0a, 0x05, 0x73, 0x74, 0x61, 0x74, 0x73, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x0d,
 	0x2e, 0x76, 0x31, 0x2e, 0x52, 0x65, 0x70, 0x6f, 0x53, 0x74, 0x61, 0x74, 0x73, 0x52, 0x05, 0x73,
-	0x74, 0x61, 0x74, 0x73, 0x22, 0x7d, 0x0a, 0x10, 0x4f, 0x70, 0x65, 0x72, 0x61, 0x74, 0x69, 0x6f,
-	0x6e, 0x52, 0x75, 0x6e, 0x48, 0x6f, 0x6f, 0x6b, 0x12, 0x12, 0x0a, 0x04, 0x6e, 0x61, 0x6d, 0x65,
-	0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x12, 0x23, 0x0a, 0x0d,
-	0x6f, 0x75, 0x74, 0x70, 0x75, 0x74, 0x5f, 0x6c, 0x6f, 0x67, 0x72, 0x65, 0x66, 0x18, 0x02, 0x20,
-	0x01, 0x28, 0x09, 0x52, 0x0c, 0x6f, 0x75, 0x74, 0x70, 0x75, 0x74, 0x4c, 0x6f, 0x67, 0x72, 0x65,
-	0x66, 0x12, 0x30, 0x0a, 0x09, 0x63, 0x6f, 0x6e, 0x64, 0x69, 0x74, 0x69, 0x6f, 0x6e, 0x18, 0x03,
-	0x20, 0x01, 0x28, 0x0e, 0x32, 0x12, 0x2e, 0x76, 0x31, 0x2e, 0x48, 0x6f, 0x6f, 0x6b, 0x2e, 0x43,
-	0x6f, 0x6e, 0x64, 0x69, 0x74, 0x69, 0x6f, 0x6e, 0x52, 0x09, 0x63, 0x6f, 0x6e, 0x64, 0x69, 0x74,
-	0x69, 0x6f, 0x6e, 0x2a, 0x60, 0x0a, 0x12, 0x4f, 0x70, 0x65, 0x72, 0x61, 0x74, 0x69, 0x6f, 0x6e,
-	0x45, 0x76, 0x65, 0x6e, 0x74, 0x54, 0x79, 0x70, 0x65, 0x12, 0x11, 0x0a, 0x0d, 0x45, 0x56, 0x45,
-	0x4e, 0x54, 0x5f, 0x55, 0x4e, 0x4b, 0x4e, 0x4f, 0x57, 0x4e, 0x10, 0x00, 0x12, 0x11, 0x0a, 0x0d,
-	0x45, 0x56, 0x45, 0x4e, 0x54, 0x5f, 0x43, 0x52, 0x45, 0x41, 0x54, 0x45, 0x44, 0x10, 0x01, 0x12,
-	0x11, 0x0a, 0x0d, 0x45, 0x56, 0x45, 0x4e, 0x54, 0x5f, 0x55, 0x50, 0x44, 0x41, 0x54, 0x45, 0x44,
-	0x10, 0x02, 0x12, 0x11, 0x0a, 0x0d, 0x45, 0x56, 0x45, 0x4e, 0x54, 0x5f, 0x44, 0x45, 0x4c, 0x45,
-	0x54, 0x45, 0x44, 0x10, 0x03, 0x2a, 0xc2, 0x01, 0x0a, 0x0f, 0x4f, 0x70, 0x65, 0x72, 0x61, 0x74,
-	0x69, 0x6f, 0x6e, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x12, 0x12, 0x0a, 0x0e, 0x53, 0x54, 0x41,
-	0x54, 0x55, 0x53, 0x5f, 0x55, 0x4e, 0x4b, 0x4e, 0x4f, 0x57, 0x4e, 0x10, 0x00, 0x12, 0x12, 0x0a,
-	0x0e, 0x53, 0x54, 0x41, 0x54, 0x55, 0x53, 0x5f, 0x50, 0x45, 0x4e, 0x44, 0x49, 0x4e, 0x47, 0x10,
-	0x01, 0x12, 0x15, 0x0a, 0x11, 0x53, 0x54, 0x41, 0x54, 0x55, 0x53, 0x5f, 0x49, 0x4e, 0x50, 0x52,
-	0x4f, 0x47, 0x52, 0x45, 0x53, 0x53, 0x10, 0x02, 0x12, 0x12, 0x0a, 0x0e, 0x53, 0x54, 0x41, 0x54,
-	0x55, 0x53, 0x5f, 0x53, 0x55, 0x43, 0x43, 0x45, 0x53, 0x53, 0x10, 0x03, 0x12, 0x12, 0x0a, 0x0e,
-	0x53, 0x54, 0x41, 0x54, 0x55, 0x53, 0x5f, 0x57, 0x41, 0x52, 0x4e, 0x49, 0x4e, 0x47, 0x10, 0x07,
-	0x12, 0x10, 0x0a, 0x0c, 0x53, 0x54, 0x41, 0x54, 0x55, 0x53, 0x5f, 0x45, 0x52, 0x52, 0x4f, 0x52,
-	0x10, 0x04, 0x12, 0x1b, 0x0a, 0x17, 0x53, 0x54, 0x41, 0x54, 0x55, 0x53, 0x5f, 0x53, 0x59, 0x53,
-	0x54, 0x45, 0x4d, 0x5f, 0x43, 0x41, 0x4e, 0x43, 0x45, 0x4c, 0x4c, 0x45, 0x44, 0x10, 0x05, 0x12,
-	0x19, 0x0a, 0x15, 0x53, 0x54, 0x41, 0x54, 0x55, 0x53, 0x5f, 0x55, 0x53, 0x45, 0x52, 0x5f, 0x43,
-	0x41, 0x4e, 0x43, 0x45, 0x4c, 0x4c, 0x45, 0x44, 0x10, 0x06, 0x42, 0x2c, 0x5a, 0x2a, 0x67, 0x69,
-	0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x67, 0x61, 0x72, 0x65, 0x74, 0x68, 0x67,
-	0x65, 0x6f, 0x72, 0x67, 0x65, 0x2f, 0x62, 0x61, 0x63, 0x6b, 0x72, 0x65, 0x73, 0x74, 0x2f, 0x67,
-	0x65, 0x6e, 0x2f, 0x67, 0x6f, 0x2f, 0x76, 0x31, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33,
+	0x74, 0x61, 0x74, 0x73, 0x22, 0x9a, 0x01, 0x0a, 0x10, 0x4f, 0x70, 0x65, 0x72, 0x61, 0x74, 0x69,
+	0x6f, 0x6e, 0x52, 0x75, 0x6e, 0x48, 0x6f, 0x6f, 0x6b, 0x12, 0x1b, 0x0a, 0x09, 0x70, 0x61, 0x72,
+	0x65, 0x6e, 0x74, 0x5f, 0x6f, 0x70, 0x18, 0x04, 0x20, 0x01, 0x28, 0x03, 0x52, 0x08, 0x70, 0x61,
+	0x72, 0x65, 0x6e, 0x74, 0x4f, 0x70, 0x12, 0x12, 0x0a, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x01,
+	0x20, 0x01, 0x28, 0x09, 0x52, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x12, 0x23, 0x0a, 0x0d, 0x6f, 0x75,
+	0x74, 0x70, 0x75, 0x74, 0x5f, 0x6c, 0x6f, 0x67, 0x72, 0x65, 0x66, 0x18, 0x02, 0x20, 0x01, 0x28,
+	0x09, 0x52, 0x0c, 0x6f, 0x75, 0x74, 0x70, 0x75, 0x74, 0x4c, 0x6f, 0x67, 0x72, 0x65, 0x66, 0x12,
+	0x30, 0x0a, 0x09, 0x63, 0x6f, 0x6e, 0x64, 0x69, 0x74, 0x69, 0x6f, 0x6e, 0x18, 0x03, 0x20, 0x01,
+	0x28, 0x0e, 0x32, 0x12, 0x2e, 0x76, 0x31, 0x2e, 0x48, 0x6f, 0x6f, 0x6b, 0x2e, 0x43, 0x6f, 0x6e,
+	0x64, 0x69, 0x74, 0x69, 0x6f, 0x6e, 0x52, 0x09, 0x63, 0x6f, 0x6e, 0x64, 0x69, 0x74, 0x69, 0x6f,
+	0x6e, 0x2a, 0x60, 0x0a, 0x12, 0x4f, 0x70, 0x65, 0x72, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x45, 0x76,
+	0x65, 0x6e, 0x74, 0x54, 0x79, 0x70, 0x65, 0x12, 0x11, 0x0a, 0x0d, 0x45, 0x56, 0x45, 0x4e, 0x54,
+	0x5f, 0x55, 0x4e, 0x4b, 0x4e, 0x4f, 0x57, 0x4e, 0x10, 0x00, 0x12, 0x11, 0x0a, 0x0d, 0x45, 0x56,
+	0x45, 0x4e, 0x54, 0x5f, 0x43, 0x52, 0x45, 0x41, 0x54, 0x45, 0x44, 0x10, 0x01, 0x12, 0x11, 0x0a,
+	0x0d, 0x45, 0x56, 0x45, 0x4e, 0x54, 0x5f, 0x55, 0x50, 0x44, 0x41, 0x54, 0x45, 0x44, 0x10, 0x02,
+	0x12, 0x11, 0x0a, 0x0d, 0x45, 0x56, 0x45, 0x4e, 0x54, 0x5f, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45,
+	0x44, 0x10, 0x03, 0x2a, 0xc2, 0x01, 0x0a, 0x0f, 0x4f, 0x70, 0x65, 0x72, 0x61, 0x74, 0x69, 0x6f,
+	0x6e, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x12, 0x12, 0x0a, 0x0e, 0x53, 0x54, 0x41, 0x54, 0x55,
+	0x53, 0x5f, 0x55, 0x4e, 0x4b, 0x4e, 0x4f, 0x57, 0x4e, 0x10, 0x00, 0x12, 0x12, 0x0a, 0x0e, 0x53,
+	0x54, 0x41, 0x54, 0x55, 0x53, 0x5f, 0x50, 0x45, 0x4e, 0x44, 0x49, 0x4e, 0x47, 0x10, 0x01, 0x12,
+	0x15, 0x0a, 0x11, 0x53, 0x54, 0x41, 0x54, 0x55, 0x53, 0x5f, 0x49, 0x4e, 0x50, 0x52, 0x4f, 0x47,
+	0x52, 0x45, 0x53, 0x53, 0x10, 0x02, 0x12, 0x12, 0x0a, 0x0e, 0x53, 0x54, 0x41, 0x54, 0x55, 0x53,
+	0x5f, 0x53, 0x55, 0x43, 0x43, 0x45, 0x53, 0x53, 0x10, 0x03, 0x12, 0x12, 0x0a, 0x0e, 0x53, 0x54,
+	0x41, 0x54, 0x55, 0x53, 0x5f, 0x57, 0x41, 0x52, 0x4e, 0x49, 0x4e, 0x47, 0x10, 0x07, 0x12, 0x10,
+	0x0a, 0x0c, 0x53, 0x54, 0x41, 0x54, 0x55, 0x53, 0x5f, 0x45, 0x52, 0x52, 0x4f, 0x52, 0x10, 0x04,
+	0x12, 0x1b, 0x0a, 0x17, 0x53, 0x54, 0x41, 0x54, 0x55, 0x53, 0x5f, 0x53, 0x59, 0x53, 0x54, 0x45,
+	0x4d, 0x5f, 0x43, 0x41, 0x4e, 0x43, 0x45, 0x4c, 0x4c, 0x45, 0x44, 0x10, 0x05, 0x12, 0x19, 0x0a,
+	0x15, 0x53, 0x54, 0x41, 0x54, 0x55, 0x53, 0x5f, 0x55, 0x53, 0x45, 0x52, 0x5f, 0x43, 0x41, 0x4e,
+	0x43, 0x45, 0x4c, 0x4c, 0x45, 0x44, 0x10, 0x06, 0x42, 0x2c, 0x5a, 0x2a, 0x67, 0x69, 0x74, 0x68,
+	0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x67, 0x61, 0x72, 0x65, 0x74, 0x68, 0x67, 0x65, 0x6f,
+	0x72, 0x67, 0x65, 0x2f, 0x62, 0x61, 0x63, 0x6b, 0x72, 0x65, 0x73, 0x74, 0x2f, 0x67, 0x65, 0x6e,
+	0x2f, 0x67, 0x6f, 0x2f, 0x76, 0x31, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33,
 }
 
 var (
diff --git a/internal/hook/hook.go b/internal/hook/hook.go
index 015591cc..827801b3 100644
--- a/internal/hook/hook.go
+++ b/internal/hook/hook.go
@@ -14,7 +14,7 @@ import (
 	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
 )
 
-func TasksTriggeredByEvent(config *v1.Config, repoID string, planID string, flowID int64, events []v1.Hook_Condition, vars interface{}) ([]tasks.Task, error) {
+func TasksTriggeredByEvent(config *v1.Config, repoID string, planID string, parentOp *v1.Operation, events []v1.Hook_Condition, vars interface{}) ([]tasks.Task, error) {
 	var taskSet []tasks.Task
 
 	repo := cfg.FindRepo(config, repoID)
@@ -33,7 +33,7 @@ func TasksTriggeredByEvent(config *v1.Config, repoID string, planID string, flow
 		}
 
 		name := fmt.Sprintf("repo/%v/hook/%v", repo.Id, idx)
-		taskSet = append(taskSet, newOneoffRunHookTask(name, config.Instance, repoID, planID, flowID, time.Now(), hook, event, vars))
+		taskSet = append(taskSet, newOneoffRunHookTask(name, config.Instance, repoID, planID, parentOp, time.Now(), hook, event, vars))
 	}
 
 	for idx, hook := range plan.GetHooks() {
@@ -43,13 +43,13 @@ func TasksTriggeredByEvent(config *v1.Config, repoID string, planID string, flow
 		}
 
 		name := fmt.Sprintf("plan/%v/hook/%v", plan.Id, idx)
-		taskSet = append(taskSet, newOneoffRunHookTask(name, config.Instance, repoID, planID, flowID, time.Now(), hook, event, vars))
+		taskSet = append(taskSet, newOneoffRunHookTask(name, config.Instance, repoID, planID, parentOp, time.Now(), hook, event, vars))
 	}
 
 	return taskSet, nil
 }
 
-func newOneoffRunHookTask(title, instanceID, repoID, planID string, flowID int64, at time.Time, hook *v1.Hook, event v1.Hook_Condition, vars interface{}) tasks.Task {
+func newOneoffRunHookTask(title, instanceID, repoID, planID string, parentOp *v1.Operation, at time.Time, hook *v1.Hook, event v1.Hook_Condition, vars interface{}) tasks.Task {
 	return &tasks.GenericOneoffTask{
 		OneoffTask: tasks.OneoffTask{
 			BaseTask: tasks.BaseTask{
@@ -57,18 +57,20 @@ func newOneoffRunHookTask(title, instanceID, repoID, planID string, flowID int64
 				TaskRepoID: repoID,
 				TaskPlanID: planID,
 			},
-			FlowID: flowID,
+			FlowID: parentOp.GetFlowId(),
 			RunAt:  at,
 			ProtoOp: &v1.Operation{
 				InstanceId: instanceID,
 				RepoId:     repoID,
 				PlanId:     planID,
+				FlowId:     parentOp.GetFlowId(),
 
 				DisplayMessage: fmt.Sprintf("running %v triggered by %v", title, event.String()),
 				Op: &v1.Operation_OperationRunHook{
 					OperationRunHook: &v1.OperationRunHook{
 						Name:      title,
 						Condition: event,
+						ParentOp:  parentOp.GetId(),
 					},
 				},
 			},
diff --git a/internal/orchestrator/taskrunnerimpl.go b/internal/orchestrator/taskrunnerimpl.go
index 00015d58..7cd6f621 100644
--- a/internal/orchestrator/taskrunnerimpl.go
+++ b/internal/orchestrator/taskrunnerimpl.go
@@ -96,14 +96,8 @@ func (t *taskRunnerImpl) ExecuteHooks(ctx context.Context, events []v1.Hook_Cond
 		plan, _ = t.findPlan()
 		vars.Plan = plan
 	}
-	var flowID int64
-	if t.op != nil {
-		// a hook always belongs to a flow associated with the ID of the triggering task.
-		// this forms a tree.
-		flowID = t.op.Id
-	}
 
-	hookTasks, err := hook.TasksTriggeredByEvent(t.Config(), repoID, planID, flowID, events, vars)
+	hookTasks, err := hook.TasksTriggeredByEvent(t.Config(), repoID, planID, t.op, events, vars)
 	if err != nil {
 		return err
 	}
diff --git a/proto/v1/operations.proto b/proto/v1/operations.proto
index aaa84dda..01a8e62b 100644
--- a/proto/v1/operations.proto
+++ b/proto/v1/operations.proto
@@ -24,7 +24,7 @@ message Operation {
   OperationStatus status = 4;
   // required, unix time in milliseconds of the operation's creation (ID is derived from this)
   int64 unix_time_start_ms = 5; 
-  // optional, unix time in milliseconds of the operation's completion
+  // ptional, unix time in milliseconds of the operation's completion
   int64 unix_time_end_ms = 6;
   // optional, human readable context message, typically an error message.
   string display_message = 7;
@@ -40,7 +40,7 @@ message Operation {
     OperationStats operation_stats = 105;
     OperationRunHook operation_run_hook = 106;
     OperationCheck operation_check = 107;
-  }
+  } 
 }
 
 // OperationEvent is used in the wireformat to stream operation changes to clients
@@ -110,6 +110,7 @@ message OperationStats {
 
 // OperationRunHook tracks a hook that was run.
 message OperationRunHook {
+  int64 parent_op = 4; // ID of the operation that ran the hook.
   string name = 1; // description of the hook that was run. typically repo/hook_idx or plan/hook_idx.
   string output_logref = 2; // logref of the hook's output. DEPRECATED.
   Hook.Condition condition = 3; // triggering condition of the hook.
diff --git a/webui/gen/ts/v1/operations_pb.ts b/webui/gen/ts/v1/operations_pb.ts
index 0b8782dd..fc0ec563 100644
--- a/webui/gen/ts/v1/operations_pb.ts
+++ b/webui/gen/ts/v1/operations_pb.ts
@@ -206,7 +206,7 @@ export class Operation extends Message<Operation> {
   unixTimeStartMs = protoInt64.zero;
 
   /**
-   * optional, unix time in milliseconds of the operation's completion
+   * ptional, unix time in milliseconds of the operation's completion
    *
    * @generated from field: int64 unix_time_end_ms = 6;
    */
@@ -699,6 +699,13 @@ export class OperationStats extends Message<OperationStats> {
  * @generated from message v1.OperationRunHook
  */
 export class OperationRunHook extends Message<OperationRunHook> {
+  /**
+   * ID of the operation that ran the hook.
+   *
+   * @generated from field: int64 parent_op = 4;
+   */
+  parentOp = protoInt64.zero;
+
   /**
    * description of the hook that was run. typically repo/hook_idx or plan/hook_idx.
    *
@@ -728,6 +735,7 @@ export class OperationRunHook extends Message<OperationRunHook> {
   static readonly runtime: typeof proto3 = proto3;
   static readonly typeName = "v1.OperationRunHook";
   static readonly fields: FieldList = proto3.util.newFieldList(() => [
+    { no: 4, name: "parent_op", kind: "scalar", T: 3 /* ScalarType.INT64 */ },
     { no: 1, name: "name", kind: "scalar", T: 9 /* ScalarType.STRING */ },
     { no: 2, name: "output_logref", kind: "scalar", T: 9 /* ScalarType.STRING */ },
     { no: 3, name: "condition", kind: "enum", T: proto3.getEnumType(Hook_Condition) },
diff --git a/webui/src/components/OperationList.tsx b/webui/src/components/OperationList.tsx
index b22bc396..0c0be7d2 100644
--- a/webui/src/components/OperationList.tsx
+++ b/webui/src/components/OperationList.tsx
@@ -24,10 +24,12 @@ import { OperationRow } from "./OperationRow";
 export const OperationList = ({
   req,
   useBackups,
+  useOperations,
   showPlan,
 }: React.PropsWithoutRef<{
   req?: GetOperationsRequest;
-  useBackups?: BackupInfo[];
+  useBackups?: BackupInfo[]; // a backup to display; some operations will be filtered out e.g. hook executions.
+  useOperations?: Operation[]; // exact set of operations to display; no filtering will be applied.
   showPlan?: boolean;
 }>) => {
   const alertApi = useAlertApi();
@@ -64,7 +66,7 @@ export const OperationList = ({
     backups = [...(useBackups || [])];
   }
 
-  if (backups.length === 0) {
+  if (backups.length === 0 && !useOperations) {
     return (
       <Empty
         description="No operations yet."
@@ -73,7 +75,25 @@ export const OperationList = ({
     );
   }
 
-  let operations = backups.flatMap((b) => b.operations);
+  const hookExecutionsForOperation: Map<BigInt, Operation[]> = new Map();
+  let operations: Operation[] = [];
+  if (useOperations) {
+    operations = useOperations;
+  } else {
+    operations = backups
+      .flatMap((b) => b.operations)
+      .filter((op) => {
+        if (op.op.case === "operationRunHook") {
+          const parentOp = op.op.value.parentOp;
+          if (!hookExecutionsForOperation.has(parentOp)) {
+            hookExecutionsForOperation.set(parentOp, []);
+          }
+          hookExecutionsForOperation.get(parentOp)!.push(op);
+          return false;
+        }
+        return true;
+      });
+  }
   operations.sort((a, b) => {
     return Number(b.unixTimeStartMs - a.unixTimeStartMs);
   });
@@ -82,13 +102,14 @@ export const OperationList = ({
       itemLayout="horizontal"
       size="small"
       dataSource={operations}
-      renderItem={(op, index) => {
+      renderItem={(op) => {
         return (
           <OperationRow
             alertApi={alertApi!}
             key={op.id}
             operation={op}
             showPlan={showPlan || false}
+            hookOperations={hookExecutionsForOperation.get(op.id)}
           />
         );
       }}
diff --git a/webui/src/components/OperationRow.tsx b/webui/src/components/OperationRow.tsx
index 09de251c..e4c299e8 100644
--- a/webui/src/components/OperationRow.tsx
+++ b/webui/src/components/OperationRow.tsx
@@ -1,23 +1,21 @@
 import React, { useEffect, useState } from "react";
 import {
   Operation,
-  OperationEvent,
-  OperationEventType,
   OperationForget,
-  OperationRunHook,
+  OperationRestore,
   OperationStatus,
 } from "../../gen/ts/v1/operations_pb";
 import {
   Button,
   Col,
   Collapse,
-  Empty,
   List,
   Modal,
   Progress,
   Row,
   Typography,
 } from "antd";
+import type { ItemType } from "rc-collapse/es/interface";
 import {
   PaperClipOutlined,
   SaveOutlined,
@@ -47,15 +45,19 @@ import { backrestService } from "../api";
 import { useShowModal } from "./ModalManager";
 import { proto3 } from "@bufbuild/protobuf";
 import { Hook_Condition } from "../../gen/ts/v1/config_pb";
+import { useAlertApi } from "./Alerts";
+import { OperationList } from "./OperationList";
 
 export const OperationRow = ({
   operation,
   alertApi,
   showPlan,
+  hookOperations,
 }: React.PropsWithoutRef<{
   operation: Operation;
   alertApi?: MessageInstance;
-  showPlan: boolean;
+  showPlan?: boolean;
+  hookOperations?: Operation[];
 }>) => {
   const showModal = useShowModal();
   const details = detailsForOperation(operation);
@@ -108,6 +110,38 @@ export const OperationRow = ({
       break;
   }
 
+  const doCancel = () => {
+    backrestService
+      .cancel({ value: operation.id! })
+      .then(() => {
+        alertApi?.success("Requested to cancel operation");
+      })
+      .catch((e) => {
+        alertApi?.error("Failed to cancel operation: " + e.message);
+      });
+  };
+
+  const doShowLogs = () => {
+    showModal(
+      <Modal
+        width="70%"
+        title={
+          "Logs for operation " +
+          opName +
+          " at " +
+          formatTime(Number(operation.unixTimeStartMs))
+        }
+        open={true}
+        footer={null}
+        onCancel={() => {
+          showModal(null);
+        }}
+      >
+        <BigOperationDataVerbatim logref={operation.logref!} />
+      </Modal>
+    );
+  };
+
   const opName = displayTypeToString(getTypeForDisplay(operation));
   let title = (
     <>
@@ -125,16 +159,7 @@ export const OperationRow = ({
           type="link"
           size="small"
           className="backrest operation-details"
-          onClick={() => {
-            backrestService
-              .cancel({ value: operation.id! })
-              .then(() => {
-                alertApi?.success("Requested to cancel operation");
-              })
-              .catch((e) => {
-                alertApi?.error("Failed to cancel operation: " + e.message);
-              });
-          }}
+          onClick={doCancel}
         >
           [Cancel Operation]
         </Button>
@@ -151,26 +176,7 @@ export const OperationRow = ({
             type="link"
             size="middle"
             className="backrest operation-details"
-            onClick={() => {
-              showModal(
-                <Modal
-                  width="50%"
-                  title={
-                    "Logs for operation " +
-                    opName +
-                    " at " +
-                    formatTime(Number(operation.unixTimeStartMs))
-                  }
-                  visible={true}
-                  footer={null}
-                  onCancel={() => {
-                    showModal(null);
-                  }}
-                >
-                  <BigOperationDataVerbatim logref={operation.logref!} />
-                </Modal>
-              );
-            }}
+            onClick={doShowLogs}
           >
             [View Logs]
           </Button>
@@ -179,22 +185,23 @@ export const OperationRow = ({
     );
   }
 
-  let body: React.ReactNode | undefined;
   let displayMessage = operation.displayMessage;
 
+  const bodyItems: ItemType[] = [];
+  const expandedBodyItems: string[] = [];
+
   if (operation.op.case === "operationBackup") {
+    expandedBodyItems.push("details");
     const backupOp = operation.op.value;
-    const items: { key: number; label: string; children: React.ReactNode }[] = [
-      {
-        key: 1,
-        label: "Backup Details",
-        children: <BackupOperationStatus status={backupOp.lastStatus} />,
-      },
-    ];
+    bodyItems.push({
+      key: "details",
+      label: "Backup Details",
+      children: <BackupOperationStatus status={backupOp.lastStatus} />,
+    });
 
     if (backupOp.errors.length > 0) {
-      items.splice(0, 0, {
-        key: 2,
+      bodyItems.push({
+        key: "errors",
         label: "Item Errors",
         children: (
           <pre>
@@ -203,193 +210,182 @@ export const OperationRow = ({
         ),
       });
     }
-
-    body = (
-      <>
-        <Collapse
-          size="small"
-          destroyInactivePanel
-          defaultActiveKey={[1]}
-          items={items}
-        />
-      </>
-    );
   } else if (operation.op.case === "operationIndexSnapshot") {
+    expandedBodyItems.push("details");
     const snapshotOp = operation.op.value;
-    body = (
-      <SnapshotInfo
-        snapshot={snapshotOp.snapshot!}
-        repoId={operation.repoId!}
-        planId={operation.planId}
-      />
-    );
+    bodyItems.push({
+      key: "details",
+      label: "Details",
+      children: <SnapshotDetails snapshot={snapshotOp.snapshot!} />,
+    });
+    bodyItems.push({
+      key: "browser",
+      label: "Snapshot Browser",
+      children: (
+        <SnapshotBrowser
+          snapshotId={snapshotOp.snapshot!.id}
+          repoId={operation.repoId}
+          planId={operation.planId}
+        />
+      ),
+    });
   } else if (operation.op.case === "operationForget") {
     const forgetOp = operation.op.value;
-    body = <ForgetOperationDetails forgetOp={forgetOp} />;
+    bodyItems.push({
+      key: "forgot",
+      label: "Removed " + forgetOp.forget?.length + " Snapshots",
+      children: <ForgetOperationDetails forgetOp={forgetOp} />,
+    });
   } else if (operation.op.case === "operationPrune") {
     const prune = operation.op.value;
-    body = (
-      <Collapse
-        size="small"
-        destroyInactivePanel
-        items={[
-          {
-            key: 1,
-            label: "Prune Output",
-            children: <pre>{prune.output}</pre>,
-          },
-        ]}
-      />
-    );
+    bodyItems.push({
+      key: "prune",
+      label: "Prune Output",
+      children: <pre>{prune.output}</pre>,
+    });
   } else if (operation.op.case === "operationCheck") {
     const check = operation.op.value;
-    body = (
-      <Collapse
-        size="small"
-        destroyInactivePanel
-        items={[
-          {
-            key: 1,
-            label: "Check Output",
-            children: <pre>{check.output}</pre>,
-          },
-        ]}
-      />
-    );
+    bodyItems.push({
+      key: "check",
+      label: "Check Output",
+      children: <pre>{check.output}</pre>,
+    });
   } else if (operation.op.case === "operationRestore") {
-    const restore = operation.op.value;
-    const progress = Math.round((details.percentage || 0) * 10) / 10;
-    const st = restore.lastStatus! || {};
-
-    body = (
-      <>
-        Restore {restore.path} to {restore.target}
-        {details.percentage !== undefined ? (
-          <Progress percent={progress} status="active" />
-        ) : null}
-        {operation.status == OperationStatus.STATUS_SUCCESS ? (
-          <>
-            <Button
-              type="link"
-              onClick={() => {
-                backrestService
-                  .getDownloadURL({ value: operation.id })
-                  .then((resp) => {
-                    window.open(resp.value, "_blank");
-                  })
-                  .catch((e) => {
-                    alertApi?.error(
-                      "Failed to fetch download URL: " + e.message
-                    );
-                  });
-              }}
-            >
-              Download File(s)
-            </Button>
-          </>
-        ) : null}
-        <br />
-        Snapshot ID: {normalizeSnapshotId(operation.snapshotId!)}
-        <Row gutter={16}>
-          <Col span={12}>
-            <Typography.Text strong>Bytes Done/Total</Typography.Text>
-            <br />
-            {formatBytes(Number(st.bytesRestored))}/
-            {formatBytes(Number(st.totalBytes))}
-          </Col>
-          <Col span={12}>
-            <Typography.Text strong>Files Done/Total</Typography.Text>
-            <br />
-            {Number(st.filesRestored)}/{Number(st.totalFiles)}
-          </Col>
-        </Row>
-      </>
-    );
+    expandedBodyItems.push("restore");
+    bodyItems.push({
+      key: "restore",
+      label: "Restore Details",
+      children: <RestoreOperationStatus operation={operation} />,
+    });
   } else if (operation.op.case === "operationRunHook") {
     const hook = operation.op.value;
-    const triggeringCondition = proto3
-      .getEnumType(Hook_Condition)
-      .findNumber(hook.condition);
+    // TODO: customized view of hook execution info
   }
 
-  const children = [];
+  if (hookOperations) {
+    bodyItems.push({
+      key: "hookOperations",
+      label: "Hooks Triggered",
+      children: <OperationList useOperations={hookOperations} />,
+    });
 
-  if (operation.displayMessage) {
-    children.push(
-      <div key="message">
-        <pre>
-          {details.state ? details.state + ": " : null}
-          {displayMessage}
-        </pre>
-      </div>
-    );
+    for (const op of hookOperations) {
+      if (op.status !== OperationStatus.STATUS_SUCCESS) {
+        expandedBodyItems.push("hookOperations");
+        break;
+      }
+    }
   }
 
-  children.push(<div key="body">{body}</div>);
-
   return (
     <List.Item key={operation.id}>
-      <List.Item.Meta title={title} avatar={avatar} description={children} />
+      <List.Item.Meta
+        title={title}
+        avatar={avatar}
+        description={
+          <>
+            {operation.displayMessage && (
+              <div key="message">
+                <pre>
+                  {details.state ? details.state + ": " : null}
+                  {displayMessage}
+                </pre>
+              </div>
+            )}
+            <Collapse
+              size="small"
+              destroyInactivePanel={true}
+              items={bodyItems}
+              defaultActiveKey={expandedBodyItems}
+            />
+          </>
+        }
+      />
     </List.Item>
   );
 };
 
-const SnapshotInfo = ({
-  snapshot,
-  repoId,
-  planId,
-}: {
-  snapshot: ResticSnapshot;
-  repoId: string;
-  planId?: string;
-}) => {
+const SnapshotDetails = ({ snapshot }: { snapshot: ResticSnapshot }) => {
   return (
-    <Collapse
-      size="small"
-      defaultActiveKey={[1]}
-      items={[
-        {
-          key: 1,
-          label: "Snapshot Details",
-          children: (
-            <>
-              <Typography.Text>
-                <Typography.Text strong>Snapshot ID: </Typography.Text>
-                {normalizeSnapshotId(snapshot.id!)}
-              </Typography.Text>
-              <Row gutter={16}>
-                <Col span={8}>
-                  <Typography.Text strong>Host</Typography.Text>
-                  <br />
-                  {snapshot.hostname}
-                </Col>
-                <Col span={8}>
-                  <Typography.Text strong>Username</Typography.Text>
-                  <br />
-                  {snapshot.hostname}
-                </Col>
-                <Col span={8}>
-                  <Typography.Text strong>Tags</Typography.Text>
-                  <br />
-                  {snapshot.tags?.join(", ")}
-                </Col>
-              </Row>
-            </>
-          ),
-        },
-        {
-          key: 2,
-          label: "Browse and Restore Files in Backup",
-          children: (
-            <SnapshotBrowser
-              snapshotId={snapshot.id!}
-              repoId={repoId}
-              planId={planId}
-            />
-          ),
-        },
-      ]}
-    />
+    <>
+      <Typography.Text>
+        <Typography.Text strong>Snapshot ID: </Typography.Text>
+        {normalizeSnapshotId(snapshot.id!)}
+      </Typography.Text>
+      <Row gutter={16}>
+        <Col span={8}>
+          <Typography.Text strong>Host</Typography.Text>
+          <br />
+          {snapshot.hostname}
+        </Col>
+        <Col span={8}>
+          <Typography.Text strong>Username</Typography.Text>
+          <br />
+          {snapshot.hostname}
+        </Col>
+        <Col span={8}>
+          <Typography.Text strong>Tags</Typography.Text>
+          <br />
+          {snapshot.tags?.join(", ")}
+        </Col>
+      </Row>
+    </>
+  );
+};
+
+const RestoreOperationStatus = ({ operation }: { operation: Operation }) => {
+  const restoreOp = operation.op.value as OperationRestore;
+  const isDone = restoreOp.lastStatus?.messageType === "summary";
+  const progress = restoreOp.lastStatus?.percentDone || 0;
+  const alertApi = useAlertApi();
+  const lastStatus = restoreOp.lastStatus;
+
+  return (
+    <>
+      Restore {restoreOp.path} to {restoreOp.target}
+      {!isDone ? (
+        <Progress
+          percent={Math.round(progress * 1000) / 1000}
+          status="active"
+        />
+      ) : null}
+      {operation.status == OperationStatus.STATUS_SUCCESS ? (
+        <>
+          <Button
+            type="link"
+            onClick={() => {
+              backrestService
+                .getDownloadURL({ value: operation.id })
+                .then((resp) => {
+                  window.open(resp.value, "_blank");
+                })
+                .catch((e) => {
+                  alertApi?.error("Failed to fetch download URL: " + e.message);
+                });
+            }}
+          >
+            Download File(s)
+          </Button>
+        </>
+      ) : null}
+      <br />
+      Restored Snapshot ID: {normalizeSnapshotId(operation.snapshotId!)}
+      {lastStatus && (
+        <Row gutter={16}>
+          <Col span={12}>
+            <Typography.Text strong>Bytes Done/Total</Typography.Text>
+            <br />
+            {formatBytes(Number(lastStatus.bytesRestored))}/
+            {formatBytes(Number(lastStatus.totalBytes))}
+          </Col>
+          <Col span={12}>
+            <Typography.Text strong>Files Done/Total</Typography.Text>
+            <br />
+            {Number(lastStatus.filesRestored)}/{Number(lastStatus.totalFiles)}
+          </Col>
+        </Row>
+      )}
+    </>
   );
 };
 
@@ -507,38 +503,26 @@ const ForgetOperationDetails = ({
   }
 
   return (
-    <Collapse
-      size="small"
-      destroyInactivePanel
-      items={[
-        {
-          key: 1,
-          label: "Removed " + forgetOp.forget?.length + " Snapshots",
-          children: (
-            <>
-              Removed snapshots:
-              <pre>
-                {forgetOp.forget?.map((f) => (
-                  <div key={f.id}>
-                    {"removed snapshot " +
-                      normalizeSnapshotId(f.id!) +
-                      " taken at " +
-                      formatTime(Number(f.unixTimeMs))}{" "}
-                    <br />
-                  </div>
-                ))}
-              </pre>
-              {/* Policy:
+    <>
+      Removed snapshots:
+      <pre>
+        {forgetOp.forget?.map((f) => (
+          <div key={f.id}>
+            {"removed snapshot " +
+              normalizeSnapshotId(f.id!) +
+              " taken at " +
+              formatTime(Number(f.unixTimeMs))}{" "}
+            <br />
+          </div>
+        ))}
+      </pre>
+      {/* Policy:
             <ul>
               {policyDesc.map((desc, idx) => (
                 <li key={idx}>{desc}</li>
               ))}
             </ul> */}
-            </>
-          ),
-        },
-      ]}
-    />
+    </>
   );
 };
 
diff --git a/webui/src/components/OperationTree.tsx b/webui/src/components/OperationTree.tsx
index 973023b9..f487163b 100644
--- a/webui/src/components/OperationTree.tsx
+++ b/webui/src/components/OperationTree.tsx
@@ -151,7 +151,7 @@ export const OperationTree = ({
               );
             } else if (b.backupLastStatus.entry.case === "status") {
               const s = b.backupLastStatus.entry.value;
-              const percent = Number(s.bytesDone / s.totalBytes) * 100;
+              const percent = Number(s.percentDone) * 100;
               details.push(
                 `${percent.toFixed(1)}% processed ${formatBytes(
                   Number(s.bytesDone)
diff --git a/webui/src/state/oplog.ts b/webui/src/state/oplog.ts
index 93ff6e21..28707362 100644
--- a/webui/src/state/oplog.ts
+++ b/webui/src/state/oplog.ts
@@ -192,17 +192,10 @@ export class BackupInfoCollector {
     });
 
     // use the lowest ID of all operations as the ID of the backup, this will be the first created operation.
-    const id = operations.reduce((prev, curr) => {
-      return prev < curr.id ? prev : curr.id;
-    }, operations[0].id!);
-
     const startTimeMs = Number(operations[0].unixTimeStartMs);
     const endTimeMs = Number(operations[operations.length - 1].unixTimeEndMs!);
     const displayTime = new Date(startTimeMs);
-    let displayType = DisplayType.SNAPSHOT;
-    if (operations.length === 1) {
-      displayType = getTypeForDisplay(operations[0]);
-    }
+    const displayType = getTypeForDisplay(operations[0]);
 
     // use the latest status that is not a hidden status
     let statusIdx = operations.length - 1;
@@ -365,8 +358,6 @@ export class BackupInfoCollector {
 export const shouldHideOperation = (operation: Operation) => {
   return (
     operation.op.case === "operationStats" ||
-    (operation.op.case === "operationRunHook" &&
-      operation.status === OperationStatus.STATUS_SUCCESS) ||
     shouldHideStatus(operation.status)
   );
 };

From d9e6dde17310c934997d42af808afa4f14057d03 Mon Sep 17 00:00:00 2001
From: garethgeorge <garethgeorge97@gmail.com>
Date: Thu, 11 Jul 2024 01:00:26 -0700
Subject: [PATCH 12/15] set an appropriate default shell for windows

---
 internal/hook/types/command.go | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/internal/hook/types/command.go b/internal/hook/types/command.go
index ba6b374e..0153b551 100644
--- a/internal/hook/types/command.go
+++ b/internal/hook/types/command.go
@@ -2,9 +2,11 @@ package types
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os/exec"
 	"reflect"
+	"runtime"
 	"strings"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
@@ -12,6 +14,7 @@ import (
 	"github.com/garethgeorge/backrest/internal/ioutil"
 	"github.com/garethgeorge/backrest/internal/orchestrator/logging"
 	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
+	"github.com/google/shlex"
 )
 
 type commandHandler struct{}
@@ -25,24 +28,33 @@ func (commandHandler) Execute(ctx context.Context, h *v1.Hook, vars interface{},
 	writer := logging.WriterFromContext(ctx)
 
 	// Parse out the shell to use if a #! prefix is present
-	shell := "sh"
+	shell := []string{"sh"}
+	if runtime.GOOS == "windows" {
+		shell = []string{"powershell", "-nologo", "-noprofile"}
+	}
+
 	if len(command) > 2 && command[0:2] == "#!" {
 		nextLine := strings.Index(command, "\n")
 		if nextLine == -1 {
 			nextLine = len(command)
 		}
-		shell = strings.Trim(command[2:nextLine], " ")
+		shell, err = shlex.Split(strings.Trim(command[2:nextLine], " "))
+		if err != nil {
+			return fmt.Errorf("parsing shell for command: %w", err)
+		} else if len(shell) == 0 {
+			return errors.New("must specify shell for command")
+		}
 		command = command[nextLine+1:]
 	}
 
 	scriptWriter := &ioutil.LinePrefixer{W: writer, Prefix: []byte("[script] ")}
-	defer scriptWriter.Close()
+	fmt.Fprintf(scriptWriter, "%v\n%v\n", shell, command)
+	scriptWriter.Close()
 	outputWriter := &ioutil.LinePrefixer{W: writer, Prefix: []byte("[output] ")}
 	defer outputWriter.Close()
-	fmt.Fprintf(scriptWriter, "------- script -------\n#! %v\n%v\n", shell, command)
 
 	// Run the command in the specified shell
-	execCmd := exec.Command(shell)
+	execCmd := exec.Command(shell[0], shell[1:]...)
 	execCmd.Stdin = strings.NewReader(command)
 
 	stdout := &ioutil.SynchronizedWriter{W: outputWriter}

From ef4da629325ea23fc97405b3e999ac4c00c4892e Mon Sep 17 00:00:00 2001
From: garethgeorge <garethgeorge97@gmail.com>
Date: Thu, 11 Jul 2024 01:02:58 -0700
Subject: [PATCH 13/15] fix cancellation behavior

---
 internal/hook/hook_test.go              | 49 ++++++++++++-------------
 internal/orchestrator/orchestrator.go   |  2 +-
 internal/orchestrator/taskrunnerimpl.go |  2 +-
 3 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/internal/hook/hook_test.go b/internal/hook/hook_test.go
index 3f467578..e1615b83 100644
--- a/internal/hook/hook_test.go
+++ b/internal/hook/hook_test.go
@@ -2,7 +2,6 @@ package hook
 
 import (
 	"bytes"
-	"errors"
 	"os/exec"
 	"runtime"
 	"testing"
@@ -58,29 +57,29 @@ exit $counter`,
 	}
 }
 
-func TestCommandHookErrorHandling(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("skipping test on windows")
-	}
+// func TestCommandHookErrorHandling(t *testing.T) {
+// 	if runtime.GOOS == "windows" {
+// 		t.Skip("skipping test on windows")
+// 	}
 
-	hook := Hook(v1.Hook{
-		Conditions: []v1.Hook_Condition{
-			v1.Hook_CONDITION_SNAPSHOT_START,
-		},
-		Action: &v1.Hook_ActionCommand{
-			ActionCommand: &v1.Hook_Command{
-				Command: "exit 1",
-			},
-		},
-		OnError: v1.Hook_ON_ERROR_CANCEL,
-	})
+// 	hook := Hook(v1.Hook{
+// 		Conditions: []v1.Hook_Condition{
+// 			v1.Hook_CONDITION_SNAPSHOT_START,
+// 		},
+// 		Action: &v1.Hook_ActionCommand{
+// 			ActionCommand: &v1.Hook_Command{
+// 				Command: "exit 1",
+// 			},
+// 		},
+// 		OnError: v1.Hook_ON_ERROR_CANCEL,
+// 	})
 
-	err := applyHookErrorPolicy(hook.OnError, hook.Do(v1.Hook_CONDITION_SNAPSHOT_START, struct{}{}, &bytes.Buffer{}))
-	if err == nil {
-		t.Fatal("expected error")
-	}
-	var cancelErr *HookErrorRequestCancel
-	if !errors.As(err, &cancelErr) {
-		t.Fatalf("expected HookErrorRequestCancel, got %v", err)
-	}
-}
+// 	err := applyHookErrorPolicy(hook.OnError, hook.Do(v1.Hook_CONDITION_SNAPSHOT_START, struct{}{}, &bytes.Buffer{}))
+// 	if err == nil {
+// 		t.Fatal("expected error")
+// 	}
+// 	var cancelErr *HookErrorRequestCancel
+// 	if !errors.As(err, &cancelErr) {
+// 		t.Fatalf("expected HookErrorRequestCancel, got %v", err)
+// 	}
+// }
diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go
index 7ef36d92..30cfe6b3 100644
--- a/internal/orchestrator/orchestrator.go
+++ b/internal/orchestrator/orchestrator.go
@@ -369,7 +369,7 @@ func (o *Orchestrator) RunTask(ctx context.Context, st tasks.ScheduledTask) erro
 				op.Status = v1.OperationStatus_STATUS_ERROR
 			}
 
-			// prepend the error to the display message
+			// prepend the error to the display
 			if op.DisplayMessage != "" {
 				op.DisplayMessage = err.Error() + "\n\n" + op.DisplayMessage
 			} else {
diff --git a/internal/orchestrator/taskrunnerimpl.go b/internal/orchestrator/taskrunnerimpl.go
index 7cd6f621..0a8e3b3f 100644
--- a/internal/orchestrator/taskrunnerimpl.go
+++ b/internal/orchestrator/taskrunnerimpl.go
@@ -109,7 +109,7 @@ func (t *taskRunnerImpl) ExecuteHooks(ctx context.Context, events []v1.Hook_Cond
 			return fmt.Errorf("%v: %w", task.Name(), err)
 		}
 		if err := t.orchestrator.RunTask(ctx, st); hook.IsHaltingError(err) {
-			var cancelErr hook.HookErrorRequestCancel
+			var cancelErr *hook.HookErrorRequestCancel
 			if errors.As(err, &cancelErr) {
 				return fmt.Errorf("%w: %w", tasks.ErrTaskCancelled, err)
 			}

From 1ad385f5a3a25f971ebb7239a42309a98008381b Mon Sep 17 00:00:00 2001
From: garethgeorge <garethgeorge97@gmail.com>
Date: Thu, 11 Jul 2024 01:14:17 -0700
Subject: [PATCH 14/15] add new hook tests

---
 internal/api/backresthandler_test.go | 92 ++++++++++++++++++++++++++++
 internal/hook/types/command.go       |  4 ++
 internal/hook/types/discord.go       |  4 ++
 internal/hook/types/gotify.go        |  4 ++
 internal/hook/types/registry.go      |  1 +
 internal/hook/types/shoutrrr.go      |  4 ++
 internal/hook/types/slack.go         |  4 ++
 7 files changed, 113 insertions(+)

diff --git a/internal/api/backresthandler_test.go b/internal/api/backresthandler_test.go
index ed72ce99..1fe3bf3e 100644
--- a/internal/api/backresthandler_test.go
+++ b/internal/api/backresthandler_test.go
@@ -334,6 +334,98 @@ func TestHookExecution(t *testing.T) {
 	}
 }
 
+func TestHookCancellation(t *testing.T) {
+	t.Parallel()
+
+	if runtime.GOOS == "windows" {
+		t.Skip("skipping test on windows")
+	}
+
+	sut := createSystemUnderTest(t, &config.MemoryStore{
+		Config: &v1.Config{
+			Modno:    1234,
+			Instance: "test",
+			Repos: []*v1.Repo{
+				{
+					Id:       "local",
+					Uri:      t.TempDir(),
+					Password: "test",
+				},
+			},
+			Plans: []*v1.Plan{
+				{
+					Id:   "test",
+					Repo: "local",
+					Paths: []string{
+						t.TempDir(),
+					},
+					Schedule: &v1.Schedule{
+						Schedule: &v1.Schedule_Disabled{Disabled: true},
+					},
+					Hooks: []*v1.Hook{
+						{
+							Conditions: []v1.Hook_Condition{
+								v1.Hook_CONDITION_SNAPSHOT_START,
+							},
+							Action: &v1.Hook_ActionCommand{
+								ActionCommand: &v1.Hook_Command{
+									Command: "exit 123",
+								},
+							},
+							OnError: v1.Hook_ON_ERROR_CANCEL,
+						},
+					},
+				},
+			},
+		},
+	})
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	go func() {
+		sut.orch.Run(ctx)
+	}()
+
+	_, err := sut.handler.Backup(context.Background(), connect.NewRequest(&types.StringValue{Value: "test"}))
+	if err != nil {
+		t.Fatalf("Backup() error = %v", err)
+	}
+
+	// Wait for a hook operation to appear in the oplog
+	if err := retry(t, 10, 2*time.Second, func() error {
+		hookOps := slices.DeleteFunc(getOperations(t, sut.oplog), func(op *v1.Operation) bool {
+			_, ok := op.GetOp().(*v1.Operation_OperationRunHook)
+			return !ok
+		})
+		if len(hookOps) != 1 {
+			return fmt.Errorf("expected 1 hook operations, got %d", len(hookOps))
+		}
+		if hookOps[0].Status != v1.OperationStatus_STATUS_ERROR {
+			return fmt.Errorf("expected hook operation error status, got %v", hookOps[0].Status)
+		}
+		return nil
+	}); err != nil {
+		t.Fatalf("Couldn't find hooks in oplog: %v", err)
+	}
+
+	// assert that the backup operation is in the log and is cancelled
+	if err := retry(t, 10, 2*time.Second, func() error {
+		backupOps := slices.DeleteFunc(getOperations(t, sut.oplog), func(op *v1.Operation) bool {
+			_, ok := op.GetOp().(*v1.Operation_OperationBackup)
+			return !ok
+		})
+		if len(backupOps) != 1 {
+			return fmt.Errorf("expected 1 backup operation, got %d", len(backupOps))
+		}
+		if backupOps[0].Status != v1.OperationStatus_STATUS_USER_CANCELLED {
+			return fmt.Errorf("expected backup operation cancelled status, got %v", backupOps[0].Status)
+		}
+		return nil
+	}); err != nil {
+		t.Fatalf("Couldn't find hooks in oplog: %v", err)
+	}
+}
+
 func TestCancelBackup(t *testing.T) {
 	t.Parallel()
 
diff --git a/internal/hook/types/command.go b/internal/hook/types/command.go
index 0153b551..6db05533 100644
--- a/internal/hook/types/command.go
+++ b/internal/hook/types/command.go
@@ -19,6 +19,10 @@ import (
 
 type commandHandler struct{}
 
+func (commandHandler) Name() string {
+	return "command"
+}
+
 func (commandHandler) Execute(ctx context.Context, h *v1.Hook, vars interface{}, runner tasks.TaskRunner) error {
 	command, err := hookutil.RenderTemplate(h.GetActionCommand().GetCommand(), vars)
 	if err != nil {
diff --git a/internal/hook/types/discord.go b/internal/hook/types/discord.go
index f1677d54..1c755ff4 100644
--- a/internal/hook/types/discord.go
+++ b/internal/hook/types/discord.go
@@ -14,6 +14,10 @@ import (
 
 type discordHandler struct{}
 
+func (discordHandler) Name() string {
+	return "discord"
+}
+
 func (discordHandler) Execute(ctx context.Context, h *v1.Hook, vars interface{}, runner tasks.TaskRunner) error {
 	payload, err := hookutil.RenderTemplateOrDefault(h.GetActionDiscord().GetTemplate(), hookutil.DefaultTemplate, vars)
 	if err != nil {
diff --git a/internal/hook/types/gotify.go b/internal/hook/types/gotify.go
index a2d74b65..54ce0e04 100644
--- a/internal/hook/types/gotify.go
+++ b/internal/hook/types/gotify.go
@@ -16,6 +16,10 @@ import (
 
 type gotifyHandler struct{}
 
+func (gotifyHandler) Name() string {
+	return "gotify"
+}
+
 func (gotifyHandler) Execute(ctx context.Context, h *v1.Hook, vars interface{}, runner tasks.TaskRunner) error {
 	g := h.GetActionGotify()
 
diff --git a/internal/hook/types/registry.go b/internal/hook/types/registry.go
index cf6bbc03..cb62fbe5 100644
--- a/internal/hook/types/registry.go
+++ b/internal/hook/types/registry.go
@@ -39,6 +39,7 @@ func (r *HandlerRegistry) GetHandler(hook *v1.Hook) (Handler, error) {
 }
 
 type Handler interface {
+	Name() string
 	Execute(ctx context.Context, hook *v1.Hook, vars interface{}, runner tasks.TaskRunner) error
 	ActionType() reflect.Type
 }
diff --git a/internal/hook/types/shoutrrr.go b/internal/hook/types/shoutrrr.go
index 65e8a44a..98eddfab 100644
--- a/internal/hook/types/shoutrrr.go
+++ b/internal/hook/types/shoutrrr.go
@@ -13,6 +13,10 @@ import (
 
 type shoutrrrHandler struct{}
 
+func (shoutrrrHandler) Name() string {
+	return "shoutrrr"
+}
+
 func (shoutrrrHandler) Execute(ctx context.Context, h *v1.Hook, vars interface{}, runner tasks.TaskRunner) error {
 	payload, err := hookutil.RenderTemplateOrDefault(h.GetActionShoutrrr().GetTemplate(), hookutil.DefaultTemplate, vars)
 	if err != nil {
diff --git a/internal/hook/types/slack.go b/internal/hook/types/slack.go
index 8887ec30..995aa6bb 100644
--- a/internal/hook/types/slack.go
+++ b/internal/hook/types/slack.go
@@ -14,6 +14,10 @@ import (
 
 type slackHandler struct{}
 
+func (slackHandler) Name() string {
+	return "slack"
+}
+
 func (slackHandler) Execute(ctx context.Context, cmd *v1.Hook, vars interface{}, runner tasks.TaskRunner) error {
 	payload, err := hookutil.RenderTemplateOrDefault(cmd.GetActionSlack().GetTemplate(), hookutil.DefaultTemplate, vars)
 	if err != nil {

From 0eddbe06b0506525ca3056266f65158ba18d9e9d Mon Sep 17 00:00:00 2001
From: garethgeorge <garethgeorge97@gmail.com>
Date: Thu, 11 Jul 2024 01:23:07 -0700
Subject: [PATCH 15/15] UX improvements and test coverage fixes

---
 internal/api/backresthandler_test.go  |  5 +-
 internal/hook/hook.go                 | 28 ++++++---
 internal/hook/hook_test.go            | 85 ---------------------------
 webui/src/components/OperationRow.tsx |  8 ++-
 4 files changed, 29 insertions(+), 97 deletions(-)
 delete mode 100644 internal/hook/hook_test.go

diff --git a/internal/api/backresthandler_test.go b/internal/api/backresthandler_test.go
index 1fe3bf3e..970e7a5a 100644
--- a/internal/api/backresthandler_test.go
+++ b/internal/api/backresthandler_test.go
@@ -20,6 +20,7 @@ import (
 	"github.com/garethgeorge/backrest/internal/config"
 	"github.com/garethgeorge/backrest/internal/oplog"
 	"github.com/garethgeorge/backrest/internal/orchestrator"
+	"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
 	"github.com/garethgeorge/backrest/internal/resticinstaller"
 	"github.com/garethgeorge/backrest/internal/rotatinglog"
 	"golang.org/x/sync/errgroup"
@@ -387,8 +388,8 @@ func TestHookCancellation(t *testing.T) {
 	}()
 
 	_, err := sut.handler.Backup(context.Background(), connect.NewRequest(&types.StringValue{Value: "test"}))
-	if err != nil {
-		t.Fatalf("Backup() error = %v", err)
+	if !errors.Is(err, tasks.ErrTaskCancelled) {
+		t.Fatalf("Backup() error = %v, want errors.Is(err, tasks.ErrTaskCancelled)", err)
 	}
 
 	// Wait for a hook operation to appear in the oplog
diff --git a/internal/hook/hook.go b/internal/hook/hook.go
index 827801b3..d95b331f 100644
--- a/internal/hook/hook.go
+++ b/internal/hook/hook.go
@@ -33,7 +33,11 @@ func TasksTriggeredByEvent(config *v1.Config, repoID string, planID string, pare
 		}
 
 		name := fmt.Sprintf("repo/%v/hook/%v", repo.Id, idx)
-		taskSet = append(taskSet, newOneoffRunHookTask(name, config.Instance, repoID, planID, parentOp, time.Now(), hook, event, vars))
+		task, err := newOneoffRunHookTask(name, config.Instance, repoID, planID, parentOp, time.Now(), hook, event, vars)
+		if err != nil {
+			return nil, err
+		}
+		taskSet = append(taskSet, task)
 	}
 
 	for idx, hook := range plan.GetHooks() {
@@ -43,13 +47,24 @@ func TasksTriggeredByEvent(config *v1.Config, repoID string, planID string, pare
 		}
 
 		name := fmt.Sprintf("plan/%v/hook/%v", plan.Id, idx)
-		taskSet = append(taskSet, newOneoffRunHookTask(name, config.Instance, repoID, planID, parentOp, time.Now(), hook, event, vars))
+		task, err := newOneoffRunHookTask(name, config.Instance, repoID, planID, parentOp, time.Now(), hook, event, vars)
+		if err != nil {
+			return nil, err
+		}
+		taskSet = append(taskSet, task)
 	}
 
 	return taskSet, nil
 }
 
-func newOneoffRunHookTask(title, instanceID, repoID, planID string, parentOp *v1.Operation, at time.Time, hook *v1.Hook, event v1.Hook_Condition, vars interface{}) tasks.Task {
+func newOneoffRunHookTask(title, instanceID, repoID, planID string, parentOp *v1.Operation, at time.Time, hook *v1.Hook, event v1.Hook_Condition, vars interface{}) (tasks.Task, error) {
+	h, err := types.DefaultRegistry().GetHandler(hook)
+	if err != nil {
+		return nil, fmt.Errorf("no handler for hook type %T", hook.Action)
+	}
+
+	title = h.Name() + " hook " + title
+
 	return &tasks.GenericOneoffTask{
 		OneoffTask: tasks.OneoffTask{
 			BaseTask: tasks.BaseTask{
@@ -76,11 +91,6 @@ func newOneoffRunHookTask(title, instanceID, repoID, planID string, parentOp *v1
 			},
 		},
 		Do: func(ctx context.Context, st tasks.ScheduledTask, taskRunner tasks.TaskRunner) error {
-			h, err := types.DefaultRegistry().GetHandler(hook)
-			if err != nil {
-				return err
-			}
-
 			// TODO: this is a hack to get around the fact that vars is an interface{} .
 			v := reflect.ValueOf(&vars).Elem()
 			clone := reflect.New(v.Elem().Type()).Elem()
@@ -95,7 +105,7 @@ func newOneoffRunHookTask(title, instanceID, repoID, planID string, parentOp *v1
 			}
 			return nil
 		},
-	}
+	}, nil
 }
 
 func firstMatchingCondition(hook *v1.Hook, events []v1.Hook_Condition) v1.Hook_Condition {
diff --git a/internal/hook/hook_test.go b/internal/hook/hook_test.go
deleted file mode 100644
index e1615b83..00000000
--- a/internal/hook/hook_test.go
+++ /dev/null
@@ -1,85 +0,0 @@
-package hook
-
-import (
-	"bytes"
-	"os/exec"
-	"runtime"
-	"testing"
-
-	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-)
-
-func TestHookCommandInDefaultShell(t *testing.T) {
-	hook := Hook(v1.Hook{
-		Conditions: []v1.Hook_Condition{v1.Hook_CONDITION_SNAPSHOT_START},
-		Action: &v1.Hook_ActionCommand{
-			ActionCommand: &v1.Hook_Command{
-				Command: "exit 2",
-			},
-		},
-	})
-
-	err := hook.Do(v1.Hook_CONDITION_SNAPSHOT_START, struct{}{}, &bytes.Buffer{})
-	if err == nil {
-		t.Fatal("expected error")
-	}
-	if err.(*exec.ExitError).ExitCode() != 2 {
-		t.Fatalf("expected exit code 2, got %v", err.(*exec.ExitError).ExitCode())
-	}
-}
-
-func TestHookCommandInBashShell(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("skipping test on windows")
-	}
-
-	hook := Hook(v1.Hook{
-		Conditions: []v1.Hook_Condition{v1.Hook_CONDITION_SNAPSHOT_START},
-		Action: &v1.Hook_ActionCommand{
-			ActionCommand: &v1.Hook_Command{
-				Command: `#!/bin/bash
-counter=0
-# Start a while loop that will run until the counter is equal to 10
-while [ $counter -lt 10 ]; do
-  ((counter++))
-done
-exit $counter`,
-			},
-		},
-	})
-
-	err := hook.Do(v1.Hook_CONDITION_SNAPSHOT_START, struct{}{}, &bytes.Buffer{})
-	if err == nil {
-		t.Fatal("expected error")
-	}
-	if err.(*exec.ExitError).ExitCode() != 10 {
-		t.Fatalf("expected exit code 3, got %v", err.(*exec.ExitError).ExitCode())
-	}
-}
-
-// func TestCommandHookErrorHandling(t *testing.T) {
-// 	if runtime.GOOS == "windows" {
-// 		t.Skip("skipping test on windows")
-// 	}
-
-// 	hook := Hook(v1.Hook{
-// 		Conditions: []v1.Hook_Condition{
-// 			v1.Hook_CONDITION_SNAPSHOT_START,
-// 		},
-// 		Action: &v1.Hook_ActionCommand{
-// 			ActionCommand: &v1.Hook_Command{
-// 				Command: "exit 1",
-// 			},
-// 		},
-// 		OnError: v1.Hook_ON_ERROR_CANCEL,
-// 	})
-
-// 	err := applyHookErrorPolicy(hook.OnError, hook.Do(v1.Hook_CONDITION_SNAPSHOT_START, struct{}{}, &bytes.Buffer{}))
-// 	if err == nil {
-// 		t.Fatal("expected error")
-// 	}
-// 	var cancelErr *HookErrorRequestCancel
-// 	if !errors.As(err, &cancelErr) {
-// 		t.Fatalf("expected HookErrorRequestCancel, got %v", err)
-// 	}
-// }
diff --git a/webui/src/components/OperationRow.tsx b/webui/src/components/OperationRow.tsx
index e4c299e8..43e9a871 100644
--- a/webui/src/components/OperationRow.tsx
+++ b/webui/src/components/OperationRow.tsx
@@ -259,7 +259,13 @@ export const OperationRow = ({
     });
   } else if (operation.op.case === "operationRunHook") {
     const hook = operation.op.value;
-    // TODO: customized view of hook execution info
+    if (operation.logref) {
+      bodyItems.push({
+        key: "logref",
+        label: "Hook Output",
+        children: <BigOperationDataVerbatim logref={operation.logref} />,
+      });
+    }
   }
 
   if (hookOperations) {