From 8714e40a21a0b999deefa878eceaa361575724c1 Mon Sep 17 00:00:00 2001 From: Naitik Date: Wed, 30 Sep 2026 14:25:02 +0530 Subject: [PATCH 1/2] fix(tw): isolate Taskwarrior data with TASKDATA and serialize CLI ops cmd.Dir was not enough; Taskwarrior still used /root/.task and GET /tasks wiped it. Point TASKDATA/TASKRC at the per-request temp dir, drop the shared rm -rf, lock all CLI helpers, and log fetch errors. Fixes #471 --- backend/controllers/get_tasks.go | 8 +++++- backend/utils/exec_command.go | 39 +++++++++++++++++++++++++++++- backend/utils/tw/add_task.go | 13 +++++----- backend/utils/tw/complete_task.go | 8 +++--- backend/utils/tw/complete_tasks.go | 7 +++--- backend/utils/tw/delete_task.go | 8 +++--- backend/utils/tw/delete_tasks.go | 7 +++--- backend/utils/tw/edit_task.go | 11 ++++++--- backend/utils/tw/export_tasks.go | 2 +- backend/utils/tw/fetch_tasks.go | 7 ++---- backend/utils/tw/isolation.go | 7 ++++++ backend/utils/tw/modify_task.go | 28 ++++++++++----------- backend/utils/tw/set_config.go | 2 +- backend/utils/tw/sync_tasks.go | 2 +- backend/utils/utils_test.go | 29 ++++++++++++++++++++++ 15 files changed, 127 insertions(+), 51 deletions(-) create mode 100644 backend/utils/tw/isolation.go diff --git a/backend/controllers/get_tasks.go b/backend/controllers/get_tasks.go index b6004286..07051799 100644 --- a/backend/controllers/get_tasks.go +++ b/backend/controllers/get_tasks.go @@ -1,6 +1,8 @@ package controllers import ( + "ccsync_backend/models" + "ccsync_backend/utils" "ccsync_backend/utils/tw" "encoding/json" "net/http" @@ -35,10 +37,14 @@ func TasksHandler(w http.ResponseWriter, r *http.Request) { if r.Method == http.MethodGet { tasks, err := tw.FetchTasksFromTaskwarrior(email, encryptionSecret, origin, UUID) - if err != nil || tasks == nil { + if err != nil { + utils.Logger.Errorf("Failed to fetch tasks: %v", err) http.Error(w, "Failed to fetch tasks at backend", http.StatusInternalServerError) return } + if tasks == nil { + tasks = []models.Task{} + } w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(tasks) diff --git a/backend/utils/exec_command.go b/backend/utils/exec_command.go index d1473e10..ef5c3ccd 100644 --- a/backend/utils/exec_command.go +++ b/backend/utils/exec_command.go @@ -1,6 +1,11 @@ package utils -import "os/exec" +import ( + "os" + "os/exec" + "path/filepath" + "strings" +) func ExecCommandInDir(dir, command string, args ...string) error { cmd := exec.Command(command, args...) @@ -18,3 +23,35 @@ func ExecCommandForOutputInDir(dir, command string, args ...string) ([]byte, err cmd.Dir = dir return cmd.Output() } + +// TaskwarriorEnv returns a copy of the process environment with TASKDATA and +// TASKRC pointing at tempDir so Taskwarrior does not use /root/.task. +func TaskwarriorEnv(tempDir string) []string { + env := os.Environ() + filtered := make([]string, 0, len(env)+2) + for _, e := range env { + if strings.HasPrefix(e, "TASKDATA=") || strings.HasPrefix(e, "TASKRC=") { + continue + } + filtered = append(filtered, e) + } + filtered = append(filtered, + "TASKDATA="+tempDir, + "TASKRC="+filepath.Join(tempDir, "taskrc"), + ) + return filtered +} + +func ExecTaskInDir(dir string, args ...string) error { + cmd := exec.Command("task", args...) + cmd.Dir = dir + cmd.Env = TaskwarriorEnv(dir) + return cmd.Run() +} + +func ExecTaskOutputInDir(dir string, args ...string) ([]byte, error) { + cmd := exec.Command("task", args...) + cmd.Dir = dir + cmd.Env = TaskwarriorEnv(dir) + return cmd.Output() +} diff --git a/backend/utils/tw/add_task.go b/backend/utils/tw/add_task.go index ce65c15f..47253743 100644 --- a/backend/utils/tw/add_task.go +++ b/backend/utils/tw/add_task.go @@ -9,9 +9,8 @@ import ( ) func AddTaskToTaskwarrior(req models.AddTaskRequestBody, dueDate string) error { - if err := utils.ExecCommand("rm", "-rf", "/root/.task"); err != nil { - return fmt.Errorf("error deleting Taskwarrior data: %v", err) - } + taskwarriorMu.Lock() + defer taskwarriorMu.Unlock() tempDir, err := os.MkdirTemp("", utils.SafeTempDirPrefix("taskwarrior-", req.Email)) if err != nil { @@ -75,13 +74,13 @@ func AddTaskToTaskwarrior(req models.AddTaskRequestBody, dueDate string) error { } } - if err := utils.ExecCommandInDir(tempDir, "task", cmdArgs...); err != nil { + if err := utils.ExecTaskInDir(tempDir, cmdArgs...); err != nil { return fmt.Errorf("failed to add task: %v\n %v", err, cmdArgs) } var taskID string if req.End != "" || len(req.Annotations) > 0 { - output, err := utils.ExecCommandForOutputInDir(tempDir, "task", "+LATEST", "_ids") + output, err := utils.ExecTaskOutputInDir(tempDir, "+LATEST", "_ids") if err != nil { return fmt.Errorf("failed to get latest task Id: %v", err) } @@ -95,7 +94,7 @@ func AddTaskToTaskwarrior(req models.AddTaskRequestBody, dueDate string) error { return fmt.Errorf("unexpected end date format error: %v", err) } doneArgs := []string{"rc.confirmation=off", taskID, "done", "end:" + end} - if err := utils.ExecCommandInDir(tempDir, "task", doneArgs...); err != nil { + if err := utils.ExecTaskInDir(tempDir, doneArgs...); err != nil { return fmt.Errorf("failed to complete task with end date: %v", err) } } @@ -104,7 +103,7 @@ func AddTaskToTaskwarrior(req models.AddTaskRequestBody, dueDate string) error { for _, annotation := range req.Annotations { if annotation.Description != "" { annotateArgs := []string{"rc.confirmation=off", taskID, "annotate", annotation.Description} - if err := utils.ExecCommandInDir(tempDir, "task", annotateArgs...); err != nil { + if err := utils.ExecTaskInDir(tempDir, annotateArgs...); err != nil { return fmt.Errorf("failed to add annotation to task %s: %v", taskID, err) } } diff --git a/backend/utils/tw/complete_task.go b/backend/utils/tw/complete_task.go index 1195a941..fee3e219 100644 --- a/backend/utils/tw/complete_task.go +++ b/backend/utils/tw/complete_task.go @@ -7,9 +7,9 @@ import ( ) func CompleteTaskInTaskwarrior(email, encryptionSecret, uuid, taskuuid string) error { - if err := utils.ExecCommand("rm", "-rf", "/root/.task"); err != nil { - return fmt.Errorf("error deleting Taskwarrior data: %v", err) - } + taskwarriorMu.Lock() + defer taskwarriorMu.Unlock() + tempDir, err := os.MkdirTemp("", utils.SafeTempDirPrefix("taskwarrior-", email)) if err != nil { return fmt.Errorf("failed to create temporary directory: %v", err) @@ -25,7 +25,7 @@ func CompleteTaskInTaskwarrior(email, encryptionSecret, uuid, taskuuid string) e return err } - if err := utils.ExecCommandInDir(tempDir, "task", taskuuid, "done", "rc.confirmation=off"); err != nil { + if err := utils.ExecTaskInDir(tempDir, taskuuid, "done", "rc.confirmation=off"); err != nil { return fmt.Errorf("failed to mark task as done: %v", err) } diff --git a/backend/utils/tw/complete_tasks.go b/backend/utils/tw/complete_tasks.go index c97f0561..df644a0b 100644 --- a/backend/utils/tw/complete_tasks.go +++ b/backend/utils/tw/complete_tasks.go @@ -9,9 +9,8 @@ import ( func CompleteTasksInTaskwarrior(email, encryptionSecret, uuid string, taskUUIDs []string) (map[string]string, error) { failedTasks := make(map[string]string) - if err := utils.ExecCommand("rm", "-rf", "/root/.task"); err != nil { - return nil, fmt.Errorf("error deleting Taskwarrior data: %v", err) - } + taskwarriorMu.Lock() + defer taskwarriorMu.Unlock() tempDir, err := os.MkdirTemp("", utils.SafeTempDirPrefix("taskwarrior-", email)) @@ -30,7 +29,7 @@ func CompleteTasksInTaskwarrior(email, encryptionSecret, uuid string, taskUUIDs } for _, taskuuid := range taskUUIDs { - if err := utils.ExecCommandInDir(tempDir, "task", taskuuid, "done", "rc.confirmation=off"); err != nil { + if err := utils.ExecTaskInDir(tempDir, taskuuid, "done", "rc.confirmation=off"); err != nil { failedTasks[taskuuid] = err.Error() continue } diff --git a/backend/utils/tw/delete_task.go b/backend/utils/tw/delete_task.go index 46c4fa2c..fa487a10 100644 --- a/backend/utils/tw/delete_task.go +++ b/backend/utils/tw/delete_task.go @@ -7,9 +7,9 @@ import ( ) func DeleteTaskInTaskwarrior(email, encryptionSecret, uuid, taskuuid string) error { - if err := utils.ExecCommand("rm", "-rf", "/root/.task"); err != nil { - return fmt.Errorf("error deleting Taskwarrior data: %v", err) - } + taskwarriorMu.Lock() + defer taskwarriorMu.Unlock() + tempDir, err := os.MkdirTemp("", utils.SafeTempDirPrefix("taskwarrior-", email)) if err != nil { return fmt.Errorf("failed to create temporary directory: %v", err) @@ -25,7 +25,7 @@ func DeleteTaskInTaskwarrior(email, encryptionSecret, uuid, taskuuid string) err return err } - if err := utils.ExecCommandInDir(tempDir, "task", taskuuid, "delete", "rc.confirmation=off"); err != nil { + if err := utils.ExecTaskInDir(tempDir, taskuuid, "delete", "rc.confirmation=off"); err != nil { return fmt.Errorf("failed to mark task as deleted: %v", err) } diff --git a/backend/utils/tw/delete_tasks.go b/backend/utils/tw/delete_tasks.go index a746abea..6aef9c11 100644 --- a/backend/utils/tw/delete_tasks.go +++ b/backend/utils/tw/delete_tasks.go @@ -9,9 +9,8 @@ import ( func DeleteTasksInTaskwarrior(email, encryptionSecret, uuid string, taskUUIDs []string) (map[string]string, error) { failedTasks := make(map[string]string) - if err := utils.ExecCommand("rm", "-rf", "/root/.task"); err != nil { - return nil, fmt.Errorf("error deleting Taskwarrior data: %v", err) - } + taskwarriorMu.Lock() + defer taskwarriorMu.Unlock() tempDir, err := os.MkdirTemp("", utils.SafeTempDirPrefix("taskwarrior-", email)) @@ -30,7 +29,7 @@ func DeleteTasksInTaskwarrior(email, encryptionSecret, uuid string, taskUUIDs [] } for _, taskuuid := range taskUUIDs { - if err := utils.ExecCommandInDir(tempDir, "task", taskuuid, "delete", "rc.confirmation=off"); err != nil { + if err := utils.ExecTaskInDir(tempDir, taskuuid, "delete", "rc.confirmation=off"); err != nil { failedTasks[taskuuid] = err.Error() continue } diff --git a/backend/utils/tw/edit_task.go b/backend/utils/tw/edit_task.go index b944e418..10b8ff54 100644 --- a/backend/utils/tw/edit_task.go +++ b/backend/utils/tw/edit_task.go @@ -14,6 +14,9 @@ func EditTaskInTaskwarrior( tags, depends []string, annotations []models.Annotation, ) error { + taskwarriorMu.Lock() + defer taskwarriorMu.Unlock() + tempDir, err := os.MkdirTemp("", utils.SafeTempDirPrefix("taskwarrior-", email)) if err != nil { return fmt.Errorf("failed to create temporary directory: %v", err) @@ -84,12 +87,12 @@ func EditTaskInTaskwarrior( } } - if err := utils.ExecCommand("task", modifyArgs...); err != nil { + if err := utils.ExecTaskInDir(tempDir, modifyArgs...); err != nil { return fmt.Errorf("failed to edit task: %v", err) } if len(annotations) > 0 { - output, err := utils.ExecCommandForOutputInDir(tempDir, "task", taskUUID, "export") + output, err := utils.ExecTaskOutputInDir(tempDir, taskUUID, "export") if err == nil { var tasks []map[string]interface{} if err := json.Unmarshal(output, &tasks); err == nil && len(tasks) > 0 { @@ -97,7 +100,7 @@ func EditTaskInTaskwarrior( for _, ann := range existingAnnotations { if annMap, ok := ann.(map[string]interface{}); ok { if desc, ok := annMap["description"].(string); ok { - utils.ExecCommand("task", taskUUID, "denotate", desc) + utils.ExecTaskInDir(tempDir, taskUUID, "denotate", desc) } } } @@ -107,7 +110,7 @@ func EditTaskInTaskwarrior( for _, annotation := range annotations { if annotation.Description != "" { - if err := utils.ExecCommand("task", taskUUID, "annotate", annotation.Description); err != nil { + if err := utils.ExecTaskInDir(tempDir, taskUUID, "annotate", annotation.Description); err != nil { return fmt.Errorf("failed to add annotation %s: %v", annotation.Description, err) } } diff --git a/backend/utils/tw/export_tasks.go b/backend/utils/tw/export_tasks.go index 5690ddd4..03ad758a 100644 --- a/backend/utils/tw/export_tasks.go +++ b/backend/utils/tw/export_tasks.go @@ -9,7 +9,7 @@ import ( // export the tasks so as to add them to DB func ExportTasks(tempDir string) ([]models.Task, error) { - output, err := utils.ExecCommandForOutputInDir(tempDir, "task", "export") + output, err := utils.ExecTaskOutputInDir(tempDir, "export") if err != nil { return nil, fmt.Errorf("error executing Taskwarrior export command: %v", err) } diff --git a/backend/utils/tw/fetch_tasks.go b/backend/utils/tw/fetch_tasks.go index 67d2d137..b7730557 100644 --- a/backend/utils/tw/fetch_tasks.go +++ b/backend/utils/tw/fetch_tasks.go @@ -7,12 +7,9 @@ import ( "os" ) -// complete logic (delete config if any->setup config->sync->get tasks->export) func FetchTasksFromTaskwarrior(email, encryptionSecret, origin, UUID string) ([]models.Task, error) { - // temporary directory for each user - if err := utils.ExecCommand("rm", "-rf", "/root/.task"); err != nil { - return nil, fmt.Errorf("error deleting Taskwarrior data: %v", err) - } + taskwarriorMu.Lock() + defer taskwarriorMu.Unlock() tempDir, err := os.MkdirTemp("", utils.SafeTempDirPrefix("taskwarrior-", email)) if err != nil { diff --git a/backend/utils/tw/isolation.go b/backend/utils/tw/isolation.go new file mode 100644 index 00000000..13920676 --- /dev/null +++ b/backend/utils/tw/isolation.go @@ -0,0 +1,7 @@ +package tw + +import "sync" + +// taskwarriorMu serializes all Taskwarrior CLI use so GET /tasks cannot +// overlap mutation jobs (and so concurrent HTTP fetches do not share a home). +var taskwarriorMu sync.Mutex diff --git a/backend/utils/tw/modify_task.go b/backend/utils/tw/modify_task.go index 0172de4c..4d3406c9 100644 --- a/backend/utils/tw/modify_task.go +++ b/backend/utils/tw/modify_task.go @@ -8,9 +8,9 @@ import ( ) func ModifyTaskInTaskwarrior(uuid, description, project, priority, status, due, email, encryptionSecret, taskID string, tags []string, depends []string) error { - if err := utils.ExecCommand("rm", "-rf", "/root/.task"); err != nil { - return fmt.Errorf("error deleting Taskwarrior data: %v", err) - } + taskwarriorMu.Lock() + defer taskwarriorMu.Unlock() + tempDir, err := os.MkdirTemp("", utils.SafeTempDirPrefix("taskwarrior-", email)) if err != nil { return fmt.Errorf("failed to create temporary directory: %v", err) @@ -28,41 +28,41 @@ func ModifyTaskInTaskwarrior(uuid, description, project, priority, status, due, escapedDescription := fmt.Sprintf(`description:"%s"`, strings.ReplaceAll(description, `"`, `\"`)) - if err := utils.ExecCommand("task", taskID, "modify", escapedDescription); err != nil { + if err := utils.ExecTaskInDir(tempDir, taskID, "modify", escapedDescription); err != nil { return fmt.Errorf("failed to edit task: %v", err) } escapedProject := fmt.Sprintf(`project:%s`, strings.ReplaceAll(project, `"`, `\"`)) - if err := utils.ExecCommand("task", taskID, "modify", escapedProject); err != nil { + if err := utils.ExecTaskInDir(tempDir, taskID, "modify", escapedProject); err != nil { return fmt.Errorf("failed to edit task project: %v", err) } escapedPriority := fmt.Sprintf(`priority:%s`, strings.ReplaceAll(priority, `"`, `\"`)) - if err := utils.ExecCommand("task", taskID, "modify", escapedPriority); err != nil { + if err := utils.ExecTaskInDir(tempDir, taskID, "modify", escapedPriority); err != nil { return fmt.Errorf("failed to edit task priority: %v", err) } escapedDue := fmt.Sprintf(`due:%s`, strings.ReplaceAll(due, `"`, `\"`)) - if err := utils.ExecCommand("task", taskID, "modify", escapedDue); err != nil { + if err := utils.ExecTaskInDir(tempDir, taskID, "modify", escapedDue); err != nil { return fmt.Errorf("failed to edit task due: %v", err) } // Handle dependencies - always set to ensure clearing works - if err := utils.ExecCommand("task", taskID, "modify", "depends:"); err != nil { + if err := utils.ExecTaskInDir(tempDir, taskID, "modify", "depends:"); err != nil { return fmt.Errorf("failed to clear dependencies: %v", err) } if len(depends) > 0 { dependsStr := strings.Join(depends, ",") - if err := utils.ExecCommand("task", taskID, "modify", "depends:"+dependsStr); err != nil { + if err := utils.ExecTaskInDir(tempDir, taskID, "modify", "depends:"+dependsStr); err != nil { return fmt.Errorf("failed to set dependencies %s: %v", dependsStr, err) } } // escapedStatus := fmt.Sprintf(`status:%s`, strings.ReplaceAll(status, `"`, `\"`)) if status == "completed" { - utils.ExecCommand("task", taskID, "done", "rc.confirmation=off") + utils.ExecTaskInDir(tempDir, taskID, "done", "rc.confirmation=off") } else if status == "deleted" { - utils.ExecCommand("task", taskID, "delete", "rc.confirmation=off") + utils.ExecTaskInDir(tempDir, taskID, "delete", "rc.confirmation=off") } // Handle tags @@ -71,18 +71,18 @@ func ModifyTaskInTaskwarrior(uuid, description, project, priority, status, due, if strings.HasPrefix(tag, "+") { // Add tag tagValue := strings.TrimPrefix(tag, "+") - if err := utils.ExecCommand("task", taskID, "modify", "+"+tagValue); err != nil { + if err := utils.ExecTaskInDir(tempDir, taskID, "modify", "+"+tagValue); err != nil { return fmt.Errorf("failed to add tag %s: %v", tagValue, err) } } else if strings.HasPrefix(tag, "-") { // Remove tag tagValue := strings.TrimPrefix(tag, "-") - if err := utils.ExecCommand("task", taskID, "modify", "-"+tagValue); err != nil { + if err := utils.ExecTaskInDir(tempDir, taskID, "modify", "-"+tagValue); err != nil { return fmt.Errorf("failed to remove tag %s: %v", tagValue, err) } } else { // Add tag without prefix - if err := utils.ExecCommand("task", taskID, "modify", "+"+tag); err != nil { + if err := utils.ExecTaskInDir(tempDir, taskID, "modify", "+"+tag); err != nil { return fmt.Errorf("failed to add tag %s: %v", tag, err) } } diff --git a/backend/utils/tw/set_config.go b/backend/utils/tw/set_config.go index 5b9926db..0ab7c431 100644 --- a/backend/utils/tw/set_config.go +++ b/backend/utils/tw/set_config.go @@ -14,7 +14,7 @@ func SetTaskwarriorConfig(tempDir, encryptionSecret, origin, UUID string) error } for _, args := range configCmds { - if err := utils.ExecCommandInDir(tempDir, args[0], args[1:]...); err != nil { + if err := utils.ExecTaskInDir(tempDir, args[1:]...); err != nil { return fmt.Errorf("error setting Taskwarrior config (%v)", err) } } diff --git a/backend/utils/tw/sync_tasks.go b/backend/utils/tw/sync_tasks.go index 79f77726..72f94546 100644 --- a/backend/utils/tw/sync_tasks.go +++ b/backend/utils/tw/sync_tasks.go @@ -7,7 +7,7 @@ import ( // sync the user's tasks to all of their TW clients func SyncTaskwarrior(tempDir string) error { - if err := utils.ExecCommandInDir(tempDir, "task", "sync"); err != nil { + if err := utils.ExecTaskInDir(tempDir, "sync"); err != nil { return fmt.Errorf("error syncing Taskwarrior: %v", err) } return nil diff --git a/backend/utils/utils_test.go b/backend/utils/utils_test.go index 39bb0e14..632ea224 100644 --- a/backend/utils/utils_test.go +++ b/backend/utils/utils_test.go @@ -4,6 +4,8 @@ import ( "crypto/sha256" "encoding/hex" "os" + "path/filepath" + "strings" "testing" "github.com/google/uuid" @@ -85,6 +87,33 @@ func Test_ExecCommandForOutputInDir(t *testing.T) { } } +func Test_TaskwarriorEnv(t *testing.T) { + t.Setenv("TASKDATA", "/root/.task") + t.Setenv("TASKRC", "/root/.taskrc") + + tempDir := t.TempDir() + env := TaskwarriorEnv(tempDir) + + var data, rc string + for _, e := range env { + switch { + case strings.HasPrefix(e, "TASKDATA="): + if data != "" { + t.Fatal("duplicate TASKDATA") + } + data = strings.TrimPrefix(e, "TASKDATA=") + case strings.HasPrefix(e, "TASKRC="): + if rc != "" { + t.Fatal("duplicate TASKRC") + } + rc = strings.TrimPrefix(e, "TASKRC=") + } + } + + assert.Equal(t, tempDir, data) + assert.Equal(t, filepath.Join(tempDir, "taskrc"), rc) +} + func Test_ValidateDependencies_EmptyList(t *testing.T) { depends := []string{} currentTaskUUID := "current-task-uuid" From 83b2c483dd22b37295cc79f5650ac44fce2638a6 Mon Sep 17 00:00:00 2001 From: Naitik Date: Tue, 13 Oct 2026 08:14:26 +0530 Subject: [PATCH 2/2] fix: serialize task fetching through job queue --- backend/controllers/get_tasks.go | 56 +++++-- backend/controllers/get_tasks_test.go | 226 ++++++++++++++++++++++++++ backend/controllers/job_queue_test.go | 68 ++++++++ backend/utils/tw/add_task.go | 3 - backend/utils/tw/complete_task.go | 3 - backend/utils/tw/complete_tasks.go | 3 - backend/utils/tw/delete_task.go | 3 - backend/utils/tw/delete_tasks.go | 3 - backend/utils/tw/edit_task.go | 3 - backend/utils/tw/fetch_tasks.go | 3 - backend/utils/tw/isolation.go | 7 - backend/utils/tw/modify_task.go | 3 - 12 files changed, 340 insertions(+), 41 deletions(-) create mode 100644 backend/controllers/get_tasks_test.go create mode 100644 backend/controllers/job_queue_test.go delete mode 100644 backend/utils/tw/isolation.go diff --git a/backend/controllers/get_tasks.go b/backend/controllers/get_tasks.go index 07051799..664463f0 100644 --- a/backend/controllers/get_tasks.go +++ b/backend/controllers/get_tasks.go @@ -5,10 +5,15 @@ import ( "ccsync_backend/utils" "ccsync_backend/utils/tw" "encoding/json" + "fmt" "net/http" "os" ) +// fetchTasksFromTaskwarrior is the Taskwarrior fetch used by TasksHandler. +// Tests override it to avoid requiring a live Taskwarrior installation. +var fetchTasksFromTaskwarrior = tw.FetchTasksFromTaskwarrior + // TasksHandler godoc // @Summary Get all tasks // @Description Fetch all tasks from Taskwarrior for a specific user via Headers @@ -36,19 +41,50 @@ func TasksHandler(w http.ResponseWriter, r *http.Request) { } if r.Method == http.MethodGet { - tasks, err := tw.FetchTasksFromTaskwarrior(email, encryptionSecret, origin, UUID) - if err != nil { - utils.Logger.Errorf("Failed to fetch tasks: %v", err) - http.Error(w, "Failed to fetch tasks at backend", http.StatusInternalServerError) - return + type fetchResult struct { + tasks []models.Task + err error } - if tasks == nil { - tasks = []models.Task{} + // Buffer of 1 so the queue worker never blocks if the client disconnects + // before the handler receives the result. + resultCh := make(chan fetchResult, 1) + + job := Job{ + Name: "Get Tasks", + Execute: func() (err error) { + var tasks []models.Task + defer func() { + if rec := recover(); rec != nil { + utils.Logger.Errorf("Get Tasks job panicked: %v", rec) + err = fmt.Errorf("panic while fetching tasks: %v", rec) + } + resultCh <- fetchResult{tasks: tasks, err: err} + }() + tasks, err = fetchTasksFromTaskwarrior(email, encryptionSecret, origin, UUID) + return err + }, } + GlobalJobQueue.AddJob(job) - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(tasks) - return + select { + case res := <-resultCh: + if res.err != nil { + utils.Logger.Errorf("Failed to fetch tasks: %v", res.err) + http.Error(w, "Failed to fetch tasks at backend", http.StatusInternalServerError) + return + } + tasks := res.tasks + if tasks == nil { + tasks = []models.Task{} + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(tasks) + return + case <-r.Context().Done(): + // Client disconnected; the job may still finish and send into resultCh. + return + } } http.Error(w, "Invalid request method", http.StatusMethodNotAllowed) diff --git a/backend/controllers/get_tasks_test.go b/backend/controllers/get_tasks_test.go new file mode 100644 index 00000000..dca78b09 --- /dev/null +++ b/backend/controllers/get_tasks_test.go @@ -0,0 +1,226 @@ +package controllers + +import ( + "ccsync_backend/models" + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func withMockFetch(t *testing.T, fn func(email, encryptionSecret, origin, UUID string) ([]models.Task, error)) { + t.Helper() + orig := fetchTasksFromTaskwarrior + fetchTasksFromTaskwarrior = fn + t.Cleanup(func() { + fetchTasksFromTaskwarrior = orig + }) +} + +func Test_TasksHandler_MissingHeaders(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/tasks", nil) + rr := httptest.NewRecorder() + TasksHandler(rr, req) + assert.Equal(t, http.StatusBadRequest, rr.Code) +} + +func Test_TasksHandler_MethodNotAllowed(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/tasks", nil) + req.Header.Set("X-User-Email", "a@b.com") + req.Header.Set("X-Encryption-Secret", "secret") + req.Header.Set("X-User-UUID", "uuid") + rr := httptest.NewRecorder() + TasksHandler(rr, req) + assert.Equal(t, http.StatusMethodNotAllowed, rr.Code) +} + +func Test_TasksHandler_ReturnsOwnFetchResult(t *testing.T) { + GlobalJobQueue = NewJobQueue() + withMockFetch(t, func(email, encryptionSecret, origin, UUID string) ([]models.Task, error) { + return []models.Task{{ + UUID: UUID, + Description: email, + }}, nil + }) + + var wg sync.WaitGroup + results := make([][]models.Task, 2) + errs := make([]error, 2) + + for i, uuid := range []string{"user-a", "user-b"} { + wg.Add(1) + go func(i int, uuid string) { + defer wg.Done() + req := httptest.NewRequest(http.MethodGet, "/tasks", nil) + req.Header.Set("X-User-Email", uuid+"@example.com") + req.Header.Set("X-Encryption-Secret", "secret") + req.Header.Set("X-User-UUID", uuid) + rr := httptest.NewRecorder() + TasksHandler(rr, req) + if rr.Code != http.StatusOK { + errs[i] = fmt.Errorf("status %d: %s", rr.Code, rr.Body.String()) + return + } + var tasks []models.Task + if err := json.Unmarshal(rr.Body.Bytes(), &tasks); err != nil { + errs[i] = err + return + } + results[i] = tasks + }(i, uuid) + } + wg.Wait() + + require.NoError(t, errs[0]) + require.NoError(t, errs[1]) + require.Len(t, results[0], 1) + require.Len(t, results[1], 1) + assert.Equal(t, "user-a", results[0][0].UUID) + assert.Equal(t, "user-b", results[1][0].UUID) + assert.Equal(t, "user-a@example.com", results[0][0].Description) + assert.Equal(t, "user-b@example.com", results[1][0].Description) +} + +func Test_TasksHandler_EmptyListReturnsJSONArray(t *testing.T) { + GlobalJobQueue = NewJobQueue() + withMockFetch(t, func(email, encryptionSecret, origin, UUID string) ([]models.Task, error) { + return nil, nil + }) + + req := httptest.NewRequest(http.MethodGet, "/tasks", nil) + req.Header.Set("X-User-Email", "a@b.com") + req.Header.Set("X-Encryption-Secret", "secret") + req.Header.Set("X-User-UUID", "uuid") + rr := httptest.NewRecorder() + TasksHandler(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code) + assert.Equal(t, "application/json", rr.Header().Get("Content-Type")) + assert.JSONEq(t, "[]", rr.Body.String()) +} + +func Test_TasksHandler_FetchFailureReturns500(t *testing.T) { + GlobalJobQueue = NewJobQueue() + withMockFetch(t, func(email, encryptionSecret, origin, UUID string) ([]models.Task, error) { + return nil, errors.New("sync failed") + }) + + req := httptest.NewRequest(http.MethodGet, "/tasks", nil) + req.Header.Set("X-User-Email", "a@b.com") + req.Header.Set("X-Encryption-Secret", "secret") + req.Header.Set("X-User-UUID", "uuid") + rr := httptest.NewRecorder() + TasksHandler(rr, req) + + assert.Equal(t, http.StatusInternalServerError, rr.Code) + assert.Contains(t, rr.Body.String(), "Failed to fetch tasks at backend") +} + +func Test_TasksHandler_FetchPanicReturns500AndKeepsQueueUsable(t *testing.T) { + GlobalJobQueue = NewJobQueue() + withMockFetch(t, func(email, encryptionSecret, origin, UUID string) ([]models.Task, error) { + panic("simulated fetch panic") + }) + + handlerDone := make(chan *httptest.ResponseRecorder, 1) + go func() { + req := httptest.NewRequest(http.MethodGet, "/tasks", nil) + req.Header.Set("X-User-Email", "a@b.com") + req.Header.Set("X-Encryption-Secret", "secret") + req.Header.Set("X-User-UUID", "uuid") + rr := httptest.NewRecorder() + TasksHandler(rr, req) + handlerDone <- rr + }() + + var rr *httptest.ResponseRecorder + select { + case rr = <-handlerDone: + case <-time.After(2 * time.Second): + t.Fatal("handler did not complete after fetch panic") + } + + assert.Equal(t, http.StatusInternalServerError, rr.Code) + assert.Contains(t, rr.Body.String(), "Failed to fetch tasks at backend") + + followUpDone := make(chan struct{}) + GlobalJobQueue.AddJob(Job{ + Name: "follow-up-after-panic", + Execute: func() error { + close(followUpDone) + return nil + }, + }) + + select { + case <-followUpDone: + case <-time.After(2 * time.Second): + t.Fatal("queue worker blocked after fetch panic") + } +} + +func Test_TasksHandler_ClientDisconnectDoesNotBlockQueue(t *testing.T) { + GlobalJobQueue = NewJobQueue() + + fetchStarted := make(chan struct{}) + fetchRelease := make(chan struct{}) + withMockFetch(t, func(email, encryptionSecret, origin, UUID string) ([]models.Task, error) { + close(fetchStarted) + <-fetchRelease + return []models.Task{}, nil + }) + + ctx, cancel := context.WithCancel(context.Background()) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "/tasks", nil) + require.NoError(t, err) + req.Header.Set("X-User-Email", "a@b.com") + req.Header.Set("X-Encryption-Secret", "secret") + req.Header.Set("X-User-UUID", "uuid") + + handlerDone := make(chan struct{}) + go func() { + defer close(handlerDone) + rr := httptest.NewRecorder() + TasksHandler(rr, req) + }() + + select { + case <-fetchStarted: + case <-time.After(2 * time.Second): + t.Fatal("fetch job did not start") + } + + cancel() + + select { + case <-handlerDone: + case <-time.After(2 * time.Second): + t.Fatal("handler did not return after client cancel") + } + + // A subsequent job must still be able to run (queue worker not stuck). + followUpDone := make(chan struct{}) + GlobalJobQueue.AddJob(Job{ + Name: "follow-up", + Execute: func() error { + close(followUpDone) + return nil + }, + }) + + close(fetchRelease) + + select { + case <-followUpDone: + case <-time.After(2 * time.Second): + t.Fatal("queue worker blocked after client disconnect") + } +} diff --git a/backend/controllers/job_queue_test.go b/backend/controllers/job_queue_test.go new file mode 100644 index 00000000..ad339a98 --- /dev/null +++ b/backend/controllers/job_queue_test.go @@ -0,0 +1,68 @@ +package controllers + +import ( + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_JobQueue_ExecutesSequentially(t *testing.T) { + q := NewJobQueue() + + var mu sync.Mutex + var order []int + startedSecond := make(chan struct{}) + releaseFirst := make(chan struct{}) + done := make(chan struct{}, 2) + + q.AddJob(Job{ + Name: "first", + Execute: func() error { + mu.Lock() + order = append(order, 1) + mu.Unlock() + close(startedSecond) + <-releaseFirst + done <- struct{}{} + return nil + }, + }) + q.AddJob(Job{ + Name: "second", + Execute: func() error { + mu.Lock() + order = append(order, 2) + mu.Unlock() + done <- struct{}{} + return nil + }, + }) + + select { + case <-startedSecond: + case <-time.After(2 * time.Second): + t.Fatal("first job did not start") + } + + // While the first job is held open, the second must not have run yet. + mu.Lock() + require.Equal(t, []int{1}, order) + mu.Unlock() + + close(releaseFirst) + + for i := 0; i < 2; i++ { + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("jobs did not finish") + } + } + + mu.Lock() + defer mu.Unlock() + assert.Equal(t, []int{1, 2}, order) +} diff --git a/backend/utils/tw/add_task.go b/backend/utils/tw/add_task.go index 47253743..6211214a 100644 --- a/backend/utils/tw/add_task.go +++ b/backend/utils/tw/add_task.go @@ -9,9 +9,6 @@ import ( ) func AddTaskToTaskwarrior(req models.AddTaskRequestBody, dueDate string) error { - taskwarriorMu.Lock() - defer taskwarriorMu.Unlock() - tempDir, err := os.MkdirTemp("", utils.SafeTempDirPrefix("taskwarrior-", req.Email)) if err != nil { return fmt.Errorf("failed to create temporary directory: %v", err) diff --git a/backend/utils/tw/complete_task.go b/backend/utils/tw/complete_task.go index fee3e219..c9a59bb6 100644 --- a/backend/utils/tw/complete_task.go +++ b/backend/utils/tw/complete_task.go @@ -7,9 +7,6 @@ import ( ) func CompleteTaskInTaskwarrior(email, encryptionSecret, uuid, taskuuid string) error { - taskwarriorMu.Lock() - defer taskwarriorMu.Unlock() - tempDir, err := os.MkdirTemp("", utils.SafeTempDirPrefix("taskwarrior-", email)) if err != nil { return fmt.Errorf("failed to create temporary directory: %v", err) diff --git a/backend/utils/tw/complete_tasks.go b/backend/utils/tw/complete_tasks.go index df644a0b..49cc5043 100644 --- a/backend/utils/tw/complete_tasks.go +++ b/backend/utils/tw/complete_tasks.go @@ -9,9 +9,6 @@ import ( func CompleteTasksInTaskwarrior(email, encryptionSecret, uuid string, taskUUIDs []string) (map[string]string, error) { failedTasks := make(map[string]string) - taskwarriorMu.Lock() - defer taskwarriorMu.Unlock() - tempDir, err := os.MkdirTemp("", utils.SafeTempDirPrefix("taskwarrior-", email)) if err != nil { diff --git a/backend/utils/tw/delete_task.go b/backend/utils/tw/delete_task.go index fa487a10..abff318c 100644 --- a/backend/utils/tw/delete_task.go +++ b/backend/utils/tw/delete_task.go @@ -7,9 +7,6 @@ import ( ) func DeleteTaskInTaskwarrior(email, encryptionSecret, uuid, taskuuid string) error { - taskwarriorMu.Lock() - defer taskwarriorMu.Unlock() - tempDir, err := os.MkdirTemp("", utils.SafeTempDirPrefix("taskwarrior-", email)) if err != nil { return fmt.Errorf("failed to create temporary directory: %v", err) diff --git a/backend/utils/tw/delete_tasks.go b/backend/utils/tw/delete_tasks.go index 6aef9c11..e504aee7 100644 --- a/backend/utils/tw/delete_tasks.go +++ b/backend/utils/tw/delete_tasks.go @@ -9,9 +9,6 @@ import ( func DeleteTasksInTaskwarrior(email, encryptionSecret, uuid string, taskUUIDs []string) (map[string]string, error) { failedTasks := make(map[string]string) - taskwarriorMu.Lock() - defer taskwarriorMu.Unlock() - tempDir, err := os.MkdirTemp("", utils.SafeTempDirPrefix("taskwarrior-", email)) if err != nil { diff --git a/backend/utils/tw/edit_task.go b/backend/utils/tw/edit_task.go index 10b8ff54..67463fe7 100644 --- a/backend/utils/tw/edit_task.go +++ b/backend/utils/tw/edit_task.go @@ -14,9 +14,6 @@ func EditTaskInTaskwarrior( tags, depends []string, annotations []models.Annotation, ) error { - taskwarriorMu.Lock() - defer taskwarriorMu.Unlock() - tempDir, err := os.MkdirTemp("", utils.SafeTempDirPrefix("taskwarrior-", email)) if err != nil { return fmt.Errorf("failed to create temporary directory: %v", err) diff --git a/backend/utils/tw/fetch_tasks.go b/backend/utils/tw/fetch_tasks.go index b7730557..fc7b4150 100644 --- a/backend/utils/tw/fetch_tasks.go +++ b/backend/utils/tw/fetch_tasks.go @@ -8,9 +8,6 @@ import ( ) func FetchTasksFromTaskwarrior(email, encryptionSecret, origin, UUID string) ([]models.Task, error) { - taskwarriorMu.Lock() - defer taskwarriorMu.Unlock() - tempDir, err := os.MkdirTemp("", utils.SafeTempDirPrefix("taskwarrior-", email)) if err != nil { return nil, fmt.Errorf("failed to create temporary directory: %v", err) diff --git a/backend/utils/tw/isolation.go b/backend/utils/tw/isolation.go deleted file mode 100644 index 13920676..00000000 --- a/backend/utils/tw/isolation.go +++ /dev/null @@ -1,7 +0,0 @@ -package tw - -import "sync" - -// taskwarriorMu serializes all Taskwarrior CLI use so GET /tasks cannot -// overlap mutation jobs (and so concurrent HTTP fetches do not share a home). -var taskwarriorMu sync.Mutex diff --git a/backend/utils/tw/modify_task.go b/backend/utils/tw/modify_task.go index 4d3406c9..3f66635e 100644 --- a/backend/utils/tw/modify_task.go +++ b/backend/utils/tw/modify_task.go @@ -8,9 +8,6 @@ import ( ) func ModifyTaskInTaskwarrior(uuid, description, project, priority, status, due, email, encryptionSecret, taskID string, tags []string, depends []string) error { - taskwarriorMu.Lock() - defer taskwarriorMu.Unlock() - tempDir, err := os.MkdirTemp("", utils.SafeTempDirPrefix("taskwarrior-", email)) if err != nil { return fmt.Errorf("failed to create temporary directory: %v", err)