From 76825c109e66a04a2c219ca1241c2a3dc529ee90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Fri, 4 Sep 2026 02:35:15 +0000 Subject: [PATCH] feat(tasks): edit task dependencies (#3308) (#3323) --- .../internal/common/taskdependencies/lock.go | 12 + .../common/taskdependencies/lock_test.go | 29 ++ .../office/dashboard/service_tasks.go | 25 +- .../office/repository/sqlite/blockers.go | 58 +++ .../office/repository/sqlite/blockers_test.go | 86 ++++ .../task/handlers/task_dependency_handlers.go | 44 +- .../handlers/task_dependency_handlers_test.go | 233 ++++++++++ .../internal/task/handlers/task_handlers.go | 1 + .../internal/task/service/handoff_service.go | 3 + apps/backend/internal/task/service/service.go | 18 +- .../task/service/service_dependencies.go | 169 ++++++- .../task/service/service_dependencies_test.go | 46 +- .../service_dependency_replacement_test.go | 171 +++++++ .../internal/task/service/service_office.go | 34 ++ .../task-create-dialog-dependencies.test.tsx | 28 ++ .../task-create-dialog-dependencies.tsx | 188 ++++++-- .../task-create-dialog-dependency-search.ts | 11 +- .../task-create-dialog-footer.test.ts | 22 + .../components/task-create-dialog-footer.tsx | 35 +- .../task-create-dialog-prop-builders.ts | 3 + .../components/task-create-dialog-setup.ts | 72 +-- .../task-create-dialog-submit.test.tsx | 86 ++++ .../components/task-create-dialog-submit.tsx | 84 +++- .../components/task-create-dialog-types.ts | 5 + .../components/task-create-dialog.test.tsx | 4 + apps/web/components/task-create-dialog.tsx | 9 +- .../task-edit-dialog-dependencies.test.tsx | 145 ++++++ .../task-edit-dialog-dependencies.tsx | 125 ++++++ apps/web/e2e/helpers/api-client.ts | 17 + .../create-task-dependency-selector.spec.ts | 84 ++++ ...le-create-task-dependency-selector.spec.ts | 87 ++++ .../use-task-edit-dialog-dependencies.test.ts | 350 +++++++++++++++ .../task/use-task-edit-dialog-dependencies.ts | 424 ++++++++++++++++++ .../api/domains/task-dependencies-api.test.ts | 83 ++++ .../lib/api/domains/task-dependencies-api.ts | 43 +- apps/web/lib/types/task-dependencies.ts | 5 + apps/web/src/locales/en/task.json | 5 + apps/web/src/locales/pseudo/task.json | 5 + apps/web/src/locales/pt-pt/task.json | 5 + apps/web/src/locales/zh-cn/task.json | 5 + apps/web/src/locales/zh-hk/task.json | 5 + apps/web/src/locales/zh-tw/task.json | 5 + .../task-dependency-detail-editing/plan.md | 130 ++++++ .../task-01-enable-detail-editing.md | 114 +++++ .../task-02-add-edit-dialog-dependencies.md | 153 +++++++ docs/specs/tasks/README.md | 2 + ...dencies-create-dialog-advanced-settings.md | 13 +- ...ncies-create-dialog-dependency-selector.md | 9 +- .../task-dependency-detail-editing.md | 54 +++ .../tasks/system-design/task-dependencies.md | 9 +- .../task-dependency-detail-editing.md | 208 +++++++++ 51 files changed, 3394 insertions(+), 167 deletions(-) create mode 100644 apps/backend/internal/common/taskdependencies/lock.go create mode 100644 apps/backend/internal/common/taskdependencies/lock_test.go create mode 100644 apps/backend/internal/task/handlers/task_dependency_handlers_test.go create mode 100644 apps/backend/internal/task/service/service_dependency_replacement_test.go create mode 100644 apps/web/components/task-edit-dialog-dependencies.test.tsx create mode 100644 apps/web/components/task-edit-dialog-dependencies.tsx create mode 100644 apps/web/hooks/domains/task/use-task-edit-dialog-dependencies.test.ts create mode 100644 apps/web/hooks/domains/task/use-task-edit-dialog-dependencies.ts create mode 100644 apps/web/lib/api/domains/task-dependencies-api.test.ts create mode 100644 apps/web/lib/types/task-dependencies.ts create mode 100644 docs/plans/task-dependency-detail-editing/plan.md create mode 100644 docs/plans/task-dependency-detail-editing/task-01-enable-detail-editing.md create mode 100644 docs/plans/task-dependency-detail-editing/task-02-add-edit-dialog-dependencies.md create mode 100644 docs/specs/tasks/requirements/task-dependency-detail-editing.md create mode 100644 docs/specs/tasks/system-design/task-dependency-detail-editing.md diff --git a/apps/backend/internal/common/taskdependencies/lock.go b/apps/backend/internal/common/taskdependencies/lock.go new file mode 100644 index 0000000000..707bb470c4 --- /dev/null +++ b/apps/backend/internal/common/taskdependencies/lock.go @@ -0,0 +1,12 @@ +package taskdependencies + +import "sync" + +var edgeMutationMu sync.Mutex + +// AcquireMutationLock serializes dependency graph validation and mutation +// across task and Office services in this backend process. +func AcquireMutationLock() func() { + edgeMutationMu.Lock() + return edgeMutationMu.Unlock +} diff --git a/apps/backend/internal/common/taskdependencies/lock_test.go b/apps/backend/internal/common/taskdependencies/lock_test.go new file mode 100644 index 0000000000..e1c931650d --- /dev/null +++ b/apps/backend/internal/common/taskdependencies/lock_test.go @@ -0,0 +1,29 @@ +package taskdependencies + +import ( + "testing" + "time" +) + +func TestAcquireMutationLockSerializesCallers(t *testing.T) { + unlock := AcquireMutationLock() + acquired := make(chan struct{}) + go func() { + secondUnlock := AcquireMutationLock() + close(acquired) + secondUnlock() + }() + + select { + case <-acquired: + t.Fatal("second caller acquired the mutation lock before the first released it") + case <-time.After(100 * time.Millisecond): + } + + unlock() + select { + case <-acquired: + case <-time.After(time.Second): + t.Fatal("second caller did not acquire the mutation lock after release") + } +} diff --git a/apps/backend/internal/office/dashboard/service_tasks.go b/apps/backend/internal/office/dashboard/service_tasks.go index 18e70fab11..9ca0ab4f03 100644 --- a/apps/backend/internal/office/dashboard/service_tasks.go +++ b/apps/backend/internal/office/dashboard/service_tasks.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" + "github.com/kandev/kandev/internal/common/taskdependencies" "github.com/kandev/kandev/internal/events" "github.com/kandev/kandev/internal/events/bus" "github.com/kandev/kandev/internal/office/models" @@ -177,11 +178,17 @@ func joinPath(path []string) string { // of any length. On cycle detection returns a *BlockerCycleError whose // Path lists the cycle for the caller to surface. func (s *DashboardService) AddTaskBlocker(ctx context.Context, taskID, blockerTaskID string) error { - if err := s.validateBlockerPair(ctx, taskID, blockerTaskID); err != nil { - return err - } - blocker := &models.TaskBlocker{TaskID: taskID, BlockerTaskID: blockerTaskID} - if err := s.repo.CreateTaskBlocker(ctx, blocker); err != nil { + var err error + func() { + unlock := taskdependencies.AcquireMutationLock() + defer unlock() + if err = s.validateBlockerPair(ctx, taskID, blockerTaskID); err != nil { + return + } + blocker := &models.TaskBlocker{TaskID: taskID, BlockerTaskID: blockerTaskID} + err = s.repo.CreateTaskBlocker(ctx, blocker) + }() + if err != nil { return err } s.publishTaskUpdated(ctx, taskID, []string{fieldBlockers}) @@ -193,7 +200,13 @@ func (s *DashboardService) AddTaskBlocker(ctx context.Context, taskID, blockerTa // row is a no-op at the DB level; the event/activity entry are still // emitted so the UI re-fetches. func (s *DashboardService) RemoveTaskBlocker(ctx context.Context, taskID, blockerTaskID string) error { - if err := s.repo.DeleteTaskBlocker(ctx, taskID, blockerTaskID); err != nil { + var err error + func() { + unlock := taskdependencies.AcquireMutationLock() + defer unlock() + err = s.repo.DeleteTaskBlocker(ctx, taskID, blockerTaskID) + }() + if err != nil { return err } s.publishTaskUpdated(ctx, taskID, []string{fieldBlockers}) diff --git a/apps/backend/internal/office/repository/sqlite/blockers.go b/apps/backend/internal/office/repository/sqlite/blockers.go index 93e8410682..4cd8c929a4 100644 --- a/apps/backend/internal/office/repository/sqlite/blockers.go +++ b/apps/backend/internal/office/repository/sqlite/blockers.go @@ -39,6 +39,64 @@ func (r *Repository) DeleteTaskBlocker(ctx context.Context, taskID, blockerTaskI return err } +// ReplaceTaskBlockers atomically replaces the direct blockers for taskID. +// Existing edges stay in place so their creation timestamps remain stable; +// only omitted edges are deleted and new edges are inserted. +func (r *Repository) ReplaceTaskBlockers(ctx context.Context, taskID string, blockerTaskIDs []string) error { + tx, err := r.db.BeginTxx(ctx, nil) + if err != nil { + return err + } + committed := false + defer func() { + if !committed { + _ = tx.Rollback() + } + }() + + var existing []string + if err := tx.SelectContext(ctx, &existing, tx.Rebind( + `SELECT blocker_task_id FROM task_blockers WHERE task_id = ?`), taskID); err != nil { + return err + } + existingSet := make(map[string]struct{}, len(existing)) + for _, blockerTaskID := range existing { + existingSet[blockerTaskID] = struct{}{} + } + desiredSet := make(map[string]struct{}, len(blockerTaskIDs)) + for _, blockerTaskID := range blockerTaskIDs { + desiredSet[blockerTaskID] = struct{}{} + } + + for blockerTaskID := range existingSet { + if _, keep := desiredSet[blockerTaskID]; keep { + continue + } + if _, err := tx.ExecContext(ctx, tx.Rebind( + `DELETE FROM task_blockers WHERE task_id = ? AND blocker_task_id = ?`), + taskID, blockerTaskID); err != nil { + return err + } + } + for _, blockerTaskID := range blockerTaskIDs { + if _, alreadyExists := existingSet[blockerTaskID]; alreadyExists { + continue + } + if _, err := tx.ExecContext(ctx, tx.Rebind(` + INSERT INTO task_blockers (task_id, blocker_task_id, created_at) + VALUES (?, ?, ?) + `), taskID, blockerTaskID, time.Now().UTC()); err != nil { + return err + } + existingSet[blockerTaskID] = struct{}{} + } + if err := tx.Commit(); err != nil { + return err + } + committed = true + return nil +} + // ListTasksBlockedBy returns task IDs that are blocked by the given task. // This is the reverse direction of ListTaskBlockers; results are ordered by // insertion time so callers see a stable list. diff --git a/apps/backend/internal/office/repository/sqlite/blockers_test.go b/apps/backend/internal/office/repository/sqlite/blockers_test.go index eb632336b7..f5e1e581af 100644 --- a/apps/backend/internal/office/repository/sqlite/blockers_test.go +++ b/apps/backend/internal/office/repository/sqlite/blockers_test.go @@ -4,6 +4,7 @@ import ( "context" "slices" "testing" + "time" "github.com/kandev/kandev/internal/office/models" ) @@ -327,3 +328,88 @@ func TestTaskBlocker_SelfReferenceBlocked(t *testing.T) { t.Fatal("expected CHECK constraint error for self-reference") } } + +func TestReplaceTaskBlockers_ReplacesSetAndPreservesExistingEdges(t *testing.T) { + repo := newTestRepo(t) + ctx := context.Background() + + for _, blocker := range []*models.TaskBlocker{ + {TaskID: "task-1", BlockerTaskID: "task-2"}, + {TaskID: "task-1", BlockerTaskID: "task-4"}, + } { + if err := repo.CreateTaskBlocker(ctx, blocker); err != nil { + t.Fatalf("create blocker: %v", err) + } + } + before, err := repo.ListTaskBlockers(ctx, "task-1") + if err != nil { + t.Fatalf("list before replacement: %v", err) + } + if len(before) != 2 { + t.Fatalf("before replacement count = %d, want 2", len(before)) + } + var preservedCreatedAt = before[0].CreatedAt + for _, blocker := range before { + if blocker.BlockerTaskID == "task-2" { + preservedCreatedAt = blocker.CreatedAt + } + } + + if err := repo.ReplaceTaskBlockers(ctx, "task-1", []string{"task-2", "task-3"}); err != nil { + t.Fatalf("replace blockers: %v", err) + } + + after, err := repo.ListTaskBlockers(ctx, "task-1") + if err != nil { + t.Fatalf("list after replacement: %v", err) + } + if len(after) != 2 { + t.Fatalf("after replacement count = %d, want 2", len(after)) + } + got := make(map[string]time.Time, len(after)) + for _, blocker := range after { + got[blocker.BlockerTaskID] = blocker.CreatedAt + } + if _, ok := got["task-4"]; ok { + t.Error("omitted blocker task-4 remains after replacement") + } + if got["task-2"] != preservedCreatedAt { + t.Errorf("preserved task-2 timestamp changed from %v to %v", preservedCreatedAt, got["task-2"]) + } + if got["task-3"].IsZero() { + t.Error("new task-3 blocker has no created timestamp") + } + + if err := repo.ReplaceTaskBlockers(ctx, "task-1", nil); err != nil { + t.Fatalf("clear blockers: %v", err) + } + cleared, err := repo.ListTaskBlockers(ctx, "task-1") + if err != nil { + t.Fatalf("list after clear: %v", err) + } + if len(cleared) != 0 { + t.Fatalf("after clear count = %d, want 0", len(cleared)) + } +} + +func TestReplaceTaskBlockers_RollsBackPartialDiffOnWriteError(t *testing.T) { + repo := newTestRepo(t) + ctx := context.Background() + if err := repo.CreateTaskBlocker(ctx, &models.TaskBlocker{ + TaskID: "task-1", BlockerTaskID: "task-2", + }); err != nil { + t.Fatalf("create blocker: %v", err) + } + + if err := repo.ReplaceTaskBlockers(ctx, "task-1", []string{"task-3", "task-1"}); err == nil { + t.Fatal("expected self-reference write to fail") + } + + blockers, err := repo.ListTaskBlockers(ctx, "task-1") + if err != nil { + t.Fatalf("list after failed replacement: %v", err) + } + if len(blockers) != 1 || blockers[0].BlockerTaskID != "task-2" { + t.Fatalf("failed replacement changed the set: %+v", blockers) + } +} diff --git a/apps/backend/internal/task/handlers/task_dependency_handlers.go b/apps/backend/internal/task/handlers/task_dependency_handlers.go index 9ad65af798..9f24020816 100644 --- a/apps/backend/internal/task/handlers/task_dependency_handlers.go +++ b/apps/backend/internal/task/handlers/task_dependency_handlers.go @@ -37,8 +37,9 @@ func dependencyRefs(refs []service.DependencyRef) []dto.TaskDependencyRefDTO { // Response keys repeated across the dependency handlers. const ( - dependencyKeyError = "error" - dependencyKeyTaskID = "task_id" + dependencyKeyError = "error" + dependencyKeyTaskID = "task_id" + maxDependencyRequestBytes = 128 << 10 ) // addTaskDependencyBody is the POST /tasks/:id/dependencies payload. @@ -46,6 +47,13 @@ type addTaskDependencyBody struct { DependsOnTaskID string `json:"depends_on_task_id" binding:"required"` } +// replaceTaskDependenciesBody is the PUT /tasks/:id/dependencies payload. +// A pointer distinguishes an explicit empty list, which clears all edges, +// from a request that omitted the complete-set field. +type replaceTaskDependenciesBody struct { + DependsOnTaskIDs *[]string `json:"depends_on_task_ids"` +} + // httpAddTaskDependency handles POST /api/v1/tasks/:id/dependencies — // "this task is blocked by depends_on_task_id". // @@ -53,6 +61,7 @@ type addTaskDependencyBody struct { // "A → B → C → A"; that body shape is what BlockersPicker already parses. func (h *TaskHandlers) httpAddTaskDependency(c *gin.Context) { taskID := c.Param("id") + c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, maxDependencyRequestBytes) var body addTaskDependencyBody if err := c.ShouldBindJSON(&body); err != nil { c.JSON(http.StatusBadRequest, gin.H{dependencyKeyError: "depends_on_task_id is required"}) @@ -75,6 +84,37 @@ func (h *TaskHandlers) httpAddTaskDependency(c *gin.Context) { handleNotFound(c, h.logger, err, "task not found") } +// httpReplaceTaskDependencies handles PUT /api/v1/tasks/:id/dependencies. +// The request is a complete predecessor set, including an empty list to clear +// all direct dependencies. +func (h *TaskHandlers) httpReplaceTaskDependencies(c *gin.Context) { + c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, maxDependencyRequestBytes) + var body replaceTaskDependenciesBody + if err := c.ShouldBindJSON(&body); err != nil || body.DependsOnTaskIDs == nil { + c.JSON(http.StatusBadRequest, gin.H{dependencyKeyError: "depends_on_task_ids is required"}) + return + } + err := h.service.ReplaceDependencies(c.Request.Context(), c.Param("id"), *body.DependsOnTaskIDs) + if err == nil { + h.respondWithDependencies(c, c.Param("id")) + return + } + var cycle *service.CycleError + if errors.As(err, &cycle) { + c.JSON(http.StatusConflict, gin.H{dependencyKeyError: cycle.Error(), "cycle": cycle.Path}) + return + } + if errors.Is(err, service.ErrInvalidDependencySet) { + c.JSON(http.StatusBadRequest, taskErrorBody(err)) + return + } + if errors.Is(err, service.ErrDependencyRepositoryUnavailable) { + c.JSON(http.StatusServiceUnavailable, gin.H{dependencyKeyError: err.Error()}) + return + } + handleNotFound(c, h.logger, err, "task not found") +} + // httpRemoveTaskDependency handles DELETE /api/v1/tasks/:id/dependencies/:depId. // Removing an absent edge is a success no-op. func (h *TaskHandlers) httpRemoveTaskDependency(c *gin.Context) { diff --git a/apps/backend/internal/task/handlers/task_dependency_handlers_test.go b/apps/backend/internal/task/handlers/task_dependency_handlers_test.go new file mode 100644 index 0000000000..73b5e7646c --- /dev/null +++ b/apps/backend/internal/task/handlers/task_dependency_handlers_test.go @@ -0,0 +1,233 @@ +package handlers + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/require" + + orchmodels "github.com/kandev/kandev/internal/office/models" + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/service" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +type dependencyHandlerRepo struct { + mockRepository + tasks map[string]*models.Task + blockers []*orchmodels.TaskBlocker +} + +func (r *dependencyHandlerRepo) GetTask(_ context.Context, id string) (*models.Task, error) { + return r.tasks[id], nil +} + +func (r *dependencyHandlerRepo) GetTasksByIDs(_ context.Context, ids []string) ([]*models.Task, error) { + result := make([]*models.Task, 0, len(ids)) + for _, id := range ids { + if task := r.tasks[id]; task != nil { + result = append(result, task) + } + } + return result, nil +} + +func (r *dependencyHandlerRepo) CreateTaskBlocker(_ context.Context, blocker *orchmodels.TaskBlocker) error { + r.blockers = append(r.blockers, blocker) + return nil +} + +func (r *dependencyHandlerRepo) ListTaskBlockers(_ context.Context, taskID string) ([]*orchmodels.TaskBlocker, error) { + result := make([]*orchmodels.TaskBlocker, 0) + for _, blocker := range r.blockers { + if blocker.TaskID == taskID { + result = append(result, blocker) + } + } + return result, nil +} + +func (r *dependencyHandlerRepo) DeleteTaskBlocker(_ context.Context, taskID, blockerTaskID string) error { + for index, blocker := range r.blockers { + if blocker.TaskID == taskID && blocker.BlockerTaskID == blockerTaskID { + r.blockers = append(r.blockers[:index], r.blockers[index+1:]...) + break + } + } + return nil +} + +func (r *dependencyHandlerRepo) ListTasksBlockedBy(_ context.Context, blockerTaskID string) ([]string, error) { + ids := make([]string, 0) + for _, blocker := range r.blockers { + if blocker.BlockerTaskID == blockerTaskID { + ids = append(ids, blocker.TaskID) + } + } + return ids, nil +} + +func (r *dependencyHandlerRepo) ListBlockersForTasks(_ context.Context, taskIDs []string) (map[string][]string, error) { + wanted := make(map[string]struct{}, len(taskIDs)) + for _, id := range taskIDs { + wanted[id] = struct{}{} + } + result := make(map[string][]string) + for _, blocker := range r.blockers { + if _, ok := wanted[blocker.TaskID]; ok { + result[blocker.TaskID] = append(result[blocker.TaskID], blocker.BlockerTaskID) + } + } + return result, nil +} + +func (r *dependencyHandlerRepo) ListDependentsForTasks(_ context.Context, taskIDs []string) (map[string][]string, error) { + wanted := make(map[string]struct{}, len(taskIDs)) + for _, id := range taskIDs { + wanted[id] = struct{}{} + } + result := make(map[string][]string) + for _, blocker := range r.blockers { + if _, ok := wanted[blocker.BlockerTaskID]; ok { + result[blocker.BlockerTaskID] = append(result[blocker.BlockerTaskID], blocker.TaskID) + } + } + return result, nil +} + +func (r *dependencyHandlerRepo) ReplaceTaskBlockers(_ context.Context, taskID string, blockerTaskIDs []string) error { + next := make([]*orchmodels.TaskBlocker, 0, len(r.blockers)+len(blockerTaskIDs)) + for _, blocker := range r.blockers { + if blocker.TaskID != taskID { + next = append(next, blocker) + } + } + for _, blockerTaskID := range blockerTaskIDs { + next = append(next, &orchmodels.TaskBlocker{TaskID: taskID, BlockerTaskID: blockerTaskID}) + } + r.blockers = next + return nil +} + +func newDependencyHandlers(t *testing.T, repo *dependencyHandlerRepo) *TaskHandlers { + t.Helper() + log := newTestLogger(t) + svc := service.NewService(service.Repos{ + Workspaces: repo, Tasks: repo, TaskRepos: repo, + Workflows: repo, Messages: repo, Turns: repo, + Sessions: repo, GitSnapshots: repo, RepoEntities: repo, + Executors: repo, Environments: repo, TaskEnvironments: repo, + Reviews: repo, + }, nil, log, service.RepositoryDiscoveryConfig{}) + svc.SetBlockerRepository(repo) + return &TaskHandlers{service: svc, logger: log} +} + +func dependencyRequest(t *testing.T, h *TaskHandlers, taskID, body string) *httptest.ResponseRecorder { + t.Helper() + recorder := httptest.NewRecorder() + context, _ := gin.CreateTestContext(recorder) + context.Request = httptest.NewRequest( + http.MethodPut, + "/api/v1/tasks/"+taskID+"/dependencies", + strings.NewReader(body), + ) + context.Params = gin.Params{{Key: "id", Value: taskID}} + h.httpReplaceTaskDependencies(context) + return recorder +} + +func dependencyTask(id, title string) *models.Task { + return &models.Task{ + ID: id, WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", + Title: title, State: v1.TaskStateTODO, + } +} + +func TestHTTPReplaceTaskDependenciesReturnsProjection(t *testing.T) { + repo := &dependencyHandlerRepo{tasks: map[string]*models.Task{ + "task-1": dependencyTask("task-1", "Dependent"), + "task-2": dependencyTask("task-2", "Predecessor"), + }} + h := newDependencyHandlers(t, repo) + + recorder := dependencyRequest(t, h, "task-1", `{"depends_on_task_ids":["task-2"]}`) + + require.Equal(t, http.StatusOK, recorder.Code) + var response map[string]interface{} + require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &response)) + require.Equal(t, "task-1", response["task_id"]) + require.Len(t, response["depends_on"], 1) + require.Len(t, repo.blockers, 1) + require.Equal(t, "task-2", repo.blockers[0].BlockerTaskID) +} + +func TestHTTPReplaceTaskDependenciesRejectsCycleAndPreservesSet(t *testing.T) { + repo := &dependencyHandlerRepo{tasks: map[string]*models.Task{ + "task-a": dependencyTask("task-a", "A"), + "task-b": dependencyTask("task-b", "B"), + "task-c": dependencyTask("task-c", "C"), + }, blockers: []*orchmodels.TaskBlocker{ + {TaskID: "task-a", BlockerTaskID: "task-b"}, + {TaskID: "task-b", BlockerTaskID: "task-c"}, + }} + h := newDependencyHandlers(t, repo) + + recorder := dependencyRequest(t, h, "task-c", `{"depends_on_task_ids":["task-a"]}`) + + require.Equal(t, http.StatusConflict, recorder.Code) + var response struct { + Cycle []string `json:"cycle"` + } + require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &response)) + require.Equal(t, []string{"task-c", "task-a", "task-b", "task-c"}, response.Cycle) + require.Len(t, repo.blockers, 2) +} + +func TestHTTPReplaceTaskDependenciesRequiresCompleteList(t *testing.T) { + repo := &dependencyHandlerRepo{tasks: map[string]*models.Task{ + "task-1": dependencyTask("task-1", "Dependent"), + }} + h := newDependencyHandlers(t, repo) + + recorder := dependencyRequest(t, h, "task-1", `{}`) + + require.Equal(t, http.StatusBadRequest, recorder.Code) + require.Contains(t, recorder.Body.String(), "depends_on_task_ids") +} + +func TestHTTPReplaceTaskDependenciesRejectsTooManyIDs(t *testing.T) { + repo := &dependencyHandlerRepo{tasks: map[string]*models.Task{ + "task-1": dependencyTask("task-1", "Dependent"), + }} + h := newDependencyHandlers(t, repo) + ids := make([]string, 513) + for index := range ids { + ids[index] = "task-2" + } + body, err := json.Marshal(map[string][]string{"depends_on_task_ids": ids}) + require.NoError(t, err) + + recorder := dependencyRequest(t, h, "task-1", string(body)) + + require.Equal(t, http.StatusBadRequest, recorder.Code) + require.Empty(t, repo.blockers) +} + +func TestHTTPReplaceTaskDependenciesRejectsOversizedBody(t *testing.T) { + repo := &dependencyHandlerRepo{tasks: map[string]*models.Task{ + "task-1": dependencyTask("task-1", "Dependent"), + }} + h := newDependencyHandlers(t, repo) + body := `{"depends_on_task_ids":["` + strings.Repeat("x", maxDependencyRequestBytes) + `"]}` + + recorder := dependencyRequest(t, h, "task-1", body) + + require.Equal(t, http.StatusBadRequest, recorder.Code) + require.Empty(t, repo.blockers) +} diff --git a/apps/backend/internal/task/handlers/task_handlers.go b/apps/backend/internal/task/handlers/task_handlers.go index e1e8b4d0bb..d8a71f2a5e 100644 --- a/apps/backend/internal/task/handlers/task_handlers.go +++ b/apps/backend/internal/task/handlers/task_handlers.go @@ -204,6 +204,7 @@ func (h *TaskHandlers) registerHTTP(router *gin.Engine) { // equivalents of the Office-only blocker routes; both go through the single // validator in the task service. api.POST("/tasks/:id/dependencies", h.httpAddTaskDependency) + api.PUT("/tasks/:id/dependencies", h.httpReplaceTaskDependencies) api.DELETE("/tasks/:id/dependencies/:depId", h.httpRemoveTaskDependency) api.POST("/tasks/bulk-move", h.httpBulkMoveTasks) diff --git a/apps/backend/internal/task/service/handoff_service.go b/apps/backend/internal/task/service/handoff_service.go index c7ab6e3a31..a733d66c5f 100644 --- a/apps/backend/internal/task/service/handoff_service.go +++ b/apps/backend/internal/task/service/handoff_service.go @@ -9,6 +9,7 @@ import ( "time" "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/common/taskdependencies" orchmodels "github.com/kandev/kandev/internal/office/models" "github.com/kandev/kandev/internal/task/models" "github.com/kandev/kandev/internal/task/repository" @@ -487,6 +488,8 @@ func (s *HandoffService) attachSequentialBlocker(ctx context.Context, taskID, pa mu := s.parentLock.lockFor(parentID) mu.Lock() defer mu.Unlock() + edgesUnlock := taskdependencies.AcquireMutationLock() + defer edgesUnlock() siblings, err := s.tasks.ListChildren(ctx, parentID) if err != nil { diff --git a/apps/backend/internal/task/service/service.go b/apps/backend/internal/task/service/service.go index 7b5be0c492..7a68da6ab1 100644 --- a/apps/backend/internal/task/service/service.go +++ b/apps/backend/internal/task/service/service.go @@ -376,17 +376,13 @@ type Service struct { repositorySelectionResolver RepositorySelectionResolver repoCloneLocation RepoCloneLocation blockers BlockerRepository - // dependencyEdgeMu serializes validate-then-insert for dependency edges so - // two concurrent adds cannot each pass a cycle walk that predates the - // other's insert and commit a cycle between them. - dependencyEdgeMu sync.Mutex - comments CommentRepository - taskStateActivity TaskStateActivityLogger - secretStore secrets.SecretStore - workspaceSecretDeleter WorkspaceSecretDeleter - baseBranchPusher AgentBaseBranchPusher - comparisonTargetPusher AgentComparisonTargetPusher - runtimeOverridesMu sync.Mutex + comments CommentRepository + taskStateActivity TaskStateActivityLogger + secretStore secrets.SecretStore + workspaceSecretDeleter WorkspaceSecretDeleter + baseBranchPusher AgentBaseBranchPusher + comparisonTargetPusher AgentComparisonTargetPusher + runtimeOverridesMu sync.Mutex workspaceSourceProviderRefresher WorkspaceSourceProviderRefresher diff --git a/apps/backend/internal/task/service/service_dependencies.go b/apps/backend/internal/task/service/service_dependencies.go index 656ec0ef41..08c17888d7 100644 --- a/apps/backend/internal/task/service/service_dependencies.go +++ b/apps/backend/internal/task/service/service_dependencies.go @@ -8,6 +8,7 @@ import ( "go.uber.org/zap" + "github.com/kandev/kandev/internal/common/taskdependencies" "github.com/kandev/kandev/internal/events" "github.com/kandev/kandev/internal/task/models" taskrepo "github.com/kandev/kandev/internal/task/repository" @@ -55,6 +56,11 @@ const ( // exceeds it rejects the edge rather than accepting an unverified one. const dependencyCycleWalkLimit = 1000 +const ( + maxTaskDependencyCount = 512 + maxTaskDependencyIDLength = 128 +) + // DependencyRef is one end of a dependency edge, carrying enough detail for the // dependency chip to render without fetching each related task. type DependencyRef struct { @@ -91,6 +97,13 @@ func (e *CycleError) Error() string { // not wired. Callers must treat it as "cannot determine", not "not blocked". var ErrDependencyRepositoryUnavailable = fmt.Errorf("dependency repository not configured") +// ErrInvalidDependencySet identifies malformed full-set replacement input. +// Authorization errors intentionally do not use this sentinel, so foreign +// task IDs remain indistinguishable from missing IDs. +var ErrInvalidDependencySet = errors.New("invalid dependency set") + +var errDependencyCrossWorkspace = errors.New("dependency tasks must share a workspace") + // ResolveStartWhenUnblocked decides whether a create request's agent start // should become a start-when-unblocked intent instead of an immediate launch. // @@ -129,7 +142,7 @@ func (s *Service) validateDependencyPair(ctx context.Context, taskID, dependsOnT return fmt.Errorf("%w: %s", taskrepo.ErrTaskNotFound, dependsOnTaskID) } if task.WorkspaceID != "" && dep.WorkspaceID != "" && task.WorkspaceID != dep.WorkspaceID { - return fmt.Errorf("task %s belongs to a different workspace", dependsOnTaskID) + return fmt.Errorf("%w: task %s belongs to a different workspace", errDependencyCrossWorkspace, dependsOnTaskID) } return nil } @@ -149,10 +162,10 @@ func (s *Service) authorizeDependencyPair(ctx context.Context, taskID, dependsOn // AddDependency records "taskID depends on dependsOnTaskID". // -// This is the single validator for dependency edges. Self-edges, -// cross-workspace edges, and cycles of any length are rejected here, and both -// the task-scoped routes and the Office blocker routes go through it — a second -// validator would let a cycle in through whichever path was weakest. +// This is the task-service validator for dependency edges. Self-edges, +// cross-workspace edges, and cycles of any length are rejected here. The task +// and Office mutation surfaces share the same process-wide mutation lock while +// they validate and write their respective repository adapters. func (s *Service) AddDependency(ctx context.Context, taskID, dependsOnTaskID string) error { if s.blockers == nil { return ErrDependencyRepositoryUnavailable @@ -163,6 +176,9 @@ func (s *Service) AddDependency(ctx context.Context, taskID, dependsOnTaskID str if taskID == dependsOnTaskID { return fmt.Errorf("a task cannot depend on itself") } + if len(dependsOnTaskID) > maxTaskDependencyIDLength { + return fmt.Errorf("dependency task IDs cannot exceed %d characters", maxTaskDependencyIDLength) + } if err := s.authorizeDependencyPair(ctx, taskID, dependsOnTaskID); err != nil { return err } @@ -184,11 +200,116 @@ func (s *Service) AddDependency(ctx context.Context, taskID, dependsOnTaskID str return nil } -// insertDependencyEdge holds dependencyEdgeMu across exactly the validate, -// cycle-walk and insert, and nothing else. +// ReplaceDependencies replaces every direct predecessor of taskID in one +// validated operation. The complete desired set is checked before storage is +// changed, and the repository applies its edge diff in one transaction. +func (s *Service) ReplaceDependencies(ctx context.Context, taskID string, dependsOnTaskIDs []string) error { + if s.blockers == nil { + return ErrDependencyRepositoryUnavailable + } + if taskID == "" { + return fmt.Errorf("%w: task_id is required", ErrInvalidDependencySet) + } + if err := s.authorizeTaskID(ctx, taskID); err != nil { + return err + } + if err := validateDependencyIDs(taskID, dependsOnTaskIDs); err != nil { + return err + } + for _, dependsOnTaskID := range dependsOnTaskIDs { + if err := s.authorizeTaskID(ctx, dependsOnTaskID); err != nil { + return err + } + } + replacer, ok := s.blockers.(taskDependencyReplacer) + if !ok { + return ErrDependencyRepositoryUnavailable + } + changed, err := s.replaceDependencyEdges(ctx, taskID, dependsOnTaskIDs, replacer) + if err != nil { + return err + } + if len(changed) == 0 { + return nil + } + s.publishDependencyChange(ctx, append([]string{taskID}, changed...)...) + return nil +} + +func validateDependencyIDs(taskID string, dependsOnTaskIDs []string) error { + if len(dependsOnTaskIDs) > maxTaskDependencyCount { + return fmt.Errorf("%w: at most %d dependency task IDs are allowed", ErrInvalidDependencySet, maxTaskDependencyCount) + } + seen := make(map[string]struct{}, len(dependsOnTaskIDs)) + for _, dependsOnTaskID := range dependsOnTaskIDs { + switch dependsOnTaskID { + case "": + return fmt.Errorf("%w: dependency task IDs cannot be empty", ErrInvalidDependencySet) + case taskID: + return fmt.Errorf("%w: a task cannot depend on itself", ErrInvalidDependencySet) + } + if len(dependsOnTaskID) > maxTaskDependencyIDLength { + return fmt.Errorf("%w: dependency task IDs cannot exceed %d characters", ErrInvalidDependencySet, maxTaskDependencyIDLength) + } + if _, duplicate := seen[dependsOnTaskID]; duplicate { + return fmt.Errorf("%w: dependency %s is listed more than once", ErrInvalidDependencySet, dependsOnTaskID) + } + seen[dependsOnTaskID] = struct{}{} + } + return nil +} + +// replaceDependencyEdges holds the dependency lock across all reads that +// validate the graph and the repository's atomic replacement. +func (s *Service) replaceDependencyEdges( + ctx context.Context, + taskID string, + dependsOnTaskIDs []string, + replacer taskDependencyReplacer, +) ([]string, error) { + unlock := taskdependencies.AcquireMutationLock() + defer unlock() + + if err := s.validateReplacementSet(ctx, taskID, dependsOnTaskIDs); err != nil { + return nil, err + } + existing, err := s.blockers.ListTaskBlockers(ctx, taskID) + if err != nil { + return nil, fmt.Errorf("list task dependencies: %w", err) + } + for _, dependsOnTaskID := range dependsOnTaskIDs { + cycle, err := s.checkDependencyCycle(ctx, taskID, dependsOnTaskID) + if err != nil { + return nil, fmt.Errorf("check dependency cycle: %w", err) + } + if cycle != nil { + return nil, cycle + } + } + changed := changedDependencyIDs(existing, dependsOnTaskIDs) + if err := replacer.ReplaceTaskBlockers(ctx, taskID, dependsOnTaskIDs); err != nil { + return nil, err + } + return changed, nil +} + +func (s *Service) validateReplacementSet(ctx context.Context, taskID string, dependsOnTaskIDs []string) error { + for _, dependsOnTaskID := range dependsOnTaskIDs { + if err := s.validateDependencyPair(ctx, taskID, dependsOnTaskID); err != nil { + if errors.Is(err, errDependencyCrossWorkspace) { + return fmt.Errorf("%w: %w", ErrInvalidDependencySet, err) + } + return err + } + } + return nil +} + +// insertDependencyEdge holds the shared mutation lock across exactly the +// validate, cycle-walk and insert, and nothing else. func (s *Service) insertDependencyEdge(ctx context.Context, taskID, dependsOnTaskID string) error { - s.dependencyEdgeMu.Lock() - defer s.dependencyEdgeMu.Unlock() + unlock := taskdependencies.AcquireMutationLock() + defer unlock() if err := s.validateDependencyPair(ctx, taskID, dependsOnTaskID); err != nil { return err } @@ -222,10 +343,10 @@ func (s *Service) RemoveDependency(ctx context.Context, taskID, dependsOnTaskID return nil } -// deleteDependencyEdge holds the edge lock across the delete only. +// deleteDependencyEdge holds the shared mutation lock across the delete only. func (s *Service) deleteDependencyEdge(ctx context.Context, taskID, dependsOnTaskID string) error { - s.dependencyEdgeMu.Lock() - defer s.dependencyEdgeMu.Unlock() + unlock := taskdependencies.AcquireMutationLock() + defer unlock() return s.blockers.DeleteTaskBlocker(ctx, taskID, dependsOnTaskID) } @@ -600,6 +721,8 @@ func (s *Service) pruneDanglingEdge(ctx context.Context, taskID, missingTaskID s if s.blockers == nil { return } + unlock := taskdependencies.AcquireMutationLock() + defer unlock() if err := s.blockers.DeleteTaskBlocker(ctx, taskID, missingTaskID); err != nil { s.logger.Warn("failed to prune dangling dependency edge", zap.String("task_id", taskID), @@ -626,14 +749,22 @@ func (s *Service) deleteDependencyEdgesForTask(ctx context.Context, taskID strin if !ok { return } - dependents, err := s.blockers.ListTasksBlockedBy(ctx, taskID) - if err != nil { - s.logger.Warn("failed to list dependents before edge cleanup", - zap.String("task_id", taskID), zap.Error(err)) - } - if err := cleaner.DeleteTaskBlockersForTask(ctx, taskID); err != nil { + var dependents []string + var cleanupErr error + func() { + unlock := taskdependencies.AcquireMutationLock() + defer unlock() + var err error + dependents, err = s.blockers.ListTasksBlockedBy(ctx, taskID) + if err != nil { + s.logger.Warn("failed to list dependents before edge cleanup", + zap.String("task_id", taskID), zap.Error(err)) + } + cleanupErr = cleaner.DeleteTaskBlockersForTask(ctx, taskID) + }() + if cleanupErr != nil { s.logger.Warn("failed to clean up dependency edges for deleted task", - zap.String("task_id", taskID), zap.Error(err)) + zap.String("task_id", taskID), zap.Error(cleanupErr)) return } // Dependents may now be unblocked, so refresh them. Deliberately no diff --git a/apps/backend/internal/task/service/service_dependencies_test.go b/apps/backend/internal/task/service/service_dependencies_test.go index 0acb461f5d..a9d88caebe 100644 --- a/apps/backend/internal/task/service/service_dependencies_test.go +++ b/apps/backend/internal/task/service/service_dependencies_test.go @@ -3,6 +3,7 @@ package service import ( "context" "errors" + "strings" "sync" "testing" "time" @@ -297,35 +298,11 @@ func TestDependencyMutationsAreScopedToTheCaller(t *testing.T) { } } -// gateBlockerRepo forces the interleaving the cycle race needs: each cycle walk -// announces itself and then waits for the other walk to start. Two bare -// goroutines practically never land inside the validate-then-insert window, so -// a test without this barrier passes even with the serialization removed and -// proves nothing. -type gateBlockerRepo struct { - *mockBlockerRepo - arrived chan struct{} - once sync.Once -} - -func (g *gateBlockerRepo) ListTaskBlockers(ctx context.Context, taskID string) ([]*orchmodels.TaskBlocker, error) { - g.once.Do(func() { close(g.arrived) }) - select { - case <-g.arrived: - case <-time.After(time.Second): - } - // Give the other goroutine room to finish its own walk before this one - // inserts. Serialization must make that impossible; without it both walks - // complete against a pre-insert view. - time.Sleep(20 * time.Millisecond) - return g.mockBlockerRepo.ListTaskBlockers(ctx, taskID) -} - // Two concurrent adds must not each pass a cycle walk that predates the other's // insert and commit a cycle between them. func TestAddDependencyConcurrentInsertsCannotCreateACycle(t *testing.T) { svc, _ := setupOfficeTest(t) - repo := &gateBlockerRepo{mockBlockerRepo: &mockBlockerRepo{}, arrived: make(chan struct{})} + repo := &mockBlockerRepo{} svc.SetBlockerRepository(repo) ctx := context.Background() a := mustSeedTask(t, svc, "A").ID @@ -440,3 +417,22 @@ func TestDependencyEventFieldsCarryTheProjection(t *testing.T) { t.Errorf("depends_on = %+v; want one entry for %s", deps, predecessor.ID) } } + +func TestValidateDependencyIDsRejectsOversizedSets(t *testing.T) { + ids := make([]string, maxTaskDependencyCount+1) + for index := range ids { + ids[index] = "dependency" + } + + err := validateDependencyIDs("task", ids) + if !errors.Is(err, ErrInvalidDependencySet) { + t.Fatalf("error = %v, want ErrInvalidDependencySet", err) + } +} + +func TestValidateDependencyIDsRejectsOversizedID(t *testing.T) { + err := validateDependencyIDs("task", []string{strings.Repeat("x", maxTaskDependencyIDLength+1)}) + if !errors.Is(err, ErrInvalidDependencySet) { + t.Fatalf("error = %v, want ErrInvalidDependencySet", err) + } +} diff --git a/apps/backend/internal/task/service/service_dependency_replacement_test.go b/apps/backend/internal/task/service/service_dependency_replacement_test.go new file mode 100644 index 0000000000..f9e89d6ee9 --- /dev/null +++ b/apps/backend/internal/task/service/service_dependency_replacement_test.go @@ -0,0 +1,171 @@ +package service + +import ( + "context" + "errors" + "testing" + + "github.com/kandev/kandev/internal/events" + orchmodels "github.com/kandev/kandev/internal/office/models" + taskrepo "github.com/kandev/kandev/internal/task/repository" +) + +type replacementBlockerRepo struct { + *mockBlockerRepo + replaceErr error +} + +func (r *replacementBlockerRepo) ReplaceTaskBlockers( + _ context.Context, + taskID string, + blockerTaskIDs []string, +) error { + if r.replaceErr != nil { + return r.replaceErr + } + next := make([]*orchmodels.TaskBlocker, 0, len(r.blockers)+len(blockerTaskIDs)) + for _, blocker := range r.blockers { + if blocker.TaskID != taskID { + next = append(next, blocker) + } + } + for _, blockerTaskID := range blockerTaskIDs { + next = append(next, &orchmodels.TaskBlocker{ + TaskID: taskID, + BlockerTaskID: blockerTaskID, + }) + } + r.blockers = next + return nil +} + +func TestReplaceDependencies_ReplacesSetAndPublishesChangedTasks(t *testing.T) { + svc, repo := setupOfficeTest(t) + blockers := &replacementBlockerRepo{mockBlockerRepo: &mockBlockerRepo{}} + svc.SetBlockerRepository(blockers) + ctx := context.Background() + oldPredecessor := mustSeedTask(t, svc, "Old predecessor") + newPredecessor := mustSeedTask(t, svc, "New predecessor") + dependent := mustSeedTask(t, svc, "Dependent") + if err := blockers.CreateTaskBlocker(ctx, &orchmodels.TaskBlocker{ + TaskID: dependent.ID, BlockerTaskID: oldPredecessor.ID, + }); err != nil { + t.Fatalf("seed dependency: %v", err) + } + eventBus, ok := svc.eventBus.(*MockEventBus) + if !ok { + t.Fatal("test service does not use MockEventBus") + } + eventBus.ClearEvents() + + if err := svc.ReplaceDependencies(ctx, dependent.ID, []string{newPredecessor.ID}); err != nil { + t.Fatalf("ReplaceDependencies: %v", err) + } + + got, err := svc.GetBlockers(ctx, dependent.ID) + if err != nil { + t.Fatalf("GetBlockers: %v", err) + } + if len(got) != 1 || got[0] != newPredecessor.ID { + t.Fatalf("blockers = %v, want [%s]", got, newPredecessor.ID) + } + + updatedIDs := map[string]bool{} + for _, event := range eventBus.GetPublishedEvents() { + if event.Type != events.TaskUpdated { + continue + } + data, ok := event.Data.(map[string]interface{}) + if !ok { + t.Fatalf("task.updated data type = %T", event.Data) + } + if id, ok := data["task_id"].(string); ok { + updatedIDs[id] = true + } + } + if len(updatedIDs) != 3 { + t.Fatalf("updated task IDs = %v, want dependent and both changed peers", updatedIDs) + } + for _, id := range []string{dependent.ID, oldPredecessor.ID, newPredecessor.ID} { + if !updatedIDs[id] { + t.Errorf("task.updated missing for %s", id) + } + } + _ = repo +} + +func TestReplaceDependencies_RejectsDuplicateWithoutChangingSet(t *testing.T) { + svc, _ := setupOfficeTest(t) + blockers := &replacementBlockerRepo{mockBlockerRepo: &mockBlockerRepo{}} + svc.SetBlockerRepository(blockers) + ctx := context.Background() + predecessor := mustSeedTask(t, svc, "Predecessor") + dependent := mustSeedTask(t, svc, "Dependent") + if err := svc.AddDependency(ctx, dependent.ID, predecessor.ID); err != nil { + t.Fatalf("AddDependency: %v", err) + } + + err := svc.ReplaceDependencies(ctx, dependent.ID, []string{predecessor.ID, predecessor.ID}) + if !errors.Is(err, ErrInvalidDependencySet) { + t.Fatalf("error = %v, want ErrInvalidDependencySet", err) + } + got, err := svc.GetBlockers(ctx, dependent.ID) + if err != nil { + t.Fatalf("GetBlockers: %v", err) + } + if len(got) != 1 || got[0] != predecessor.ID { + t.Fatalf("blockers after rejected replacement = %v, want [%s]", got, predecessor.ID) + } +} + +func TestReplaceDependencies_RejectsCycleWithoutChangingSet(t *testing.T) { + svc, _ := setupOfficeTest(t) + blockers := &replacementBlockerRepo{mockBlockerRepo: &mockBlockerRepo{}} + svc.SetBlockerRepository(blockers) + ctx := context.Background() + a := mustSeedTask(t, svc, "A") + b := mustSeedTask(t, svc, "B") + c := mustSeedTask(t, svc, "C") + if err := svc.AddDependency(ctx, a.ID, b.ID); err != nil { + t.Fatalf("AddDependency(a,b): %v", err) + } + if err := svc.AddDependency(ctx, b.ID, c.ID); err != nil { + t.Fatalf("AddDependency(b,c): %v", err) + } + + err := svc.ReplaceDependencies(ctx, c.ID, []string{a.ID}) + var cycle *CycleError + if !errors.As(err, &cycle) { + t.Fatalf("error = %v, want CycleError", err) + } + got, err := svc.GetBlockers(ctx, c.ID) + if err != nil { + t.Fatalf("GetBlockers: %v", err) + } + if len(got) != 0 { + t.Fatalf("cycle rejection changed blockers = %v", got) + } +} + +func TestReplaceDependencies_ReturnsUnavailableWithoutReplacementCapability(t *testing.T) { + svc, _ := setupOfficeTest(t) + svc.SetBlockerRepository(&mockBlockerRepo{}) + dependent := mustSeedTask(t, svc, "Dependent") + predecessor := mustSeedTask(t, svc, "Predecessor") + + err := svc.ReplaceDependencies(context.Background(), dependent.ID, []string{predecessor.ID}) + if !errors.Is(err, ErrDependencyRepositoryUnavailable) { + t.Fatalf("error = %v, want ErrDependencyRepositoryUnavailable", err) + } +} + +func TestReplaceDependencies_PreservesNotFoundAuthorizationContract(t *testing.T) { + svc, _, repo := createTestService(t) + seedForeignPair(t, repo) + svc.SetBlockerRepository(&replacementBlockerRepo{mockBlockerRepo: &mockBlockerRepo{}}) + + err := svc.ReplaceDependencies(ctxAs("user-a"), "task-b", []string{"task-b2"}) + if !errors.Is(err, taskrepo.ErrTaskNotFound) { + t.Fatalf("foreign replacement error = %v, want ErrTaskNotFound", err) + } +} diff --git a/apps/backend/internal/task/service/service_office.go b/apps/backend/internal/task/service/service_office.go index f58d8597de..d91a3d4d49 100644 --- a/apps/backend/internal/task/service/service_office.go +++ b/apps/backend/internal/task/service/service_office.go @@ -31,6 +31,40 @@ type taskDependencyCleaner interface { DeleteTaskBlockersForTask(ctx context.Context, taskID string) error } +// taskDependencyReplacer is optional so existing repository test doubles and +// integrations that only need one-edge operations keep compiling. +type taskDependencyReplacer interface { + ReplaceTaskBlockers(ctx context.Context, taskID string, blockerTaskIDs []string) error +} + +func changedDependencyIDs(existing []*orchmodels.TaskBlocker, desired []string) []string { + existingSet := make(map[string]struct{}, len(existing)) + for _, blocker := range existing { + if blocker != nil { + existingSet[blocker.BlockerTaskID] = struct{}{} + } + } + desiredSet := make(map[string]struct{}, len(desired)) + for _, blockerTaskID := range desired { + desiredSet[blockerTaskID] = struct{}{} + } + changed := make([]string, 0) + for _, blocker := range existing { + if blocker == nil { + continue + } + if _, keep := desiredSet[blocker.BlockerTaskID]; !keep { + changed = append(changed, blocker.BlockerTaskID) + } + } + for _, blockerTaskID := range desired { + if _, alreadyExists := existingSet[blockerTaskID]; !alreadyExists { + changed = append(changed, blockerTaskID) + } + } + return changed +} + // CommentRepository provides access to task comment persistence. type CommentRepository interface { CreateTaskComment(ctx context.Context, comment *orchmodels.TaskComment) error diff --git a/apps/web/components/task-create-dialog-dependencies.test.tsx b/apps/web/components/task-create-dialog-dependencies.test.tsx index fe236b56c4..448104535b 100644 --- a/apps/web/components/task-create-dialog-dependencies.test.tsx +++ b/apps/web/components/task-create-dialog-dependencies.test.tsx @@ -234,6 +234,34 @@ describe("TaskCreateDependencies", () => { }); }); +describe("selected dependency preservation", () => { + it("keeps an omitted selected predecessor titled and individually removable", () => { + const selectedID = "task-archived"; + const onChange = vi.fn(); + render( + + + , + ); + + fireEvent.click(screen.getByTestId(TRIGGER_TEST_ID)); + + const selectedOption = within( + screen.getByTestId("task-create-dependencies-popover"), + ).getByTestId(`task-create-dependency-option-${selectedID}`); + expect(selectedOption).toBeTruthy(); + expect(selectedOption.textContent).toContain("Archived predecessor"); + + fireEvent.click(selectedOption); + expect(onChange).toHaveBeenCalledWith([]); + }); +}); + describe("TaskCreateDependencies change-request search", () => { it("narrows to the PR-linked task when searching by PR number and shows its badge", () => { const prTask = task(ALPHA_ID, ALPHA_TITLE, { diff --git a/apps/web/components/task-create-dialog-dependencies.tsx b/apps/web/components/task-create-dialog-dependencies.tsx index 0a3aaa2e50..d5df0cf41d 100644 --- a/apps/web/components/task-create-dialog-dependencies.tsx +++ b/apps/web/components/task-create-dialog-dependencies.tsx @@ -24,6 +24,7 @@ import { useAppStore } from "@/components/state-provider"; import { useWorkspacePRs } from "@/hooks/domains/github/use-task-pr"; import { useWorkspaceMRs } from "@/hooks/domains/gitlab/use-task-mr"; import { useTaskCreateDialogPopoverContainer } from "@/hooks/use-task-create-dialog-popover-container"; +import type { TaskDependencyCandidate } from "@/lib/types/task-dependencies"; import type { KanbanState } from "@/lib/state/slices/kanban/types"; import type { TaskMR } from "@/lib/types/gitlab"; import type { TaskPR } from "@/lib/types/github"; @@ -49,6 +50,17 @@ export type TaskCreateDependenciesProps = { value: string[]; onChange: (next: string[]) => void; disabled?: boolean; + /** Optional server-backed candidates for the edit dialog. */ + candidates?: TaskDependencyCandidate[]; + /** Titles for selected tasks that are outside the current search page. */ + selectedTitles?: Record; + /** True while the server-backed candidate search is running. */ + candidatesLoading?: boolean; + /** Controlled search value for the server-backed candidate search. */ + searchValue?: string; + onSearchValueChange?: (value: string) => void; + /** The edited task is never a valid predecessor of itself. */ + excludeTaskId?: string; }; function useBoardTasks(): KanbanState["tasks"] { @@ -114,7 +126,7 @@ function DependencyOption({ selected, onSelect, }: { - task: KanbanState["tasks"][number]; + task: TaskDependencyCandidate; numbers: number[]; selected: boolean; onSelect: () => void; @@ -152,25 +164,33 @@ function DependencyOption({ } type DependencyPickerContentProps = { - candidates: KanbanState["tasks"]; - mrsByTaskId: Record; - prsByTaskId: Record; + candidates: TaskDependencyCandidate[]; + numbersByTaskId: ReadonlyMap; value: string[]; onChange: (next: string[]) => void; setOpen: (open: boolean) => void; portalContainer: HTMLElement | null; + candidatesLoading: boolean; + searchValue?: string; + onSearchValueChange?: (value: string) => void; }; function DependencyPickerContent({ candidates, - mrsByTaskId, - prsByTaskId, + numbersByTaskId, value, onChange, setOpen, portalContainer, + candidatesLoading, + searchValue, + onSearchValueChange, }: DependencyPickerContentProps) { const { t } = useTranslation(); + const commandInputProps = + searchValue === undefined + ? {} + : { value: searchValue, onValueChange: onSearchValueChange ?? (() => undefined) }; return (
- +
@@ -203,7 +228,9 @@ function DependencyPickerContent({
- {t("task:noTasksFound")} + + {candidatesLoading ? t("tasks:loadingTasks") : t("task:noTasksFound")} + { onChange( @@ -249,7 +272,108 @@ function DependencyPickerContent({ ); } -export function TaskCreateDependencies({ value, onChange, disabled }: TaskCreateDependenciesProps) { +type DependencyPickerDataArgs = { + boardTasks: KanbanState["tasks"]; + providedCandidates?: TaskDependencyCandidate[]; + selectedTitles?: Record; + value: string[]; + excludeTaskId?: string; + mrsByTaskId: Record; + prsByTaskId: Record; +}; + +type DependencyPickerData = { + candidates: TaskDependencyCandidate[]; + numbersByTaskId: ReadonlyMap; + selectedTitle?: string; + selectedNumber?: number; +}; + +function useDependencyPickerData({ + boardTasks, + providedCandidates, + selectedTitles, + value, + excludeTaskId, + mrsByTaskId, + prsByTaskId, +}: DependencyPickerDataArgs): DependencyPickerData { + const tasks = useMemo(() => { + const byID = new Map(); + for (const task of providedCandidates ?? boardTasks) byID.set(task.id, task); + for (const id of value) { + const title = selectedTitles?.[id]; + if (title !== undefined && !byID.has(id)) { + byID.set(id, { id, title, isArchived: true }); + } + } + return [...byID.values()]; + }, [boardTasks, providedCandidates, selectedTitles, value]); + + const boardTasksById = useMemo( + () => new Map(boardTasks.map((task) => [task.id, task] as const)), + [boardTasks], + ); + const numbersByTaskId = useMemo( + () => + new Map( + tasks.map((task) => { + const boardTask = boardTasksById.get(task.id); + return [ + task.id, + boardTask + ? changeRequestNumbers( + boardTask, + mrsByTaskId[task.id] ?? NO_MRS, + prsByTaskId[task.id] ?? NO_PRS, + ) + : [], + ] as const; + }), + ), + [boardTasksById, mrsByTaskId, prsByTaskId, tasks], + ); + + const selectedTask = useMemo(() => { + if (value.length !== 1) return undefined; + return tasks.find((task) => task.id === value[0]); + }, [tasks, value]); + const selectedTitle = useMemo(() => { + if (!selectedTask) return undefined; + return selectedTitles?.[selectedTask.id] ?? selectedTask.title; + }, [selectedTask, selectedTitles]); + const selectedNumber = useMemo( + () => (selectedTask ? numbersByTaskId.get(selectedTask.id)?.[0] : undefined), + [numbersByTaskId, selectedTask], + ); + const candidates = useMemo( + () => + tasks + .filter( + (task) => task.id !== excludeTaskId && (!task.isArchived || value.includes(task.id)), + ) + .sort((left, right) => { + const leftTask = boardTasksById.get(left.id) ?? left; + const rightTask = boardTasksById.get(right.id) ?? right; + return compareDependencyCandidates(leftTask, rightTask); + }), + [boardTasksById, excludeTaskId, tasks, value], + ); + + return { candidates, numbersByTaskId, selectedTitle, selectedNumber }; +} + +export function TaskCreateDependencies({ + value, + onChange, + disabled, + candidates: providedCandidates, + selectedTitles, + candidatesLoading = false, + searchValue, + onSearchValueChange, + excludeTaskId, +}: TaskCreateDependenciesProps) { const { t } = useTranslation(); const [open, setOpen] = useState(false); const portalContainer = useTaskCreateDialogPopoverContainer(); @@ -258,30 +382,22 @@ export function TaskCreateDependencies({ value, onChange, disabled }: TaskCreate // routes, which normally hydrate these association stores. useWorkspacePRs(activeWorkspaceId); useWorkspaceMRs(activeWorkspaceId); - const tasks = useBoardTasks(); + const boardTasks = useBoardTasks(); const mrsByTaskId = useMRsByTaskId(); const prsByTaskId = usePRsByTaskId(); - - const selectedTask = useMemo(() => { - if (value.length !== 1) return undefined; - return tasks.find((task) => task.id === value[0]); - }, [tasks, value]); - const selectedNumber = useMemo(() => { - if (!selectedTask) return undefined; - return changeRequestNumbers( - selectedTask, - mrsByTaskId[selectedTask.id] ?? NO_MRS, - prsByTaskId[selectedTask.id] ?? NO_PRS, - )[0]; - }, [selectedTask, mrsByTaskId, prsByTaskId]); - const candidates = useMemo( - () => tasks.filter((task) => !task.isArchived).sort(compareDependencyCandidates), - [tasks], - ); + const { candidates, numbersByTaskId, selectedTitle, selectedNumber } = useDependencyPickerData({ + boardTasks, + providedCandidates, + selectedTitles, + value, + excludeTaskId, + mrsByTaskId, + prsByTaskId, + }); const triggerLabel = ( @@ -320,12 +436,14 @@ export function TaskCreateDependencies({ value, onChange, disabled }: TaskCreate diff --git a/apps/web/components/task-create-dialog-dependency-search.ts b/apps/web/components/task-create-dialog-dependency-search.ts index 36c71a2dde..88adfdd9a2 100644 --- a/apps/web/components/task-create-dialog-dependency-search.ts +++ b/apps/web/components/task-create-dialog-dependency-search.ts @@ -20,24 +20,27 @@ export function changeRequestNumbers( } /** cmdk search key: title, id, and both '#N' and bare 'N' for each change-request number. */ -export function dependencyOptionValue(task: Task, numbers: number[]): string { +export function dependencyOptionValue(task: Pick, numbers: number[]): string { const parts = [task.title, task.id, ...numbers.map((n) => `#${n} ${n}`)]; return parts.join(" "); } -function timestampMs(task: Task): number | undefined { +function timestampMs(task: Pick): number | undefined { const updated = task.updatedAt ? Date.parse(task.updatedAt) : NaN; if (Number.isFinite(updated)) return updated; return createdAtMs(task); } -function createdAtMs(task: Task): number | undefined { +function createdAtMs(task: Pick): number | undefined { const created = task.createdAt ? Date.parse(task.createdAt) : NaN; return Number.isFinite(created) ? created : undefined; } /** Most-recently-updated first, using createdAt and then title as tie-breakers. */ -export function compareDependencyCandidates(a: Task, b: Task): number { +export function compareDependencyCandidates( + a: Pick, + b: Pick, +): number { const aTime = timestampMs(a); const bTime = timestampMs(b); if (aTime !== undefined && bTime !== undefined && aTime !== bTime) return bTime - aTime; diff --git a/apps/web/components/task-create-dialog-footer.test.ts b/apps/web/components/task-create-dialog-footer.test.ts index 8be6fac153..c278a8de31 100644 --- a/apps/web/components/task-create-dialog-footer.test.ts +++ b/apps/web/components/task-create-dialog-footer.test.ts @@ -12,6 +12,7 @@ import { REASON_AGENT, REASON_DESCRIPTION, REASON_PROMPT, + REASON_LOADING_DEPENDENCIES, } from "./task-create-dialog-footer"; import { t } from "@/lib/i18n"; import type { ButtonKind, TaskCreateDialogFooterProps } from "./task-create-dialog-footer"; @@ -60,6 +61,14 @@ describe("isNativeSubmitDisabled", () => { it("allows a complete native submission", () => { expect(isNativeSubmitDisabled(makeProps())).toBe(false); }); + + it("blocks edit submission until dependencies are ready", () => { + expect( + isNativeSubmitDisabled( + makeProps({ isCreateMode: false, isEditMode: true, editDependenciesReady: false }), + ), + ).toBe(true); + }); }); describe("computeDisabledReason (start-task)", () => { @@ -183,6 +192,19 @@ describe("computeDisabledReason (update)", () => { ), ).toBeNull(); }); + + it("explains why edit update waits for dependencies", () => { + expect( + computeDisabledReason( + makeProps({ + isCreateMode: false, + isEditMode: true, + editDependenciesReady: false, + }), + KIND_UPDATE, + ), + ).toBe(REASON_LOADING_DEPENDENCIES); + }); }); describe("computeDisabledReason (default)", () => { diff --git a/apps/web/components/task-create-dialog-footer.tsx b/apps/web/components/task-create-dialog-footer.tsx index 2fdd009495..2b70ab498d 100644 --- a/apps/web/components/task-create-dialog-footer.tsx +++ b/apps/web/components/task-create-dialog-footer.tsx @@ -23,17 +23,23 @@ import { useTranslation } from "react-i18next"; type UpdateButtonProps = { isCreatingTask: boolean; hasTitle: boolean; + editDependenciesReady?: boolean; onUpdate: () => void; }; -function UpdateButton({ isCreatingTask, hasTitle, onUpdate }: UpdateButtonProps) { +function UpdateButton({ + isCreatingTask, + hasTitle, + editDependenciesReady, + onUpdate, +}: UpdateButtonProps) { const { t } = useTranslation(); return ( + + ); + } else { + dependencyField = ( + + ); + } + + return ( +
+
+ {t("task:dependsOn")} + + + + + + {t("task:dependencyInfo")} + + +
+ {dependencyField} + {Boolean(candidateError) && !loadError && ( +
+

+ {t("task:failedToLoadDependencyCandidates")} +

+ +
+ )} + {Boolean(saveError) && ( +

+ {dependencySaveError(saveError, t)} +

+ )} +
+ ); +} diff --git a/apps/web/e2e/helpers/api-client.ts b/apps/web/e2e/helpers/api-client.ts index 1c13386aef..367bfe56d6 100644 --- a/apps/web/e2e/helpers/api-client.ts +++ b/apps/web/e2e/helpers/api-client.ts @@ -2342,6 +2342,23 @@ export class ApiClient { }); } + /** Replace the complete predecessor set, including an empty set. */ + async replaceTaskDependencies( + taskId: string, + dependsOnTaskIds: string[], + ): Promise { + return this.request("PUT", `/api/v1/tasks/${taskId}/dependencies`, { + depends_on_task_ids: dependsOnTaskIds, + }); + } + + /** Raw replacement helper for asserting structured validation failures. */ + async rawReplaceTaskDependencies(taskId: string, dependsOnTaskIds: string[]): Promise { + return this.rawRequest("PUT", `/api/v1/tasks/${taskId}/dependencies`, { + depends_on_task_ids: dependsOnTaskIds, + }); + } + /** * Raw add, for asserting the rejection path. A cycle answers 409 with a * `cycle` array; the typed helper above would throw the body away. diff --git a/apps/web/e2e/tests/task/create-task-dependency-selector.spec.ts b/apps/web/e2e/tests/task/create-task-dependency-selector.spec.ts index 9562ad16e7..c19bfd4507 100644 --- a/apps/web/e2e/tests/task/create-task-dependency-selector.spec.ts +++ b/apps/web/e2e/tests/task/create-task-dependency-selector.spec.ts @@ -174,6 +174,90 @@ test.describe("Task-create dependency selector", () => { } }); + test("edits a started task's predecessor set and keeps cancel draft-only", async ({ + testPage, + apiClient, + seedData, + prCapture, + }) => { + const taskOptions = { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }; + const alpha = await apiClient.createTask( + seedData.workspaceId, + "Edit dependency Alpha", + taskOptions, + ); + const beta = await apiClient.createTask( + seedData.workspaceId, + "Edit dependency Beta", + taskOptions, + ); + const archived = await apiClient.createTask( + seedData.workspaceId, + "Edit archived dependency", + taskOptions, + ); + await apiClient.archiveTask(archived.id); + const target = await apiClient.createTask(seedData.workspaceId, "Edit dependency target", { + ...taskOptions, + description: "The started task prompt remains unchanged.", + blocked_by: [alpha.id], + }); + await apiClient.updateTaskState(target.id, "IN_PROGRESS"); + + const kanban = new KanbanPage(testPage); + await kanban.goto(); + await kanban.openTaskActionsMenu(target.id); + await testPage.getByRole("menuitem", { name: "Edit", exact: true }).click(); + + const dialog = testPage.getByTestId("create-task-dialog"); + await expect(dialog.getByTestId("task-edit-dependencies")).toBeVisible(); + await prCapture.screenshot("desktop-task-dependency-edit-dialog", { + caption: "Desktop task edit dialog with predecessor dependencies", + }); + const dependency = dialog.getByTestId(DEPENDENCY_TRIGGER); + await expect(dependency).toContainText("Edit dependency Alpha"); + await dependency.click(); + const picker = dialog.getByTestId("task-create-dependencies-popover"); + const search = picker.getByPlaceholder("Search tasks..."); + await search.fill("Edit dependency Beta"); + await expect(picker.getByTestId(`${DEPENDENCY_OPTION_PREFIX}${beta.id}`)).toBeVisible(); + await picker.getByTestId(`${DEPENDENCY_OPTION_PREFIX}${beta.id}`).click(); + await expect(dependency).toContainText("2 dependencies"); + + await testPage.keyboard.press("Escape"); + await dialog.getByRole("button", { name: "Cancel", exact: true }).click(); + await expect(dialog).toHaveCount(0); + expect( + (await apiClient.getTaskDependencies(target.id)).depends_on?.map((ref) => ref.id), + ).toEqual([alpha.id]); + + await kanban.openTaskActionsMenu(target.id); + await testPage.getByRole("menuitem", { name: "Edit", exact: true }).click(); + const reopened = testPage.getByTestId("create-task-dialog"); + await expect(reopened.getByTestId("task-edit-dependencies")).toBeVisible(); + const reopenedDependency = reopened.getByTestId(DEPENDENCY_TRIGGER); + await reopenedDependency.click(); + const reopenedPicker = reopened.getByTestId("task-create-dependencies-popover"); + await reopenedPicker.getByPlaceholder("Search tasks...").fill(""); + await expect( + reopenedPicker.getByTestId(`${DEPENDENCY_OPTION_PREFIX}${archived.id}`), + ).toHaveCount(0); + await reopenedPicker.getByPlaceholder("Search tasks...").fill("Edit dependency Beta"); + await reopenedPicker.getByTestId(`${DEPENDENCY_OPTION_PREFIX}${beta.id}`).click(); + await testPage.keyboard.press("Escape"); + await reopened.getByRole("button", { name: "Update", exact: true }).click(); + await expect(reopened).toHaveCount(0); + await expect + .poll(async () => + (await apiClient.getTaskDependencies(target.id)).depends_on?.map((ref) => ref.id), + ) + .toEqual(expect.arrayContaining([alpha.id, beta.id])); + }); + test("finds a candidate by its linked GitHub PR number and shows the badge", async ({ testPage, apiClient, diff --git a/apps/web/e2e/tests/task/mobile-create-task-dependency-selector.spec.ts b/apps/web/e2e/tests/task/mobile-create-task-dependency-selector.spec.ts index cf6cea84fb..b75f177400 100644 --- a/apps/web/e2e/tests/task/mobile-create-task-dependency-selector.spec.ts +++ b/apps/web/e2e/tests/task/mobile-create-task-dependency-selector.spec.ts @@ -111,4 +111,91 @@ test.describe("Create task dependency selector on mobile", () => { await expect(trigger).toContainText("#188 · Mobile dependency predecessor"); await assertNoDocumentHorizontalOverflow(testPage, "mobile dependency selection"); }); + + test("edits predecessors from the same touch-friendly dialog", async ({ + testPage, + apiClient, + seedData, + prCapture, + }) => { + const taskOptions = { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }; + const predecessor = await apiClient.createTask( + seedData.workspaceId, + "Mobile edit dependency Alpha", + taskOptions, + ); + const replacement = await apiClient.createTask( + seedData.workspaceId, + "Mobile edit dependency Beta", + taskOptions, + ); + const target = await apiClient.createTask( + seedData.workspaceId, + "Mobile edit dependency target", + { + ...taskOptions, + description: "Mobile edit prompt", + blocked_by: [predecessor.id], + }, + ); + await apiClient.updateTaskState(target.id, "IN_PROGRESS"); + + const mobile = new MobileKanbanPage(testPage); + await mobile.goto(); + await mobile.taskCard(target.id).getByRole("button", { name: "More options" }).tap(); + await testPage + .locator('[data-slot="dropdown-menu-content"]:visible') + .getByRole("menuitem", { name: "Edit", exact: true }) + .tap(); + + const dialog = testPage.getByTestId("create-task-dialog"); + const editDependencies = dialog.getByTestId("task-edit-dependencies"); + await expect(editDependencies).toBeVisible(); + const trigger = dialog.getByTestId("task-create-dependencies-trigger"); + await expect(trigger).toBeVisible({ timeout: 30_000 }); + const triggerBox = await trigger.boundingBox(); + expect(triggerBox).not.toBeNull(); + expect(triggerBox!.height).toBeGreaterThanOrEqual(44); + await assertLocatorWithinViewportX(trigger, "mobile edit dependency trigger"); + await assertNoElementHorizontalOverflow(trigger, "mobile edit dependency trigger"); + await expect(trigger).toContainText("Mobile edit dependency Alpha"); + + await trigger.tap(); + const picker = dialog.getByTestId("task-create-dependencies-popover"); + await expect(picker).toBeVisible(); + await assertLocatorWithinViewportX(picker, "mobile edit dependency picker"); + await assertNoElementHorizontalOverflow(picker, "mobile edit dependency picker"); + await assertNoDocumentHorizontalOverflow(testPage, "mobile edit dependency picker"); + const info = picker.getByTestId("task-create-dependency-info"); + const infoBox = await info.boundingBox(); + expect(infoBox).not.toBeNull(); + expect(infoBox!.height).toBeGreaterThanOrEqual(44); + expect(infoBox!.width).toBeGreaterThanOrEqual(44); + + const search = picker.getByPlaceholder("Search tasks..."); + await search.fill("Mobile edit dependency Beta"); + const option = picker.getByTestId(`task-create-dependency-option-${replacement.id}`); + await expect(option).toBeVisible(); + const optionBox = await option.boundingBox(); + expect(optionBox).not.toBeNull(); + expect(optionBox!.height).toBeGreaterThanOrEqual(44); + await prCapture.screenshot("mobile-task-dependency-picker", { + caption: "Mobile task edit dialog predecessor picker", + }); + await option.tap(); + await expect(trigger).toContainText("2 dependencies"); + await testPage.keyboard.press("Escape"); + await dialog.getByRole("button", { name: "Update", exact: true }).tap(); + await expect(dialog).toHaveCount(0); + await expect + .poll(async () => + (await apiClient.getTaskDependencies(target.id)).depends_on?.map((ref) => ref.id), + ) + .toEqual(expect.arrayContaining([predecessor.id, replacement.id])); + await assertNoDocumentHorizontalOverflow(testPage, "mobile edit dependency saved"); + }); }); diff --git a/apps/web/hooks/domains/task/use-task-edit-dialog-dependencies.test.ts b/apps/web/hooks/domains/task/use-task-edit-dialog-dependencies.test.ts new file mode 100644 index 0000000000..805d350d1f --- /dev/null +++ b/apps/web/hooks/domains/task/use-task-edit-dialog-dependencies.test.ts @@ -0,0 +1,350 @@ +import { act, renderHook, waitFor } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { ApiError } from "@/lib/api/client"; +import { + getTaskDependencies, + replaceTaskDependencies, +} from "@/lib/api/domains/task-dependencies-api"; +import { listTasksByWorkspace } from "@/lib/api/domains/kanban-api"; +import { + isTaskDependencyUpdateFailure, + useTaskEditDialogDependencies, +} from "./use-task-edit-dialog-dependencies"; + +vi.mock("@/lib/api/domains/task-dependencies-api", () => ({ + getTaskDependencies: vi.fn(), + replaceTaskDependencies: vi.fn(), +})); + +vi.mock("@/lib/api/domains/kanban-api", () => ({ + listTasksByWorkspace: vi.fn(), +})); + +const dependency = (id: string, title: string) => ({ id, title, state: "TODO" as const }); + +function deferred() { + let resolve!: (value: T) => void; + let reject!: (error: unknown) => void; + const promise = new Promise((resolvePromise, rejectPromise) => { + resolve = resolvePromise; + reject = rejectPromise; + }); + return { promise, resolve, reject }; +} + +beforeEach(() => { + vi.mocked(getTaskDependencies).mockReset(); + vi.mocked(replaceTaskDependencies).mockReset(); + vi.mocked(listTasksByWorkspace).mockReset(); + vi.mocked(getTaskDependencies).mockResolvedValue({ + id: "task-1", + depends_on: [dependency("task-2", "Predecessor")], + }); + vi.mocked(listTasksByWorkspace).mockResolvedValue({ + total: 3, + tasks: [ + { id: "task-1", title: "Edited task", archived_at: null }, + { id: "task-2", title: "Predecessor", archived_at: null }, + { id: "task-archived", title: "Archived", archived_at: "2026-01-01" }, + ], + } as never); + vi.mocked(replaceTaskDependencies).mockResolvedValue({ task_id: "task-1" }); +}); + +describe("useTaskEditDialogDependencies", () => { + it("loads current predecessors and filters the edited and archived tasks", async () => { + const { result } = renderHook(() => + useTaskEditDialogDependencies({ open: true, workspaceId: "ws-1", taskId: "task-1" }), + ); + + await waitFor(() => expect(result.current.loading).toBe(false)); + + expect(result.current.confirmedIds).toEqual(["task-2"]); + expect(result.current.draftIds).toEqual(["task-2"]); + expect(result.current.selectedTitles).toEqual({ "task-2": "Predecessor" }); + expect(result.current.candidates.map((task) => task.id)).toEqual(["task-2"]); + expect(listTasksByWorkspace).toHaveBeenCalledWith( + "ws-1", + { + page: 1, + pageSize: 100, + query: "", + }, + { + init: { signal: expect.any(AbortSignal) }, + }, + ); + }); + + it("loads every candidate page", async () => { + const firstPage = Array.from({ length: 100 }, (_, index) => ({ + id: `task-${index + 1}`, + title: `Task ${index + 1}`, + archived_at: null, + })); + vi.mocked(listTasksByWorkspace).mockImplementation(async (_workspaceId, params) => { + if (params?.page === 1) return { total: 101, tasks: firstPage } as never; + return { + total: 101, + tasks: [{ id: "task-101", title: "Task 101", archived_at: null }], + } as never; + }); + + const { result } = renderHook(() => + useTaskEditDialogDependencies({ open: true, workspaceId: "ws-1", taskId: "task-1" }), + ); + + await waitFor(() => expect(result.current.candidates).toHaveLength(100)); + + expect(result.current.candidates.map((task) => task.id)).toContain("task-101"); + expect(listTasksByWorkspace).toHaveBeenNthCalledWith( + 2, + "ws-1", + { + page: 2, + pageSize: 100, + query: "", + }, + { + init: { signal: expect.any(AbortSignal) }, + }, + ); + }); +}); + +describe("candidate loading cancellation", () => { + it("does not request later pages when a candidate query is superseded", async () => { + const oldFirstPage = deferred<{ + total: number; + tasks: Array<{ id: string; title: string; archived_at: null }>; + }>(); + vi.mocked(listTasksByWorkspace).mockImplementation(async (_workspaceId, params) => { + if (params?.query === "old" && params.page === 1) return oldFirstPage.promise as never; + if (params?.query === "old") { + return { + total: 200, + tasks: Array.from({ length: 100 }, (_, index) => ({ + id: `old-page-${index}`, + title: `Old page ${index}`, + archived_at: null, + })), + } as never; + } + if (params?.query === "new") { + return { + total: 1, + tasks: [{ id: "task-new", title: "New result", archived_at: null }], + } as never; + } + return { total: 0, tasks: [] } as never; + }); + + const { result } = renderHook(() => + useTaskEditDialogDependencies({ open: true, workspaceId: "ws-1", taskId: "task-1" }), + ); + await waitFor(() => + expect( + vi.mocked(listTasksByWorkspace).mock.calls.some(([, params]) => params?.query === ""), + ).toBe(true), + ); + + act(() => result.current.setQuery("old")); + await waitFor(() => + expect( + vi.mocked(listTasksByWorkspace).mock.calls.some(([, params]) => params?.query === "old"), + ).toBe(true), + ); + const oldFirstPageCall = vi + .mocked(listTasksByWorkspace) + .mock.calls.find(([, params]) => params?.query === "old" && params.page === 1); + expect(oldFirstPageCall?.[2]?.init?.signal).toBeInstanceOf(AbortSignal); + + act(() => result.current.setQuery("new")); + expect(oldFirstPageCall?.[2]?.init?.signal?.aborted).toBe(true); + await waitFor(() => + expect(result.current.candidates).toEqual([ + { id: "task-new", title: "New result", isArchived: false }, + ]), + ); + + oldFirstPage.resolve({ + total: 200, + tasks: Array.from({ length: 100 }, (_, index) => ({ + id: `old-${index}`, + title: `Old ${index}`, + archived_at: null, + })), + }); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + + expect( + vi + .mocked(listTasksByWorkspace) + .mock.calls.filter(([, params]) => params?.query === "old" && (params.page ?? 0) > 1), + ).toHaveLength(0); + }); +}); + +describe("useTaskEditDialogDependencies updates", () => { + it("saves only draft changes and updates the confirmed set after success", async () => { + const { result } = renderHook(() => + useTaskEditDialogDependencies({ open: true, workspaceId: "ws-1", taskId: "task-1" }), + ); + await waitFor(() => expect(result.current.loading).toBe(false)); + + act(() => result.current.setDraftIds([])); + expect(replaceTaskDependencies).not.toHaveBeenCalled(); + expect(result.current.isDirty).toBe(true); + + await act(async () => result.current.save()); + + expect(replaceTaskDependencies).toHaveBeenCalledWith("task-1", []); + expect(result.current.confirmedIds).toEqual([]); + expect(result.current.isDirty).toBe(false); + }); + + it("keeps the draft and exposes the original error when replacement fails", async () => { + const apiError = new ApiError("would create a dependency cycle", 409, { + error: "would create a dependency cycle", + cycle: ["task-1", "task-2", "task-1"], + }); + vi.mocked(replaceTaskDependencies).mockRejectedValueOnce(apiError); + const { result } = renderHook(() => + useTaskEditDialogDependencies({ open: true, workspaceId: "ws-1", taskId: "task-1" }), + ); + await waitFor(() => expect(result.current.loading).toBe(false)); + + act(() => result.current.setDraftIds(["task-2", "task-3"])); + let failure: unknown; + await act(async () => { + try { + await result.current.save(); + } catch (error: unknown) { + failure = error; + } + }); + + expect(isTaskDependencyUpdateFailure(failure)).toBe(true); + expect(result.current.confirmedIds).toEqual(["task-2"]); + expect(result.current.draftIds).toEqual(["task-2", "task-3"]); + expect(result.current.error).toBe(apiError); + expect(isTaskDependencyUpdateFailure(result.current.submitError)).toBe(true); + }); + + it("retains a newly selected title when a later candidate result omits it", async () => { + vi.mocked(listTasksByWorkspace).mockImplementation(async (_workspaceId, params) => { + if (params?.query === "later") { + return { + total: 1, + tasks: [{ id: "task-b", title: "Task B", archived_at: null }], + } as never; + } + return { + total: 1, + tasks: [{ id: "task-a", title: "Task A", archived_at: null }], + } as never; + }); + + const { result } = renderHook(() => + useTaskEditDialogDependencies({ open: true, workspaceId: "ws-1", taskId: "task-1" }), + ); + await waitFor(() => + expect(result.current.candidates).toEqual([ + { id: "task-a", title: "Task A", isArchived: false }, + ]), + ); + + act(() => { + result.current.setDraftIds(["task-a"]); + result.current.setQuery("later"); + }); + await waitFor(() => + expect(result.current.candidates).toEqual([ + { id: "task-b", title: "Task B", isArchived: false }, + ]), + ); + + expect(result.current.draftIds).toEqual(["task-a"]); + expect(result.current.selectedTitles).toMatchObject({ "task-a": "Task A" }); + }); +}); + +describe("useTaskEditDialogDependencies failure handling", () => { + it("keeps the editor ready when candidate search fails", async () => { + vi.mocked(listTasksByWorkspace).mockRejectedValueOnce(new Error("candidate search failed")); + const { result } = renderHook(() => + useTaskEditDialogDependencies({ open: true, workspaceId: "ws-1", taskId: "task-1" }), + ); + + await waitFor(() => expect(result.current.candidateError).not.toBeNull()); + + expect(result.current.loading).toBe(false); + expect(result.current.ready).toBe(true); + }); + + it("retries candidates without reloading the projection or losing the draft", async () => { + vi.mocked(listTasksByWorkspace).mockRejectedValueOnce(new Error("candidate search failed")); + const { result } = renderHook(() => + useTaskEditDialogDependencies({ open: true, workspaceId: "ws-1", taskId: "task-1" }), + ); + + await waitFor(() => expect(result.current.candidateError).not.toBeNull()); + act(() => result.current.setDraftIds(["task-3"])); + vi.mocked(listTasksByWorkspace).mockResolvedValueOnce({ + total: 1, + tasks: [{ id: "task-3", title: "Recovered candidate", archived_at: null }], + } as never); + + act(() => result.current.retryCandidates()); + await waitFor(() => + expect(result.current.candidates).toEqual([ + { id: "task-3", title: "Recovered candidate", isArchived: false }, + ]), + ); + + expect(result.current.draftIds).toEqual(["task-3"]); + expect(getTaskDependencies).toHaveBeenCalledOnce(); + }); + + it("ignores a save result from the previous task after the dialog changes identity", async () => { + const saveRequest = deferred<{ task_id: string }>(); + vi.mocked(replaceTaskDependencies).mockReturnValue(saveRequest.promise as never); + const { result, rerender } = renderHook( + ({ open, workspaceId, taskId }: { open: boolean; workspaceId: string; taskId: string }) => + useTaskEditDialogDependencies({ open, workspaceId, taskId }), + { + initialProps: { open: true, workspaceId: "ws-1", taskId: "task-1" }, + }, + ); + await waitFor(() => expect(result.current.loading).toBe(false)); + + act(() => result.current.setDraftIds(["task-3"])); + let savePromise!: Promise; + act(() => { + savePromise = result.current.save(); + }); + + vi.mocked(getTaskDependencies).mockResolvedValueOnce({ + id: "task-2", + depends_on: [dependency("task-4", "New predecessor")], + }); + vi.mocked(listTasksByWorkspace).mockResolvedValueOnce({ + total: 1, + tasks: [{ id: "task-4", title: "New predecessor", archived_at: null }], + } as never); + rerender({ open: true, workspaceId: "ws-2", taskId: "task-2" }); + await waitFor(() => expect(result.current.draftIds).toEqual(["task-4"])); + + await act(async () => { + saveRequest.resolve({ task_id: "task-1" }); + await savePromise; + }); + + expect(result.current.confirmedIds).toEqual(["task-4"]); + expect(result.current.draftIds).toEqual(["task-4"]); + expect(result.current.saveError).toBeNull(); + }); +}); diff --git a/apps/web/hooks/domains/task/use-task-edit-dialog-dependencies.ts b/apps/web/hooks/domains/task/use-task-edit-dialog-dependencies.ts new file mode 100644 index 0000000000..13e1a2d824 --- /dev/null +++ b/apps/web/hooks/domains/task/use-task-edit-dialog-dependencies.ts @@ -0,0 +1,424 @@ +"use client"; + +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { listTasksByWorkspace } from "@/lib/api/domains/kanban-api"; +import { + getTaskDependencies, + replaceTaskDependencies, + type TaskDependencyProjectionResponse, +} from "@/lib/api/domains/task-dependencies-api"; +import type { TaskDependencyCandidate } from "@/lib/types/task-dependencies"; + +export type TaskDependencyUpdateFailure = { + readonly dependencyUpdate: true; + readonly cause: unknown; +}; + +export function isTaskDependencyUpdateFailure( + error: unknown, +): error is TaskDependencyUpdateFailure { + return ( + typeof error === "object" && + error !== null && + "dependencyUpdate" in error && + (error as { dependencyUpdate?: unknown }).dependencyUpdate === true + ); +} + +export type TaskEditDialogDependenciesState = { + taskId: string | null; + confirmedIds: string[]; + draftIds: string[]; + setDraftIds: (ids: string[]) => void; + selectedTitles: Record; + candidates: TaskDependencyCandidate[]; + candidatesLoading: boolean; + query: string; + setQuery: (query: string) => void; + loading: boolean; + loadError: unknown | null; + candidateError: unknown | null; + saveError: unknown | null; + error: unknown | null; + submitError: TaskDependencyUpdateFailure | null; + ready: boolean; + isDirty: boolean; + save: () => Promise; + retry: () => void; + retryCandidates: () => void; +}; + +type TaskEditDialogDependenciesArgs = { + open: boolean; + workspaceId: string | null | undefined; + taskId: string | null | undefined; +}; + +type DependencyRef = NonNullable[number]; + +type DependencyProjectionState = { + confirmedIds: string[]; + setConfirmedIds: (ids: string[]) => void; + draftIds: string[]; + setDraftIds: (ids: string[]) => void; + knownTitles: Record; + loading: boolean; + loadError: unknown | null; +}; + +type DependencyCandidateState = { + candidates: TaskDependencyCandidate[]; + candidatesLoading: boolean; + candidateError: unknown | null; +}; + +type DependencyTitleCacheState = { + identity: string; + titles: Record; +}; + +type DependencySaveRequest = { + identity: string; + generation: number; +}; + +function uniqueIDs(ids: string[]): string[] { + return [...new Set(ids)]; +} + +function sameIDs(left: string[], right: string[]): boolean { + return left.length === right.length && left.every((id) => right.includes(id)); +} + +function dependencyRefs(response: TaskDependencyProjectionResponse): DependencyRef[] { + return response.depends_on ?? []; +} + +function mapCandidates( + tasks: Array<{ id: string; title: string; archived_at?: string | null }>, + taskId: string, +): TaskDependencyCandidate[] { + return tasks + .filter((task) => task.id !== taskId && task.archived_at == null) + .map((task) => ({ id: task.id, title: task.title, isArchived: false })) + .sort((left, right) => left.title.localeCompare(right.title)); +} + +function titlesFromRefs(refs: DependencyRef[]): Record { + return Object.fromEntries( + refs + .filter((ref): ref is DependencyRef & { title: string } => typeof ref.title === "string") + .map((ref) => [ref.id, ref.title]), + ); +} + +function useDependencySaveGuard(identity: string) { + const identityRef = useRef(identity); + const generationRef = useRef(0); + if (identityRef.current !== identity) { + identityRef.current = identity; + generationRef.current += 1; + } + + const begin = useCallback((): DependencySaveRequest => { + const generation = generationRef.current + 1; + generationRef.current = generation; + return { identity: identityRef.current, generation }; + }, []); + const isCurrent = useCallback((request: DependencySaveRequest) => { + return identityRef.current === request.identity && generationRef.current === request.generation; + }, []); + + return { begin, isCurrent }; +} + +function useTaskDependencyProjection( + { open, workspaceId, taskId }: TaskEditDialogDependenciesArgs, + reloadToken: number, +): DependencyProjectionState { + const [confirmedIds, setConfirmedIds] = useState([]); + const [draftIds, setDraftIds] = useState([]); + const [knownTitles, setKnownTitles] = useState>({}); + const [loading, setLoading] = useState(false); + const [loadError, setLoadError] = useState(null); + + useEffect(() => { + let cancelled = false; + if (!open || !workspaceId || !taskId) { + setConfirmedIds([]); + setDraftIds([]); + setKnownTitles({}); + setLoading(false); + setLoadError(null); + return () => { + cancelled = true; + }; + } + + setLoading(true); + setLoadError(null); + void getTaskDependencies(taskId) + .then((response) => { + if (cancelled) return; + const refs = dependencyRefs(response); + const ids = refs.map((ref) => ref.id); + setConfirmedIds(ids); + setDraftIds(ids); + setKnownTitles(titlesFromRefs(refs)); + }) + .catch((error: unknown) => { + if (!cancelled) setLoadError(error); + }) + .finally(() => { + if (!cancelled) setLoading(false); + }); + + return () => { + cancelled = true; + }; + }, [open, reloadToken, taskId, workspaceId]); + + return { + confirmedIds, + setConfirmedIds, + draftIds, + setDraftIds, + knownTitles, + loading, + loadError, + }; +} + +function useTaskDependencyCandidates( + { open, workspaceId, taskId, query }: TaskEditDialogDependenciesArgs & { query: string }, + reloadToken: number, +): DependencyCandidateState { + const [candidates, setCandidates] = useState([]); + const [candidatesLoading, setCandidatesLoading] = useState(false); + const [candidateError, setCandidateError] = useState(null); + + useEffect(() => { + let cancelled = false; + if (!open || !workspaceId || !taskId) { + setCandidates([]); + setCandidatesLoading(false); + setCandidateError(null); + return () => { + cancelled = true; + }; + } + + const controller = new AbortController(); + setCandidatesLoading(true); + setCandidateError(null); + + const loadCandidates = () => { + void listAllDependencyCandidates(workspaceId, query, controller.signal) + .then((tasks) => { + if (!cancelled && !controller.signal.aborted) { + setCandidates(mapCandidates(tasks, taskId)); + } + }) + .catch((error: unknown) => { + if (!cancelled && !controller.signal.aborted) setCandidateError(error); + }) + .finally(() => { + if (!cancelled && !controller.signal.aborted) setCandidatesLoading(false); + }); + }; + const debounceTimer = + query.length > 0 ? window.setTimeout(loadCandidates, dependencySearchDebounceMs) : undefined; + if (debounceTimer === undefined) loadCandidates(); + + return () => { + cancelled = true; + controller.abort(); + if (debounceTimer !== undefined) window.clearTimeout(debounceTimer); + }; + }, [open, query, reloadToken, taskId, workspaceId]); + + return { candidates, candidatesLoading, candidateError }; +} + +const dependencyCandidatePageSize = 100; +const dependencySearchDebounceMs = 200; + +async function listAllDependencyCandidates( + workspaceId: string, + query: string, + signal: AbortSignal, +): Promise> { + const tasks: Array<{ id: string; title: string; archived_at?: string | null }> = []; + let page = 1; + let total = 0; + do { + if (signal.aborted) return tasks; + const response = await listTasksByWorkspace( + workspaceId, + { + page, + pageSize: dependencyCandidatePageSize, + query, + }, + { + init: { signal }, + }, + ); + if (signal.aborted) return tasks; + tasks.push(...response.tasks); + total = response.total; + if (response.tasks.length === 0) break; + page += 1; + } while (tasks.length < total); + return tasks; +} + +function useDependencyTitleCache( + identity: string, + candidates: TaskDependencyCandidate[], + draftIds: string[], +): Record { + const [titleCache, setTitleCache] = useState({ + identity: "", + titles: {}, + }); + + useEffect(() => { + setTitleCache((current) => { + if (current.identity !== identity) { + return { identity, titles: {} }; + } + const nextTitles = { ...current.titles }; + let changed = false; + for (const candidate of candidates) { + if (!draftIds.includes(candidate.id) || nextTitles[candidate.id] === candidate.title) { + continue; + } + nextTitles[candidate.id] = candidate.title; + changed = true; + } + return changed ? { identity, titles: nextTitles } : current; + }); + }, [candidates, draftIds, identity]); + + return titleCache.identity === identity ? titleCache.titles : {}; +} + +function mergeSelectedTitles( + knownTitles: Record, + cachedSelectedTitles: Record, + candidates: TaskDependencyCandidate[], +): Record { + return { + ...knownTitles, + ...cachedSelectedTitles, + ...Object.fromEntries(candidates.map((candidate) => [candidate.id, candidate.title])), + }; +} + +export function useTaskEditDialogDependencies({ + open, + workspaceId, + taskId, +}: TaskEditDialogDependenciesArgs): TaskEditDialogDependenciesState { + const [saveError, setSaveError] = useState(null); + const [submitError, setSubmitError] = useState(null); + const [projectionReloadToken, setProjectionReloadToken] = useState(0); + const [candidateReloadToken, setCandidateReloadToken] = useState(0); + const [query, setQueryState] = useState(""); + const dependencyDialogIdentity = JSON.stringify([open, workspaceId ?? null, taskId ?? null]); + const projection = useTaskDependencyProjection( + { open, workspaceId, taskId }, + projectionReloadToken, + ); + const candidateState = useTaskDependencyCandidates( + { open, workspaceId, taskId, query }, + candidateReloadToken, + ); + + const saveIdentity = JSON.stringify([ + open, + workspaceId ?? null, + taskId ?? null, + projectionReloadToken, + ]); + const saveGuard = useDependencySaveGuard(saveIdentity); + + useEffect(() => { + setSaveError(null); + setSubmitError(null); + if (open && workspaceId && taskId) setQueryState(""); + }, [open, taskId, workspaceId]); + + const setDraftIds = useCallback( + (ids: string[]) => { + projection.setDraftIds(uniqueIDs(ids)); + setSaveError(null); + setSubmitError(null); + }, + [projection], + ); + + const setQuery = useCallback((value: string) => { + setQueryState(value); + }, []); + + const { confirmedIds, setConfirmedIds, draftIds, knownTitles, loading, loadError } = projection; + const { candidates, candidatesLoading, candidateError } = candidateState; + const isDirty = !sameIDs(confirmedIds, draftIds); + const cachedSelectedTitles = useDependencyTitleCache( + dependencyDialogIdentity, + candidates, + draftIds, + ); + + const save = useCallback(async () => { + if (!open || !workspaceId || !taskId || loading || !isDirty) return; + const request = saveGuard.begin(); + const ids = [...draftIds]; + setSaveError(null); + setSubmitError(null); + try { + await replaceTaskDependencies(taskId, ids); + if (!saveGuard.isCurrent(request)) return; + setConfirmedIds(ids); + } catch (cause: unknown) { + if (!saveGuard.isCurrent(request)) return; + const failure: TaskDependencyUpdateFailure = { dependencyUpdate: true, cause }; + setSaveError(cause); + setSubmitError(failure); + throw failure; + } + }, [draftIds, isDirty, loading, open, saveGuard, taskId, workspaceId]); + + const selectedTitles = useMemo( + () => mergeSelectedTitles(knownTitles, cachedSelectedTitles, candidates), + [cachedSelectedTitles, candidates, knownTitles], + ); + const retry = useCallback(() => setProjectionReloadToken((value) => value + 1), []); + const retryCandidates = useCallback(() => setCandidateReloadToken((value) => value + 1), []); + const ready = !loading && !loadError; + + return { + taskId: taskId ?? null, + confirmedIds, + draftIds, + setDraftIds, + selectedTitles, + candidates, + candidatesLoading, + query, + setQuery, + loading, + loadError, + candidateError, + saveError, + error: saveError ?? loadError ?? candidateError, + submitError, + ready, + isDirty, + save, + retry, + retryCandidates, + }; +} diff --git a/apps/web/lib/api/domains/task-dependencies-api.test.ts b/apps/web/lib/api/domains/task-dependencies-api.test.ts new file mode 100644 index 0000000000..ee8e5fc581 --- /dev/null +++ b/apps/web/lib/api/domains/task-dependencies-api.test.ts @@ -0,0 +1,83 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ApiError } from "@/lib/api/client"; +import { getTaskDependencyCycle, replaceTaskDependencies } from "./task-dependencies-api"; + +const fetchSpy = vi.fn(); +const API_BASE_URL = "http://api.test"; + +beforeEach(() => { + fetchSpy.mockReset(); + vi.stubGlobal("fetch", fetchSpy); +}); + +afterEach(() => vi.unstubAllGlobals()); + +describe("replaceTaskDependencies", () => { + it("replaces the complete predecessor set with a PUT request", async () => { + fetchSpy.mockResolvedValueOnce( + new Response( + JSON.stringify({ + task_id: "task-1", + blocked: true, + blocked_reason: "pending", + depends_on: [{ id: "task-2", title: "Predecessor" }], + blocks: [], + }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ), + ); + + await expect( + replaceTaskDependencies("task-1", ["task-2"], { baseUrl: API_BASE_URL }), + ).resolves.toMatchObject({ task_id: "task-1", blocked: true }); + + expect(fetchSpy).toHaveBeenCalledOnce(); + const [url, init] = fetchSpy.mock.calls[0]; + expect(url).toBe(`${API_BASE_URL}/api/v1/tasks/task-1/dependencies`); + expect(init).toMatchObject({ + method: "PUT", + body: JSON.stringify({ depends_on_task_ids: ["task-2"] }), + }); + expect(new Headers(init?.headers).get("Content-Type")).toBe("application/json"); + }); + + it("does not let caller request options override the replacement method or body", async () => { + fetchSpy.mockResolvedValueOnce( + new Response(JSON.stringify({ task_id: "task-1" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); + + await replaceTaskDependencies("task-1", ["task-2"], { + baseUrl: API_BASE_URL, + init: { method: "POST", body: "caller-controlled body" }, + }); + + const [, init] = fetchSpy.mock.calls[0]; + expect(init?.method).toBe("PUT"); + expect(init?.body).toBe(JSON.stringify({ depends_on_task_ids: ["task-2"] })); + }); + + it("keeps the structured cycle path available to the edit dialog", async () => { + fetchSpy.mockResolvedValueOnce( + new Response( + JSON.stringify({ + error: "would create a dependency cycle", + cycle: ["task-1", "task-2", "task-1"], + }), + { status: 409, headers: { "Content-Type": "application/json" } }, + ), + ); + + let error: unknown; + try { + await replaceTaskDependencies("task-1", ["task-2"], { baseUrl: API_BASE_URL }); + } catch (caught) { + error = caught; + } + + expect(error).toBeInstanceOf(ApiError); + expect(getTaskDependencyCycle(error)).toEqual(["task-1", "task-2", "task-1"]); + }); +}); diff --git a/apps/web/lib/api/domains/task-dependencies-api.ts b/apps/web/lib/api/domains/task-dependencies-api.ts index 36d6769997..016a29a1f2 100644 --- a/apps/web/lib/api/domains/task-dependencies-api.ts +++ b/apps/web/lib/api/domains/task-dependencies-api.ts @@ -1,4 +1,4 @@ -import { fetchJson, type ApiRequestOptions } from "@/lib/api/client"; +import { ApiError, fetchJson, type ApiRequestOptions } from "@/lib/api/client"; const BASE = "/api/v1"; @@ -9,6 +9,15 @@ export type TaskDependencyRefResponse = { status?: "resolved" | "failed" | "pending"; }; +export type TaskDependencyProjectionResponse = { + task_id?: string; + id?: string; + blocked?: boolean; + blocked_reason?: string; + depends_on?: TaskDependencyRefResponse[]; + blocks?: TaskDependencyRefResponse[]; +}; + /** * Reads one task's dependency projection. * @@ -19,11 +28,29 @@ export type TaskDependencyRefResponse = { * fetch-your-own-data shape `PRStatusChip` already uses via `useTaskPR`. */ export function getTaskDependencies(taskId: string, options?: ApiRequestOptions) { - return fetchJson<{ - id: string; - blocked?: boolean; - blocked_reason?: string; - depends_on?: TaskDependencyRefResponse[]; - blocks?: TaskDependencyRefResponse[]; - }>(`${BASE}/tasks/${taskId}`, options); + return fetchJson(`${BASE}/tasks/${taskId}`, options); +} + +export function replaceTaskDependencies( + taskId: string, + dependsOnTaskIDs: string[], + options?: ApiRequestOptions, +) { + return fetchJson(`${BASE}/tasks/${taskId}/dependencies`, { + ...options, + init: { + ...options?.init, + method: "PUT", + body: JSON.stringify({ depends_on_task_ids: dependsOnTaskIDs }), + }, + }); +} + +export function getTaskDependencyCycle(error: unknown): string[] | null { + if (!(error instanceof ApiError) || !error.body || typeof error.body !== "object") return null; + const cycle = (error.body as { cycle?: unknown }).cycle; + if (!Array.isArray(cycle) || !cycle.every((id): id is string => typeof id === "string")) { + return null; + } + return cycle; } diff --git a/apps/web/lib/types/task-dependencies.ts b/apps/web/lib/types/task-dependencies.ts new file mode 100644 index 0000000000..ea8feef633 --- /dev/null +++ b/apps/web/lib/types/task-dependencies.ts @@ -0,0 +1,5 @@ +export type TaskDependencyCandidate = { + id: string; + title: string; + isArchived?: boolean; +}; diff --git a/apps/web/src/locales/en/task.json b/apps/web/src/locales/en/task.json index 4dd5057396..d4fc217515 100644 --- a/apps/web/src/locales/en/task.json +++ b/apps/web/src/locales/en/task.json @@ -350,6 +350,7 @@ "dependencyChangeRequestAria": "Linked pull or merge request #{{number}}", "dependencyChangeRequestLabel": "#{{number}}", "dependencyChipLabel": "Task dependencies", + "dependencyCycleError": "Dependency cycle: {{cycle}}", "dependencyCountBlockedBy_one": "{{count}} blocker", "dependencyCountBlockedBy_other": "{{count}} blockers", "dependencyCountBlocks_one": "blocks {{count}}", @@ -358,6 +359,7 @@ "dependencyCount_other": "{{count}} dependencies", "dependencyInfo": "This task waits until every selected task completes successfully. If you start an agent, it starts automatically when all dependencies are resolved.", "dependencyInfoLabel": "About task dependencies", + "dependencyUpdateFailed": "Failed to update dependencies.", "dependsOn": "Depends on", "describeWhatNeedsToChange": "Describe what needs to change", "describeWhatYouWantTheAgent": "Describe what you want the agent to do... (@ to insert a saved prompt)", @@ -486,6 +488,8 @@ "failedToLinkGithubPullRequest": "Failed to link GitHub pull request.", "failedToLinkTarget": "Failed to link {{target}}.", "failedToLoadDocuments": "Failed to load documents", + "failedToLoadDependencies": "Failed to load dependencies.", + "failedToLoadDependencyCandidates": "Failed to load dependency candidates.", "failedToLoadPreview": "Failed to load preview.", "failedToLoadRevision": "Failed to load revision", "failedToLoadRevisionContent": "Failed to load revision content", @@ -714,6 +718,7 @@ "loadingCommandOutput": "Loading command output...", "loadingCommit": "Loading commit...", "loadingConversation": "Loading conversation...", + "loadingDependencies": "Loading dependencies...", "loadingEditor": "Loading editor...", "loadingFile": "Loading file...", "loadingFiles": "Loading files...", diff --git a/apps/web/src/locales/pseudo/task.json b/apps/web/src/locales/pseudo/task.json index 232d7fd739..74f757ac4e 100644 --- a/apps/web/src/locales/pseudo/task.json +++ b/apps/web/src/locales/pseudo/task.json @@ -350,6 +350,7 @@ "dependencyChangeRequestAria": "Ĺĩńķēď ƥũĺĺ ōŕ ḿēŕĝē ŕēqũēśţ #{{number}}", "dependencyChangeRequestLabel": "#{{number}}", "dependencyChipLabel": "Ţàśķ ďēƥēńďēńćĩēś", + "dependencyCycleError": "Ďēƥēńďēńćŷ ćŷćĺē: {{cycle}}", "dependencyCountBlockedBy_one": "{{count}} ƀĺōćķēŕ", "dependencyCountBlockedBy_other": "{{count}} ƀĺōćķēŕś", "dependencyCountBlocks_one": "ƀĺōćķś {{count}}", @@ -358,6 +359,7 @@ "dependencyCount_other": "{{count}} ďēƥēńďēńćĩēś", "dependencyInfo": "Ţĥĩś ţàśķ ŵàĩţś ũńţĩĺ ēvēŕŷ śēĺēćţēď ţàśķ ćōḿƥĺēţēś śũććēśśƒũĺĺŷ. Ĩƒ ŷōũ śţàŕţ àń àĝēńţ, ĩţ śţàŕţś àũţōḿàţĩćàĺĺŷ ŵĥēń àĺĺ ďēƥēńďēńćĩēś àŕē ŕēśōĺvēď.", "dependencyInfoLabel": "Àƀōũţ ţàśķ ďēƥēńďēńćĩēś", + "dependencyUpdateFailed": "Ƒàĩĺēď ţō ũƥďàţē ďēƥēńďēńćĩēś.", "dependsOn": "Ďēƥēńďś ōń", "describeWhatNeedsToChange": "Ďēśćŕĩƀē ŵĥàţ ńēēďś ţō ćĥàńĝē", "describeWhatYouWantTheAgent": "Ďēśćŕĩƀē ŵĥàţ ŷōũ ŵàńţ ţĥē àĝēńţ ţō ďō... (@ ţō ĩńśēŕţ à śàvēď ƥŕōḿƥţ)", @@ -486,6 +488,8 @@ "failedToLinkGithubPullRequest": "Ƒàĩĺēď ţō ĺĩńķ ĜĩţĤũƀ ƥũĺĺ ŕēqũēśţ.", "failedToLinkTarget": "Ƒàĩĺēď ţō ĺĩńķ {{target}}.", "failedToLoadDocuments": "Ƒàĩĺēď ţō ĺōàď ďōćũḿēńţś", + "failedToLoadDependencies": "Ƒàĩĺēď ţō ĺōàď ďēƥēńďēńćĩēś.", + "failedToLoadDependencyCandidates": "Ƒàĩĺēď ţō ĺōàď ďēƥēńďēńćŷ ćàńďĩďàţēś.", "failedToLoadPreview": "Ƒàĩĺēď ţō ĺōàď ƥŕēvĩēŵ.", "failedToLoadRevision": "Ƒàĩĺēď ţō ĺōàď ŕēvĩśĩōń", "failedToLoadRevisionContent": "Ƒàĩĺēď ţō ĺōàď ŕēvĩśĩōń ćōńţēńţ", @@ -714,6 +718,7 @@ "loadingCommandOutput": "Ĺōàďĩńĝ ćōḿḿàńď ōũţƥũţ...", "loadingCommit": "Ĺōàďĩńĝ ćōḿḿĩţ...", "loadingConversation": "Ĺōàďĩńĝ ćōńvēŕśàţĩōń...", + "loadingDependencies": "Ĺōàďĩńĝ ďēƥēńďēńćĩēś...", "loadingEditor": "Ĺōàďĩńĝ ēďĩţōŕ...", "loadingFile": "Ĺōàďĩńĝ ƒĩĺē...", "loadingFiles": "Ĺōàďĩńĝ ƒĩĺēś...", diff --git a/apps/web/src/locales/pt-pt/task.json b/apps/web/src/locales/pt-pt/task.json index 098b5bbba0..75884bdc05 100644 --- a/apps/web/src/locales/pt-pt/task.json +++ b/apps/web/src/locales/pt-pt/task.json @@ -469,6 +469,8 @@ "failedToLinkGithubPullRequest": "Não foi possível associar o pull request do GitHub.", "failedToLinkTarget": "Não foi possível associar {{target}}.", "failedToLoadDocuments": "Não foi possível carregar os documentos", + "failedToLoadDependencies": "Não foi possível carregar as dependências.", + "failedToLoadDependencyCandidates": "Não foi possível carregar as tarefas disponíveis como dependências.", "failedToLoadPreview": "Não foi possível carregar a pré-visualização.", "failedToLoadRevision": "Não foi possível carregar a revisão", "failedToLoadRevisionContent": "Não foi possível carregar o conteúdo da revisão", @@ -697,6 +699,7 @@ "loadingCommandOutput": "A carregar a saída do comando...", "loadingCommit": "A carregar o commit...", "loadingConversation": "A carregar a conversa...", + "loadingDependencies": "A carregar dependências...", "loadingEditor": "A carregar o editor...", "loadingFile": "A carregar o ficheiro...", "loadingFiles": "A carregar os ficheiros...", @@ -1800,6 +1803,7 @@ "dependencyChangeRequestAria": "Pull request ou merge request associado #{{number}}", "dependencyChangeRequestLabel": "#{{number}}", "dependencyChipLabel": "Dependências da tarefa", + "dependencyCycleError": "Ciclo de dependências: {{cycle}}", "dependencyCount_one": "{{count}} dependência", "dependencyCount_other": "{{count}} dependências", "dependencyCountBlockedBy_one": "{{count}} bloqueio", @@ -1808,6 +1812,7 @@ "dependencyCountBlocks_other": "bloqueia {{count}}", "dependencyInfo": "Esta tarefa espera até que todas as tarefas selecionadas terminem com sucesso. Se iniciar um agente, ele arranca automaticamente quando todas as dependências estiverem resolvidas.", "dependencyInfoLabel": "Sobre dependências de tarefas", + "dependencyUpdateFailed": "Não foi possível atualizar as dependências.", "dependsOn": "Depende de", "enhancedPromptCopiedToClipboard": "Prompt melhorado copiado para a área de transferência.", "enhancedPromptCouldNotBeCopied": "Não foi possível copiar o prompt melhorado.", diff --git a/apps/web/src/locales/zh-cn/task.json b/apps/web/src/locales/zh-cn/task.json index 0311ec038c..d60c206557 100644 --- a/apps/web/src/locales/zh-cn/task.json +++ b/apps/web/src/locales/zh-cn/task.json @@ -469,6 +469,8 @@ "failedToLinkGithubPullRequest": "关联 GitHub 拉取请求失败。", "failedToLinkTarget": "关联 {{target}} 失败。", "failedToLoadDocuments": "加载文档失败", + "failedToLoadDependencies": "加载依赖关系失败。", + "failedToLoadDependencyCandidates": "加载依赖任务失败。", "failedToLoadPreview": "加载预览失败。", "failedToLoadRevision": "加载修订失败", "failedToLoadRevisionContent": "加载修订内容失败", @@ -696,6 +698,7 @@ "loadingCommandOutput": "正在加载命令输出…", "loadingCommit": "正在加载提交…", "loadingConversation": "正在加载对话…", + "loadingDependencies": "正在加载依赖关系…", "loadingEditor": "正在加载编辑器…", "loadingFile": "正在加载文件…", "loadingFiles": "正在加载文件…", @@ -1800,6 +1803,7 @@ "dependencyChangeRequestAria": "关联的拉取请求或合并请求 #{{number}}", "dependencyChangeRequestLabel": "#{{number}}", "dependencyChipLabel": "任务依赖", + "dependencyCycleError": "依赖关系循环：{{cycle}}", "dependencyCount_one": "{{count}} 个依赖", "dependencyCount_other": "{{count}} 个依赖", "dependencyCountBlockedBy_one": "{{count}} 个阻塞项", @@ -1808,6 +1812,7 @@ "dependencyCountBlocks_other": "阻塞 {{count}} 项", "dependencyInfo": "该任务会一直等待，直到所选的每个任务都成功完成。如果你启动了智能体，它会在所有依赖解决后自动开始。", "dependencyInfoLabel": "关于任务依赖", + "dependencyUpdateFailed": "更新依赖关系失败。", "dependsOn": "依赖于", "enhancedPromptCopiedToClipboard": "已将优化后的提示词复制到剪贴板。", "enhancedPromptCouldNotBeCopied": "无法复制优化后的提示词。", diff --git a/apps/web/src/locales/zh-hk/task.json b/apps/web/src/locales/zh-hk/task.json index 70293cce9d..b8e320ab01 100644 --- a/apps/web/src/locales/zh-hk/task.json +++ b/apps/web/src/locales/zh-hk/task.json @@ -469,6 +469,8 @@ "failedToLinkGithubPullRequest": "關聯 GitHub 拉取要求失敗。", "failedToLinkTarget": "關聯 {{target}} 失敗。", "failedToLoadDocuments": "載入文檔失敗", + "failedToLoadDependencies": "載入依賴關係失敗。", + "failedToLoadDependencyCandidates": "載入依賴任務失敗。", "failedToLoadPreview": "載入預覽失敗。", "failedToLoadRevision": "載入修訂失敗", "failedToLoadRevisionContent": "載入修訂內容失敗", @@ -696,6 +698,7 @@ "loadingCommandOutput": "正在載入命令輸出…", "loadingCommit": "正在載入提交…", "loadingConversation": "正在載入對話…", + "loadingDependencies": "正在載入依賴關係…", "loadingEditor": "正在載入編輯器…", "loadingFile": "正在載入檔案…", "loadingFiles": "正在載入檔案…", @@ -1800,6 +1803,7 @@ "dependencyChangeRequestAria": "關聯的拉取要求或合併請求 #{{number}}", "dependencyChangeRequestLabel": "#{{number}}", "dependencyChipLabel": "任務依賴", + "dependencyCycleError": "依賴關係循環：{{cycle}}", "dependencyCount_one": "{{count}} 個依賴", "dependencyCount_other": "{{count}} 個依賴", "dependencyCountBlockedBy_one": "{{count}} 個阻塞項", @@ -1808,6 +1812,7 @@ "dependencyCountBlocks_other": "阻塞 {{count}} 項", "dependencyInfo": "該任務會一直等待，直到所選的每個任務都成功完成。如果你啓動了代理程式，它會在所有依賴解決後自動開始。", "dependencyInfoLabel": "關於任務依賴", + "dependencyUpdateFailed": "更新依賴關係失敗。", "dependsOn": "依賴於", "enhancedPromptCopiedToClipboard": "已將優化後的提示詞複製到剪貼板。", "enhancedPromptCouldNotBeCopied": "無法複製優化後的提示詞。", diff --git a/apps/web/src/locales/zh-tw/task.json b/apps/web/src/locales/zh-tw/task.json index 8b690ea5e2..7c7187b6b7 100644 --- a/apps/web/src/locales/zh-tw/task.json +++ b/apps/web/src/locales/zh-tw/task.json @@ -469,6 +469,8 @@ "failedToLinkGithubPullRequest": "關聯 GitHub 提取要求失敗。", "failedToLinkTarget": "關聯 {{target}} 失敗。", "failedToLoadDocuments": "載入檔案失敗", + "failedToLoadDependencies": "載入依賴關係失敗。", + "failedToLoadDependencyCandidates": "載入依賴任務失敗。", "failedToLoadPreview": "載入預覽失敗。", "failedToLoadRevision": "載入修訂失敗", "failedToLoadRevisionContent": "載入修訂內容失敗", @@ -696,6 +698,7 @@ "loadingCommandOutput": "正在載入命令輸出…", "loadingCommit": "正在載入提交…", "loadingConversation": "正在載入對話…", + "loadingDependencies": "正在載入依賴關係…", "loadingEditor": "正在載入編輯器…", "loadingFile": "正在載入檔案…", "loadingFiles": "正在載入檔案…", @@ -1800,6 +1803,7 @@ "dependencyChangeRequestAria": "關聯的提取要求或合併請求 #{{number}}", "dependencyChangeRequestLabel": "#{{number}}", "dependencyChipLabel": "任務依賴", + "dependencyCycleError": "依賴關係迴圈：{{cycle}}", "dependencyCount_one": "{{count}} 個依賴", "dependencyCount_other": "{{count}} 個依賴", "dependencyCountBlockedBy_one": "{{count}} 個阻塞項", @@ -1808,6 +1812,7 @@ "dependencyCountBlocks_other": "阻塞 {{count}} 項", "dependencyInfo": "該任務會一直等待，直到所選的每個任務都成功完成。如果你啟動了代理程式，它會在所有依賴解決後自動開始。", "dependencyInfoLabel": "關於任務依賴", + "dependencyUpdateFailed": "更新依賴關係失敗。", "dependsOn": "依賴於", "enhancedPromptCopiedToClipboard": "已將最佳化後的提示詞複製到剪貼簿。", "enhancedPromptCouldNotBeCopied": "無法複製最佳化後的提示詞。", diff --git a/docs/plans/task-dependency-detail-editing/plan.md b/docs/plans/task-dependency-detail-editing/plan.md new file mode 100644 index 0000000000..ce79f62eea --- /dev/null +++ b/docs/plans/task-dependency-detail-editing/plan.md @@ -0,0 +1,130 @@ +--- +created: 2026-09-03 +status: complete +requirements: + - REQ-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001 +system_design: + - ../../specs/tasks/system-design/task-dependency-detail-editing.md +legacy_specs: [] +--- + +# Implementation Plan: Edit task dependencies + +## Overview + +Add dependency selection to the existing Edit task dialog. Keep the task-detail +dependency chip read-only. Add an atomic replacement route so Cancel and Update +retain their existing meanings. + +The implementation has two sequential work orders. Both use TDD. + +## Scope + +### In scope + +- Add an atomic task-scoped dependency replacement route. +- Search non-archived tasks in the edited task's workspace. +- Show current predecessors as selected in the Edit task dialog. +- Keep dependency selection as a draft until Update. +- Preserve dependencies when the user cancels. +- Add and remove direct predecessors through one replacement request. +- Keep the dialog open for cycle and request errors. +- Let existing WebSocket events update the Kanban board. +- Keep the task-detail dependency chip read-only. +- Provide desktop and touch layouts with the same user value. + +### Out of scope + +- Database migration, WebSocket, or MCP contract changes. +- Multi-task dependency edits. +- Cross-workspace edges. +- Changes to deferred launch intent. +- Changes to the Office task-properties picker. + +## Technical approach + +### Backend replacement contract + +- Add `PUT /api/v1/tasks/:id/dependencies` with a complete predecessor list. +- Authorize and validate the complete desired set before mutation. +- Hold the existing dependency lock across validation and replacement. +- Apply the edge diff in one SQL transaction. +- Publish existing `task.updated` events for the edited task and changed peers. +- Keep the existing add and remove routes unchanged. + +### Edit task dialog + +- Add the dependency field only in edit mode and outside the create-only + advanced-settings disclosure. +- Load the confirmed dependency projection when the dialog opens. +- Disable Update until the projection load finishes. +- Use `listTasksByWorkspace` for server-backed candidate search. +- Exclude the edited task and archived tasks. +- Keep selection changes in form state until Update. +- Add a typed replacement function and structured cycle parsing to the API + client. +- Keep `TaskDependencyChip` unchanged in both task status rows. + +### Desktop and mobile + +- Reuse the current centered edit dialog on desktop. +- Reuse the current full-screen edit dialog on phones. +- Keep the task-switcher sheet behind the editor on tablets. +- Reuse task-create picker rows for search and draft selection. +- Use the form as the dialog's single vertical scroll owner. +- Keep touch controls at least 44 CSS pixels in the active dimension. +- Keep the document free of horizontal overflow. + +### Localization + +- Add editor labels, loading states, empty states, and errors to the task namespace. +- Update `en`, `pt-pt`, `zh-cn`, `zh-hk`, and `zh-tw` catalogs. +- Generate the pseudo locale and both Traditional Chinese catalogs with repository scripts. + +## Tests + +| Acceptance criteria | Evidence | +| --- | --- | +| `AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.1` through `.3` | Dialog component tests for edit-mode visibility, initialization, and candidate filtering | +| `AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.4` and `.5` | Form-state tests and desktop cancel E2E | +| `AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.6` | Repository, service, and handler tests for atomic full-set replacement | +| `AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.7` | API client, dialog submit, WebSocket, and desktop E2E evidence | +| `AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.8` and `.9` | Backend rollback tests, dialog error tests, and desktop cycle E2E | +| `AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.10` and `.11` | Existing entry-point and dependency-chip tests plus focused regression tests | +| `AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.12` through `.14` | Desktop and mobile Playwright tests | +| `AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.15` | Backend authorization, cycle, and deferred-launch regression tests | + +## E2E tests + +- Extend `apps/web/e2e/tests/task/create-task-dependency-selector.spec.ts` for + desktop edit, update, cancel, and archived-task filtering. +- Extend `apps/web/e2e/tests/task/mobile-create-task-dependency-selector.spec.ts` + for touch edit, update, dialog geometry, and overflow. +- Use the `chromium` project for desktop and `mobile-chrome` for mobile. + +## Work orders + +- [x] [Task 01: Add atomic dependency replacement](task-01-enable-detail-editing.md) +- [x] [Task 02: Add dependencies to Edit task](task-02-add-edit-dialog-dependencies.md) + +## Verification results + +Implemented and verified. + +- Backend repository, service, and handler tests pass, including transaction + rollback, authorization, cycle rejection, and update publication. +- Frontend focused tests pass with 68 tests across the edit dialog, submit + flow, API client, dependency hook, and footer behavior. +- TypeScript typecheck, full frontend lint, i18n completeness, i18n ratchet, + and the full backend test and lint suites pass. +- The desktop dependency-selector suite passes with 2 tests, and the mobile + suite passes with 2 tests through the managed production E2E runner. +- Fresh desktop and mobile PR screenshots were captured, inspected, and + compressed from the same E2E flows. + +## Risks + +- The task-field update and dependency replacement are separate HTTP requests. + A dependency error must keep the dialog open and reload confirmed task fields. +- Candidate search can show stale results if a request from an old query wins a race. +- The cycle path contains task IDs. The UI must keep the error readable without changing server data. diff --git a/docs/plans/task-dependency-detail-editing/task-01-enable-detail-editing.md b/docs/plans/task-dependency-detail-editing/task-01-enable-detail-editing.md new file mode 100644 index 0000000000..b127a764a2 --- /dev/null +++ b/docs/plans/task-dependency-detail-editing/task-01-enable-detail-editing.md @@ -0,0 +1,114 @@ +--- +id: "01-enable-detail-editing" +title: "Add atomic dependency replacement" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001 +acceptance_criteria: + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.6 + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.7 + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.8 + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.9 + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.15 +system_design: + - ../../specs/tasks/system-design/task-dependency-detail-editing.md +--- + +# Task 01: Add atomic dependency replacement + +## Summary + +Add a task-scoped route that replaces one task's complete predecessor set as an +atomic dependency operation. + +## In scope + +- Add the full-set replacement handler and response contract. +- Authorize the edited task and every submitted predecessor. +- Validate empty, duplicate, self, cross-workspace, and cyclic inputs. +- Hold the dependency lock across validation and replacement. +- Apply the edge diff in one SQL transaction. +- Publish task updates for the edited task and every added or removed peer. +- Add repository, service, and handler tests. + +## Out of scope + +- Frontend changes. +- Multi-task dependency edits. +- Changes to the existing one-edge add and remove routes. +- Changes to `start_when_unblocked`. + +## Acceptance + +- A successful request replaces the exact predecessor set and returns its + dependency projection. +- Validation and storage errors leave the prior set unchanged. +- Existing authorization, cycle prevention, deferred launch, and event behavior + remain in force. + +## TDD sequence + +1. Add failing repository transaction tests. +2. Add failing service validation and publication tests. +3. Add failing HTTP contract tests. +4. Implement the repository, service, and handler changes. +5. Run the focused tests and refactor with all tests green. + +## Verification + +```bash +make -C apps/backend test +make -C apps/backend lint +git diff --check +``` + +## Files likely touched + +- `apps/backend/internal/office/repository/sqlite/blockers.go` +- `apps/backend/internal/office/repository/sqlite/blockers_test.go` +- `apps/backend/internal/task/service/service_office.go` +- `apps/backend/internal/task/service/service_dependencies.go` +- `apps/backend/internal/task/service/service_dependencies_test.go` +- `apps/backend/internal/task/handlers/task_dependency_handlers.go` +- `apps/backend/internal/task/handlers/task_dependency_handlers_test.go` +- `apps/backend/internal/task/handlers/task_handlers.go` + +## Dependencies + +None. The core dependency service and persistence table already exist. + +## Risks + +- The replacement route must not publish before the storage transaction commits. +- Adding a method to `BlockerRepository` requires updates to its test doubles. + +## Parallelism + +`sequential` + +## Inputs + +- `docs/specs/tasks/requirements/task-dependency-detail-editing.md` +- `docs/specs/tasks/system-design/task-dependency-detail-editing.md` +- `apps/web/components/task/task-dependency-chip.tsx` +- `apps/backend/internal/task/handlers/task_dependency_handlers.go` +- `apps/backend/internal/task/service/service_dependencies.go` + +## Results + +Implemented the task-scoped replacement contract while preserving the existing +one-edge routes. The service validates the complete predecessor set while +holding the dependency lock, the repository applies the edge diff in one SQL +transaction, and the handler returns the existing dependency projection. Task +updates publish for the edited task and changed peers after a successful commit. + +Validation passed: + +- `go test ./internal/office/repository/sqlite ./internal/task/service` +- `go test ./internal/task/handlers` +- `env -u KANDEV_INTERNAL_CONFIG_FILE -u KANDEV_INTERNAL_CONFIG_HOME_FILE make -C apps/backend test` +- `make -C apps/backend lint` +- `git diff --check` diff --git a/docs/plans/task-dependency-detail-editing/task-02-add-edit-dialog-dependencies.md b/docs/plans/task-dependency-detail-editing/task-02-add-edit-dialog-dependencies.md new file mode 100644 index 0000000000..abf5e47032 --- /dev/null +++ b/docs/plans/task-dependency-detail-editing/task-02-add-edit-dialog-dependencies.md @@ -0,0 +1,153 @@ +--- +id: "02-add-edit-dialog-dependencies" +title: "Add dependencies to Edit task" +status: done +wave: 2 +depends_on: + - "01-enable-detail-editing" +plan: "plan.md" +requirements: + - REQ-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001 +acceptance_criteria: + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.1 + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.2 + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.3 + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.4 + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.5 + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.7 + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.8 + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.9 + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.10 + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.11 + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.12 + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.13 + - AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.14 +system_design: + - ../../specs/tasks/system-design/task-dependency-detail-editing.md +--- + +# Task 02: Add dependencies to Edit task + +## Summary + +Add a searchable dependency field to the shared Edit task dialog. Stage changes +until Update and submit the complete predecessor draft through the replacement +route from Task 01. + +## In scope + +- Load the edited task's confirmed dependency projection. +- Search non-archived tasks in the edited task's workspace. +- Show current predecessors as selected and exclude the edited task. +- Keep picker changes in dialog form state until Update. +- Keep dependencies unchanged on Cancel. +- Keep the dialog open and preserve the draft after cycle or request errors. +- Keep the field available when the edited task has started. +- Keep the task-detail dependency chip read-only. +- Add localized copy in all required catalogs. +- Add API client, component, submit, desktop E2E, and mobile E2E coverage. + +## Out of scope + +- Changes to the Office task-properties picker. +- Inline dependency mutation from the task-detail chip. +- Multi-task dependency editing. +- Changes to `start_when_unblocked`. + +## Acceptance + +- Every existing Edit task entry point opens the same dependency field. +- Update submits the desired predecessor set and Cancel sends no replacement. +- Cycle and request errors preserve confirmed dependencies and permit a retry. +- Desktop, phone, and tablet paths provide the same user outcome. + +## TDD sequence + +1. Add failing API client and dialog state tests. +2. Add failing submit tests for update, cancel, and error retention. +3. Add failing desktop and mobile Playwright scenarios. +4. Implement the smallest shared picker, state, and submit changes. +5. Run the focused tests and refactor with all tests green. + +## Verification + +```bash +cd apps +pnpm install --frozen-lockfile +pnpm --filter @kandev/web test -- components/task-create-dialog.test.tsx components/task-create-dialog-submit.test.tsx components/task-edit-dialog-dependencies.test.tsx lib/api/domains/task-dependencies-api.test.ts hooks/domains/task/use-task-edit-dialog-dependencies.test.ts components/task-create-dialog-footer.test.ts +pnpm --filter @kandev/web run typecheck +pnpm run lint +cd web +pnpm run i18n:check +pnpm run i18n:ratchet +pnpm e2e:run --host --no-build --project chromium tests/task/create-task-dependency-selector.spec.ts +pnpm e2e:run --host --no-build --project mobile-chrome tests/task/mobile-create-task-dependency-selector.spec.ts +cd ../.. +git diff --check +``` + +## Files likely touched + +- `apps/web/lib/api/domains/task-dependencies-api.ts` +- `apps/web/lib/api/domains/task-dependencies-api.test.ts` +- `apps/web/components/task-create-dialog.tsx` +- `apps/web/components/task-create-dialog-types.ts` +- `apps/web/components/task-create-dialog-state.ts` +- `apps/web/components/task-create-dialog-setup.ts` +- `apps/web/components/task-create-dialog-prop-builders.ts` +- `apps/web/components/task-create-dialog-submit.tsx` +- `apps/web/components/task-create-dialog-submit.test.tsx` +- `apps/web/components/task-create-dialog-dependencies.tsx` +- `apps/web/components/task-edit-dialog-dependencies.tsx` +- `apps/web/components/task-edit-dialog-dependencies.test.tsx` +- `apps/web/src/locales/en/task.json` +- `apps/web/src/locales/pt-pt/task.json` +- `apps/web/src/locales/zh-cn/task.json` +- `apps/web/src/locales/zh-hk/task.json` +- `apps/web/src/locales/zh-tw/task.json` +- `apps/web/src/locales/pseudo/task.json` +- `apps/web/e2e/tests/task/task-dependencies.spec.ts` +- `apps/web/e2e/tests/task/mobile-sidebar-task-actions.spec.ts` + +## Dependencies + +Task 01 supplies the atomic replacement route. + +## Risks + +- Task-field and dependency updates use separate HTTP requests. A dependency + error must reload confirmed task fields while it keeps the dialog open. +- Candidate search responses can arrive out of order. +- A nested popover must remain inside the full-screen phone dialog. + +## Parallelism + +`sequential` + +## Inputs + +- `docs/specs/tasks/requirements/task-dependency-detail-editing.md` +- `docs/specs/tasks/system-design/task-dependency-detail-editing.md` +- `apps/web/components/task-create-dialog.tsx` +- `apps/web/components/task-create-dialog-dependencies.tsx` +- `apps/web/components/task/task-session-sidebar-edit.tsx` +- `apps/web/lib/api/domains/task-dependencies-api.ts` + +## Results + +Implemented the shared Edit task dependency field and the typed replacement +flow. The field loads confirmed predecessors, searches non-archived workspace +tasks, retains draft selection through Cancel or request errors, and appears in +the existing desktop and mobile Edit entry points. Cycle errors keep the dialog +open with the draft available for correction. The task-detail dependency chip +remains read-only. + +Validation passed: + +- 68 focused web tests passed across 6 files. +- TypeScript typecheck, full web lint, i18n completeness, and i18n ratchet passed. +- The desktop managed E2E suite passed with 2 tests. +- The mobile managed E2E suite passed with 2 tests. +- Capture-enabled desktop and mobile E2E runs produced inspected, compressed + PR assets with a complete manifest. +- `git diff --check` passed. diff --git a/docs/specs/tasks/README.md b/docs/specs/tasks/README.md index ea84b6e488..7b43f38a0d 100644 --- a/docs/specs/tasks/README.md +++ b/docs/specs/tasks/README.md @@ -88,6 +88,7 @@ signals, and task-scoped scheduling contracts. - [Task Create Workflow Memory](requirements/task-create-workflow-memory.md) - [Task-create advanced settings disclosure](requirements/task-dependencies-create-dialog-advanced-settings.md) - [Task-create dependency selector refinement](requirements/task-dependencies-create-dialog-dependency-selector.md) +- [Edit task dependencies](requirements/task-dependency-detail-editing.md) - [Task Dependencies and Auto-Start Chains](requirements/task-dependencies.md) - [Task Launch Failure Recovery](requirements/task-launch-failure-recovery.md) - [Task Title Length Limit](requirements/title-length-limit.md) @@ -142,6 +143,7 @@ signals, and task-scoped scheduling contracts. - [Queued Run Scheduling](system-design/run-scheduling.md) - [Session Delete Preserves Task Workspaces](system-design/session-delete-resource-cleanup.md) - [Task Dependencies and Auto-Start Chains](system-design/task-dependencies.md) +- [Edit task dependencies](system-design/task-dependency-detail-editing.md) - [Task Launch Failure Recovery](system-design/task-launch-failure-recovery.md) - [WIP Limits and Visible Overflow Queues](system-design/wip-limit-pull-system.md) - [Workflow quorum decision recording](system-design/workflow-quorum-decision-recording.md) diff --git a/docs/specs/tasks/requirements/task-dependencies-create-dialog-advanced-settings.md b/docs/specs/tasks/requirements/task-dependencies-create-dialog-advanced-settings.md index feb06f7b7b..ded9ed065e 100644 --- a/docs/specs/tasks/requirements/task-dependencies-create-dialog-advanced-settings.md +++ b/docs/specs/tasks/requirements/task-dependencies-create-dialog-advanced-settings.md @@ -56,9 +56,10 @@ place for dependency selection and future less-common options. advanced controls can be appended inside that region without adding another top-level row to the dialog. Do not add placeholder future controls in this change. -- Show the disclosure only for an unstarted task in create mode, matching the - dependency selector's existing availability. Session mode, edit mode, and - started-task forms do not gain a new advanced-settings row. +- Show the disclosure only for an unstarted task in create mode. Session mode, + edit mode, and started-task forms do not gain this advanced-settings row. + Edit-mode dependency behavior is owned by + [`task-dependency-detail-editing.md`](task-dependency-detail-editing.md). - Keep all new trigger copy localized through the task translation namespace. ## Interaction contract @@ -135,7 +136,7 @@ place for dependency selection and future less-common options. workflow's existing behavior is preserved and the advanced dependency control remains available under the same create-mode rules. - **GIVEN** session mode, edit mode, or a started task, **WHEN** the form - renders, **THEN** no advanced-settings disclosure is added. + renders, **THEN** this create-only advanced-settings disclosure is absent. - **GIVEN** a mobile create dialog, **WHEN** the user taps the disclosure and opens the dependency picker, **THEN** the controls remain reachable, the picker is contained in the viewport, and the document has no horizontal @@ -150,7 +151,7 @@ place for dependency selection and future less-common options. archived-task filtering, or automatic task-start behavior. - Changing the dependency selector's search, task icons, info help, or picker interaction except where needed to mount it inside the disclosure. -- Adding dependency editing to task detail, Kanban card menus, or MCP. +- Adding dependency editing as part of this advanced-settings refinement. - Adding or designing future advanced controls beyond the extensible content region. -- Changing workflow, agent, executor, repository, or branch selector behavior. \ No newline at end of file +- Changing workflow, agent, executor, repository, or branch selector behavior. diff --git a/docs/specs/tasks/requirements/task-dependencies-create-dialog-dependency-selector.md b/docs/specs/tasks/requirements/task-dependencies-create-dialog-dependency-selector.md index 56ab0d8c88..3d4b6f5d6b 100644 --- a/docs/specs/tasks/requirements/task-dependencies-create-dialog-dependency-selector.md +++ b/docs/specs/tasks/requirements/task-dependencies-create-dialog-dependency-selector.md @@ -65,8 +65,9 @@ set of tasks. unchanged. Selecting several predecessors keeps the existing AND semantics. - The dependency selector is disabled while the create flow is creating a session, matching the current dependency control behavior. -- The selector is available only for an unstarted task in create mode. It does - not add an edge editor to task detail or edit mode. +- This create-flow selector is available only for an unstarted task in create + mode. Edit-mode behavior is owned by + [`task-dependency-detail-editing.md`](task-dependency-detail-editing.md). - All new trigger labels, search copy, empty states, count labels, and help copy use the task translation namespace. No new user-facing literal is hardcoded in a component. @@ -133,6 +134,6 @@ set of tasks. - Changing the dependency graph, auto-start semantics, or backend/API payload. - Limiting a task to one predecessor. Multiple predecessors and AND semantics remain supported. -- Adding dependency editing to task detail, Kanban card menus, or MCP. +- Adding dependency editing as part of this create-dialog refinement. - Adding a new mobile navigation surface or replacing the task-create dialog. -- Changing workflow, agent, executor, repository, or branch selector behavior. \ No newline at end of file +- Changing workflow, agent, executor, repository, or branch selector behavior. diff --git a/docs/specs/tasks/requirements/task-dependency-detail-editing.md b/docs/specs/tasks/requirements/task-dependency-detail-editing.md new file mode 100644 index 0000000000..ac490583c8 --- /dev/null +++ b/docs/specs/tasks/requirements/task-dependency-detail-editing.md @@ -0,0 +1,54 @@ +--- +status: active +system: tasks +created: 2026-09-03 +owners: + - kandev +--- + +# Edit task dependencies Requirements + +## Overview + +The task system owns dependency edges and their effect on task execution. Users +need to change these edges when work plans change after task creation. + +## Terminology + +- **Predecessor:** A task that must complete successfully before the dependent task is unblocked. +- **Dependent task:** The open task that waits for one or more predecessors. + +## Requirements + +### REQ-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001: Manage dependencies in the Edit task dialog + +**Intent:** Let users correct task order from the existing task editor without task recreation or direct database changes. + +**User story:** As a developer, I want to change task dependencies after creation, so that the task graph matches the current work plan. + +#### Acceptance criteria + +- **AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.1:** The existing Edit task dialog shall show a dependency field for every non-archived task, including a task that has started. +- **AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.2:** The field shall show each current predecessor as selected and shall show `No dependency` when the set is empty. +- **AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.3:** The picker shall search non-archived tasks in the edited task's workspace and shall exclude the edited task. +- **AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.4:** Picker changes shall remain draft changes until the user activates Update. +- **AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.5:** When the user cancels the dialog, the system shall not change the dependency set. +- **AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.6:** Update shall replace the direct predecessor set as one atomic dependency operation. +- **AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.7:** After a successful update, the task editor and Kanban board shall show the new blocked state without a reload. +- **AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.8:** When the replacement creates a cycle, the dialog shall stay open, show the cycle error, preserve the confirmed dependency set, and keep the draft available for correction. +- **AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.9:** When another replacement error occurs, the dialog shall stay open, preserve the confirmed dependency set, and permit a retry. +- **AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.10:** Existing Edit actions on Kanban cards, desktop task menus, and touch task menus shall open the same dependency editor. +- **AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.11:** The task-detail dependency chip shall remain read-only. Its reverse `blocks` direction shall remain readable and navigable. +- **AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.12:** Desktop and touch layouts shall provide the same search, selection, save, cancel, and error outcomes. +- **AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.13:** Touch controls and task rows shall have a target of at least 44 CSS pixels in the active dimension. +- **AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.14:** The mobile dialog shall use one form scroll region and shall not add document-level horizontal overflow. +- **AC-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001.15:** Dependency edits shall keep the existing workspace authorization, cycle prevention, and auto-start behavior. + +## Out of scope + +- Bulk dependency edits from the Kanban selection toolbar. +- Cross-workspace dependencies. +- Changes to `start_when_unblocked` after task creation. +- Editable attributes on an edge. An edge has only add and remove operations. +- Changes to the Office task-properties dependency picker. +- Inline mutation controls in the task-detail dependency chip. diff --git a/docs/specs/tasks/system-design/task-dependencies.md b/docs/specs/tasks/system-design/task-dependencies.md index a1af4e5ffa..fa62c037d6 100644 --- a/docs/specs/tasks/system-design/task-dependencies.md +++ b/docs/specs/tasks/system-design/task-dependencies.md @@ -69,9 +69,9 @@ running agents unattended. start the dependent manually). Kandev never auto-retries a failed predecessor and never silently drops the edge. - Users declare dependencies in the **task-create dialog** ("Depends on") and - manage them afterwards over MCP. The task detail view deliberately has no edge - editor: dependencies describe how work was planned, and the surfaces that read - them (card badge, chip, graph) stay read-only. + manage them afterwards from the **Edit task dialog** or over MCP. The edit + dialog behavior is defined in + [`task-dependency-detail-editing.md`](task-dependency-detail-editing.md). - An open task SHALL show a **dependency chip** in the status row directly above its chat composer, alongside the PR status chip. The chip reports both directions — the tasks this task is blocked by, and the tasks it blocks — and @@ -571,9 +571,6 @@ MCP: A chain is a set of edges between concrete tasks in v1. - Gantt or timeline rendering, and estimated-duration modelling. - Editing dependencies from the multi-select toolbar or via bulk operations. -- An edge editor on the task detail view. Declaring dependencies is a planning - act that belongs to task creation (or to MCP for an agent decomposing work); - the task view only reports them. ## Implementation plan diff --git a/docs/specs/tasks/system-design/task-dependency-detail-editing.md b/docs/specs/tasks/system-design/task-dependency-detail-editing.md new file mode 100644 index 0000000000..3b4ecbef47 --- /dev/null +++ b/docs/specs/tasks/system-design/task-dependency-detail-editing.md @@ -0,0 +1,208 @@ +--- +status: current +system: tasks +requirements: + - REQ-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001 +created: 2026-09-03 +owners: + - kandev +--- + +# Edit task dependencies System Design + +## Purpose and boundaries + +This design adds dependency selection to the existing Edit task dialog. The +task-detail dependency chip stays read-only. The core dependency graph and +launch rules remain unchanged. + +The task system owns this design because it owns dependency state, permissions, and lifecycle effects. The UI provides the interaction surface only. + +## Requirement mapping + +| Requirement | Design section | +| --- | --- | +| `REQ-TASKS-TASK-DEPENDENCY-DETAIL-EDITING-001` | [Components and responsibilities](#components-and-responsibilities), [Control flow](#control-flow), [Failure behavior](#failure-behavior), [Responsive behavior](#responsive-behavior) | + +## Existing contracts + +The implementation keeps the one-edge contracts in +[`task-dependencies.md`](task-dependencies.md): + +- `POST /api/v1/tasks/:id/dependencies` adds one predecessor. +- `DELETE /api/v1/tasks/:id/dependencies/:depId` removes one predecessor. +- Both routes authorize both task IDs and reject cross-workspace access. +- The add route rejects cycles with HTTP `409` and a `cycle` array. +- A mutation publishes `task.updated` for both ends of the edge. +- A remove operation never consumes a deferred launch intent. + +It adds one task-scoped replacement contract: + +- `PUT /api/v1/tasks/:id/dependencies` accepts + `{ "depends_on_task_ids": ["task-id"] }`. +- The route replaces the complete direct predecessor set for one task. +- An empty list removes every direct predecessor. +- The response uses the existing dependency mutation projection. + +The replacement is one atomic dependency operation. The service validates the +complete desired set before storage changes. Storage applies the edge diff in +one transaction. A validation or storage error leaves the prior dependency set +unchanged. + +No database migration or WebSocket event change is necessary. The one-edge +routes remain available for Office, MCP, and other callers. + +### Replacement rationale + +The Edit task dialog stages several fields and applies them through Update. +Immediate one-edge mutations make Cancel misleading. A sequence of one-edge +requests can also stop after only part of a multi-selection succeeds. The +full-set route keeps the dependency part of the update atomic while it leaves +the established one-edge contracts intact. + +## Components and responsibilities + +### API client + +`apps/web/lib/api/domains/task-dependencies-api.ts` owns the typed replacement +function. It returns the dependency projection from the mutation response. The +module also parses the structured cycle response. + +The picker uses `listTasksByWorkspace` from `kanban-api.ts`. It sends the active +search text to the server and requests non-archived tasks only. + +### Dependency service and storage + +`Service.ReplaceDependencies` authorizes the edited task and every desired +predecessor. It rejects an empty ID, duplicate ID, self-edge, inaccessible task, +cross-workspace edge, and cycle. + +The existing process-wide dependency lock covers validation and replacement. +The production blocker repository replaces the edge diff in one SQL +transaction. It preserves unchanged edges and their creation times. + +After commit, the service publishes `task.updated` for the edited task and for +every predecessor that was added or removed. Publication remains outside the +lock. + +### Edit task dialog + +`TaskCreateDialog` remains the shared create, session, and edit shell. A focused +dependency field mounts only in edit mode. It stays outside the create-only +advanced-settings disclosure. + +When the dialog opens, the field loads the confirmed dependency projection for +the edited task. It initializes a draft list from `depends_on`. The Update +actions stay unavailable until this load finishes. + +The field owns these functions: + +- show current predecessors as selected +- search non-archived tasks in the edited task's workspace +- add or remove a predecessor from the draft +- show loading, empty, and error states. + +The picker excludes the edited task. The server remains the authority for +workspace and cycle validation. The same field remains available for started +tasks even when the prompt and workspace-source fields are locked. + +The current task-detail chip stays read-only and keeps its present visibility +rule. A task with no dependency edges has no chip. Users enter the editor from +the existing Edit actions. + +### Shared picker presentation + +The create and edit flows share the searchable dependency picker presentation. +They keep separate state and save behavior. Create mode still writes +`blocked_by` during task creation. Edit mode submits the complete draft through +the replacement route. + +The picker sends search text to `listTasksByWorkspace`. It ignores stale search +responses and never treats a partial page as the complete workspace task set. + +### State convergence + +The successful replacement response updates the dialog's confirmed projection. +The existing `task.updated` events then update the Kanban caches for all changed +tasks. + +The local response state is temporary. When the store contains the same +projection, the component uses the store projection again. + +If the store has no dependency projection, `getTaskDependencies` remains the +read fallback. An omitted projection does not mean that the task has no edges. + +## Control flow + +1. The user activates an existing Edit action. +2. The dialog loads the task's dependency projection and workspace candidates. +3. The user searches and changes the selected predecessor draft. +4. Cancel closes the dialog without a dependency request. +5. Update first runs the existing task-field update. +6. If the dependency draft changed, the client calls the replacement route. +7. The replacement response updates the dialog's confirmed projection. +8. WebSocket events update all affected task projections in board stores. +9. The dialog closes after every required update succeeds. + +The task-field update and dependency replacement remain separate HTTP +operations. If the task-field update succeeds and dependency replacement fails, +the dialog stays open and reloads the confirmed task fields. The dependency set +does not change. This keeps the new dependency operation atomic without +expanding the task update transaction and its repository replacement behavior. + +## Failure behavior + +For HTTP `409`, the dialog translates the cycle path and shows it in the normal +error surface. The dialog does not close or change the confirmed edge list. It +keeps the draft so the user can remove the invalid selection. + +For all other replacement errors, the dialog shows localized error copy. It +keeps the prior projection, preserves the draft, and enables Update again. + +An empty or failed candidate search shows a localized state. It does not alter +dependency data. + +## Security + +The picker requests tasks for the edited task's workspace. The server authorizes +the edited task and every submitted predecessor before replacement. It returns +not-found behavior for inaccessible tasks. + +The client does not infer permission from the picker contents. It treats the +server response as authoritative. + +## Responsive behavior + +Desktop uses the current centered Edit task dialog with its contained searchable +popover. Phones use the existing full-screen Edit task dialog. Tablets keep the +task-switcher sheet behind the same editor. + +The nearest mobile exemplars are `TaskCreateDialog` and +`session-task-switcher-sheet.tsx`. The phone task-actions menu closes before the +editor opens. The dialog form remains the single vertical scroll owner. The +dependency command list owns scrolling only while its contained popover is +open. + +Touch actions and task rows use a target of at least 44 CSS pixels. Long titles +truncate inside the surface. The document does not gain horizontal scrolling. + +The editor shares loading, candidate search, draft selection, submission, and +error logic across layouts. Existing dialog breakpoints own the presentation. + +## Test strategy + +Backend tests cover full-set validation, atomic replacement, cycle errors, +authorization, rollback, and publication for added and removed predecessors. + +Frontend tests cover edit-mode initialization, filtering, draft changes, cancel, +successful replacement, cycle errors, request errors, and started-task access. + +Desktop Playwright coverage proves edit, update, live blocked state, removal, +cancel, and cycle feedback. Mobile Playwright coverage proves the same value +through the visible task-actions menu. It also checks dialog containment, touch +targets, and horizontal overflow. + +## Related designs + +- [Task Dependencies and Auto-Start Chains](task-dependencies.md) +- [Task-create dependency selector refinement](../requirements/task-dependencies-create-dialog-dependency-selector.md)