refactor(core)!: TaskHandle, security hardening, robustness, and test/doc overhaul

.golangci.yml

@@ -5,7 +5,6 @@ run:
 
 linters:
   disable:
-    - errcheck
     - gosec
   enable:
     - govet

README.md

@@ -83,12 +83,23 @@ preflightMode := task_engine.TaskResultField("preflight", "UpdateMode")
 ```go
 manager := task_engine.NewTaskManager(logger)
 
-// Add and run tasks
-taskID := manager.AddTask(task)
-err := manager.RunTask(context.Background(), taskID)
+// Add tasks (returns error on duplicate ID or nil task)
+if err := manager.AddTask(task); err != nil {
+    logger.Error("Failed to add task", "error", err)
+}
+
+// Run tasks — returns a TaskHandle for async tracking
+handle, err := manager.RunTask("my-task-id")
+if err != nil {
+    logger.Error("Failed to start task", "error", err)
+}
+<-handle.Done() // wait for completion
+if err := handle.Err(); err != nil {
+    logger.Error("Task failed", "error", err)
+}
 
 // Stop tasks
-manager.StopTask(taskID)
+manager.StopTask("my-task-id")
 manager.StopAllTasks()
 ```
 

action_test.go

@@ -780,3 +780,63 @@ func testContext() context.Context {
 func NewDiscardLogger() *slog.Logger {
 	return slog.New(slog.NewTextHandler(io.Discard, nil))
 }
+
+func TestNewBaseAction(t *testing.T) {
+	t.Run("with logger", func(t *testing.T) {
+		logger := NewDiscardLogger()
+		ba := NewBaseAction(logger)
+		if ba.Logger != logger {
+			t.Fatal("expected provided logger")
+		}
+	})
+	t.Run("nil logger gets discard", func(t *testing.T) {
+		ba := NewBaseAction(nil)
+		if ba.Logger == nil {
+			t.Fatal("expected non-nil discard logger")
+		}
+	})
+}
+
+func TestActionGetNameFallback(t *testing.T) {
+	logger := NewDiscardLogger()
+
+	t.Run("name set", func(t *testing.T) {
+		a := NewAction[*TestAction](&TestAction{}, "My Action", logger)
+		if a.GetName() != "My Action" {
+			t.Fatalf("expected 'My Action', got %q", a.GetName())
+		}
+	})
+	t.Run("empty name falls back to ID", func(t *testing.T) {
+		a := &Action[*TestAction]{
+			ID:      "fallback-id",
+			Wrapped: &TestAction{},
+			Logger:  logger,
+		}
+		if a.GetName() != "fallback-id" {
+			t.Fatalf("expected 'fallback-id', got %q", a.GetName())
+		}
+	})
+}
+
+func TestNewActionConstructor(t *testing.T) {
+	logger := NewDiscardLogger()
+
+	t.Run("with explicit ID", func(t *testing.T) {
+		a := NewAction[*TestAction](&TestAction{}, "My Action", logger, "custom-id")
+		if a.ID != "custom-id" {
+			t.Fatalf("expected 'custom-id', got %q", a.ID)
+		}
+	})
+	t.Run("ID generated from name", func(t *testing.T) {
+		a := NewAction[*TestAction](&TestAction{}, "My Action", logger)
+		if a.ID == "" {
+			t.Fatal("expected non-empty generated ID")
+		}
+	})
+	t.Run("empty name and no ID", func(t *testing.T) {
+		a := NewAction[*TestAction](&TestAction{}, "", logger)
+		if a.ID != "" {
+			t.Fatalf("expected empty ID, got %q", a.ID)
+		}
+	})
+}

actions/common/base_constructor_test.go

@@ -0,0 +1,104 @@
+package common_test
+
+import (
+	"context"
+	"log/slog"
+	"os"
+	"testing"
+
+	task_engine "github.com/ndizazzo/task-engine"
+	"github.com/ndizazzo/task-engine/actions/common"
+	"github.com/stretchr/testify/suite"
+)
+
+type mockAction struct{}
+
+func (m *mockAction) BeforeExecute(ctx context.Context) error {
+	return nil
+}
+
+func (m *mockAction) Execute(ctx context.Context) error {
+	return nil
+}
+
+func (m *mockAction) AfterExecute(ctx context.Context) error {
+	return nil
+}
+
+func (m *mockAction) GetOutput() interface{} {
+	return nil
+}
+
+type BaseConstructorTestSuite struct {
+	suite.Suite
+	constructor *common.BaseConstructor[task_engine.ActionInterface]
+	logger      *slog.Logger
+}
+
+func (suite *BaseConstructorTestSuite) SetupTest() {
+	suite.logger = slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError}))
+	suite.constructor = common.NewBaseConstructor[task_engine.ActionInterface](suite.logger)
+}
+
+func (suite *BaseConstructorTestSuite) TestNewBaseConstructor() {
+	constructor := common.NewBaseConstructor[task_engine.ActionInterface](suite.logger)
+	suite.NotNil(constructor)
+}
+
+func (suite *BaseConstructorTestSuite) TestGetLogger() {
+	logger := suite.constructor.GetLogger()
+	suite.NotNil(logger)
+	suite.Equal(suite.logger, logger)
+}
+
+func (suite *BaseConstructorTestSuite) TestWrapAction_WithIDProvided() {
+	action := &mockAction{}
+	wrappedAction := suite.constructor.WrapAction(action, "test-action", "custom-id")
+
+	suite.NotNil(wrappedAction)
+	suite.Equal("custom-id", wrappedAction.ID)
+	suite.Equal("test-action", wrappedAction.Name)
+	suite.Equal(action, wrappedAction.Wrapped)
+}
+
+func (suite *BaseConstructorTestSuite) TestWrapAction_WithoutIDProvided() {
+	action := &mockAction{}
+	wrappedAction := suite.constructor.WrapAction(action, "test-action")
+
+	suite.NotNil(wrappedAction)
+	suite.NotEmpty(wrappedAction.ID)
+	suite.Equal("test-action", wrappedAction.Name)
+	suite.Equal(action, wrappedAction.Wrapped)
+	suite.Contains(wrappedAction.ID, "test-action")
+	suite.Contains(wrappedAction.ID, "action")
+}
+
+func (suite *BaseConstructorTestSuite) TestWrapAction_WithEmptyID() {
+	action := &mockAction{}
+	wrappedAction := suite.constructor.WrapAction(action, "my-test", "")
+
+	suite.NotNil(wrappedAction)
+	suite.NotEmpty(wrappedAction.ID)
+	suite.Contains(wrappedAction.ID, "my-test")
+}
+
+func (suite *BaseConstructorTestSuite) TestWrapAction_WithSpecialCharactersInName() {
+	action := &mockAction{}
+	wrappedAction := suite.constructor.WrapAction(action, "test@action#name", "id123")
+
+	suite.NotNil(wrappedAction)
+	suite.Equal("id123", wrappedAction.ID)
+	suite.Equal("test@action#name", wrappedAction.Name)
+}
+
+func (suite *BaseConstructorTestSuite) TestWrapAction_WithMultipleIDVarargs() {
+	action := &mockAction{}
+	wrappedAction := suite.constructor.WrapAction(action, "test-action", "id1", "id2")
+
+	suite.NotNil(wrappedAction)
+	suite.Equal("id1", wrappedAction.ID)
+}
+
+func TestBaseConstructorTestSuite(t *testing.T) {
+	suite.Run(t, new(BaseConstructorTestSuite))
+}