dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 6 Packages Projects Releases Wiki Activity

fix: preserve task repo on rename updates (#1511)

Browse Source
This commit is contained in:
Carlos Florêncio
2026-06-27 12:46:31 +01:00
committed by GitHub
parent 9484bc33d9
commit 2ec30ef8cb
9 changed files with 414 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/internal/task/service/service_events.go
+8 -7
View File
@@ -63,10 +63,11 @@ func (s *Service) publishTaskEvent(ctx context.Context, eventType string, task *
// carries the full per-task repository list — matching the HTTP DTO and
// preventing the frontend from collapsing multi-repo tasks down to the
// primary repo on WS updates.
repos := taskRepositoriesForEvent(ctx, s, task)
if len(repos) > 0 {
data["repository_id"] = repos[0].RepositoryID
if repos, ok := taskRepositoriesForEvent(ctx, s, task); ok {
data["repositories"] = serializeTaskRepositories(repos)
if len(repos) > 0 {
data["repository_id"] = repos[0].RepositoryID
}
}
if task.Metadata != nil {
data["metadata"] = task.Metadata
@@ -135,15 +136,15 @@ func (s *Service) addTaskSessionEventFields(ctx context.Context, taskID string,
// position. Prefers Task.Repositories when already loaded; falls back to a
// lookup so publishers that pass a task without eagerly loaded repositories
// (e.g. the orchestrator's raw repo.GetTask) still emit per-repo data.
func taskRepositoriesForEvent(ctx context.Context, s *Service, task *models.Task) []*models.TaskRepository {
func taskRepositoriesForEvent(ctx context.Context, s *Service, task *models.Task) ([]*models.TaskRepository, bool) {
if len(task.Repositories) > 0 {
return task.Repositories
return task.Repositories, true
}
repos, err := s.taskRepos.ListTaskRepositories(ctx, task.ID)
if err != nil {
return nil
return nil, false
}
return repos
return repos, true
}
// serializeTaskRepositories returns the WS-shaped repositories array. Mirrors
apps/backend/internal/task/service/service_events_test.go
+157
View File
@@ -0,0 +1,157 @@
package service
import (
"context"
"errors"
"testing"
"github.com/kandev/kandev/internal/task/models"
"github.com/kandev/kandev/internal/task/repository"
)
type failingTaskRepoRepository struct {
repository.TaskRepoRepository
err error
}
type taskEventTestRepository interface {
CreateWorkspace(context.Context, *models.Workspace) error
CreateWorkflow(context.Context, *models.Workflow) error
CreateTask(context.Context, *models.Task) error
}
func (r failingTaskRepoRepository) ListTaskRepositories(ctx context.Context, taskID string) ([]*models.TaskRepository, error) {
return nil, r.err
}
// TestPublishTaskUpdated_FallbackRepositoryID exercises the DB fallback in
// taskRepositoriesForEvent: orchestrator-originated events load the task via the
// raw repo.GetTask, which does not populate Repositories. The publisher must
// still emit repository_id so the frontend doesn't lose the repo link on
// workflow transitions or state changes.
func TestPublishTaskUpdated_FallbackRepositoryID(t *testing.T) {
svc, eventBus, repo := createTestService(t)
ctx := context.Background()
if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace"}); err != nil {
t.Fatalf("CreateWorkspace: %v", err)
}
if err := repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-1", WorkspaceID: "ws-1", Name: "WF"}); err != nil {
t.Fatalf("CreateWorkflow: %v", err)
}
if err := repo.CreateRepository(ctx, &models.Repository{ID: "repo-x", WorkspaceID: "ws-1", Name: "Repo"}); err != nil {
t.Fatalf("CreateRepository: %v", err)
}
if err := repo.CreateTask(ctx, &models.Task{
ID: "task-1", WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", Title: "T", Priority: "medium",
}); err != nil {
t.Fatalf("CreateTask: %v", err)
}
if err := repo.CreateTaskRepository(ctx, &models.TaskRepository{
TaskID: "task-1", RepositoryID: "repo-x", BaseBranch: "main",
}); err != nil {
t.Fatalf("CreateTaskRepository: %v", err)
}
eventBus.ClearEvents()
task := &models.Task{
ID: "task-1", WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1",
}
if len(task.Repositories) != 0 {
t.Fatal("pre-condition: task.Repositories must be nil for this test")
}
svc.PublishTaskUpdated(ctx, task)
data := singlePublishedEventData(t, eventBus)
got, ok := data["repository_id"].(string)
if !ok {
t.Fatalf("repository_id missing from payload or wrong type: %#v", data["repository_id"])
}
if got != "repo-x" {
t.Fatalf("expected repository_id=repo-x via DB fallback, got %q", got)
}
repos, ok := data["repositories"].([]map[string]interface{})
if !ok {
t.Fatalf("repositories missing from payload or wrong type: %#v", data["repositories"])
}
if len(repos) != 1 || repos[0]["repository_id"] != "repo-x" {
t.Fatalf("expected repositories payload with repo-x, got %#v", repos)
}
}
func TestPublishTaskUpdated_EmitsEmptyRepositories(t *testing.T) {
svc, eventBus, repo := createTestService(t)
ctx := context.Background()
createTaskWithoutRepositories(t, ctx, repo)
eventBus.ClearEvents()
svc.PublishTaskUpdated(ctx, &models.Task{
ID: "task-1", WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1",
})
data := singlePublishedEventData(t, eventBus)
if _, ok := data["repository_id"]; ok {
t.Fatalf("repository_id should be absent for tasks with no repositories: %#v", data["repository_id"])
}
repos, ok := data["repositories"].([]map[string]interface{})
if !ok {
t.Fatalf("repositories missing from payload or wrong type: %#v", data["repositories"])
}
if len(repos) != 0 {
t.Fatalf("expected empty repositories payload, got %#v", repos)
}
}
func TestPublishTaskUpdated_OmitsRepositoriesOnLookupError(t *testing.T) {
svc, eventBus, repo := createTestService(t)
ctx := context.Background()
createTaskWithoutRepositories(t, ctx, repo)
svc.taskRepos = failingTaskRepoRepository{
TaskRepoRepository: repo,
err: errors.New("repository lookup failed"),
}
eventBus.ClearEvents()
svc.PublishTaskUpdated(ctx, &models.Task{
ID: "task-1", WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1",
})
data := singlePublishedEventData(t, eventBus)
if _, ok := data["repository_id"]; ok {
t.Fatalf("repository_id should be absent when repository lookup fails: %#v", data["repository_id"])
}
if _, ok := data["repositories"]; ok {
t.Fatalf("repositories should be absent when repository lookup fails: %#v", data["repositories"])
}
}
func createTaskWithoutRepositories(t *testing.T, ctx context.Context, repo taskEventTestRepository) {
t.Helper()
if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace"}); err != nil {
t.Fatalf("CreateWorkspace: %v", err)
}
if err := repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-1", WorkspaceID: "ws-1", Name: "WF"}); err != nil {
t.Fatalf("CreateWorkflow: %v", err)
}
task := &models.Task{
ID: "task-1", WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1",
}
if err := repo.CreateTask(ctx, task); err != nil {
t.Fatalf("CreateTask: %v", err)
}
}
func singlePublishedEventData(t *testing.T, eventBus *MockEventBus) map[string]interface{} {
t.Helper()
events := eventBus.GetPublishedEvents()
if len(events) != 1 {
t.Fatalf("expected 1 published event, got %d", len(events))
}
data, ok := events[0].Data.(map[string]interface{})
if !ok {
t.Fatalf("event Data wrong type: %T", events[0].Data)
}
return data
}
apps/backend/internal/task/service/service_test.go
-57
View File
@@ -1418,60 +1418,3 @@ func TestService_DeleteMessage(t *testing.T) {
t.Error("expected comment to be deleted")
}
}
// TestPublishTaskUpdated_FallbackRepositoryID exercises the DB fallback in
// primaryRepositoryID: orchestrator-originated events load the task via the
// raw repo.GetTask, which does not populate Repositories. The publisher must
// still emit repository_id so the frontend doesn't lose the repo link on
// workflow transitions or state changes.
func TestPublishTaskUpdated_FallbackRepositoryID(t *testing.T) {
svc, eventBus, repo := createTestService(t)
ctx := context.Background()
// Seed workspace + workflow + repo + task with an association.
if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace"}); err != nil {
t.Fatalf("CreateWorkspace: %v", err)
}
if err := repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-1", WorkspaceID: "ws-1", Name: "WF"}); err != nil {
t.Fatalf("CreateWorkflow: %v", err)
}
if err := repo.CreateRepository(ctx, &models.Repository{ID: "repo-x", WorkspaceID: "ws-1", Name: "Repo"}); err != nil {
t.Fatalf("CreateRepository: %v", err)
}
if err := repo.CreateTask(ctx, &models.Task{
ID: "task-1", WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", Title: "T", Priority: "medium",
}); err != nil {
t.Fatalf("CreateTask: %v", err)
}
if err := repo.CreateTaskRepository(ctx, &models.TaskRepository{
TaskID: "task-1", RepositoryID: "repo-x", BaseBranch: "main",
}); err != nil {
t.Fatalf("CreateTaskRepository: %v", err)
}
eventBus.ClearEvents()
// Mimic the orchestrator path: pass a task with Repositories nil.
task := &models.Task{
ID: "task-1", WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1",
}
if len(task.Repositories) != 0 {
t.Fatal("pre-condition: task.Repositories must be nil for this test")
}
svc.PublishTaskUpdated(ctx, task)
events := eventBus.GetPublishedEvents()
if len(events) != 1 {
t.Fatalf("expected 1 published event, got %d", len(events))
}
data, ok := events[0].Data.(map[string]interface{})
if !ok {
t.Fatalf("event Data wrong type: %T", events[0].Data)
}
got, ok := data["repository_id"].(string)
if !ok {
t.Fatalf("repository_id missing from payload or wrong type: %#v", data["repository_id"])
}
if got != "repo-x" {
t.Fatalf("expected repository_id=repo-x via DB fallback, got %q", got)
}
}
apps/web/lib/types/backend.ts
+7
View File
@@ -144,6 +144,13 @@ export type TaskEventPayload = {
priority?: number;
position?: number;
repository_id?: string;
repositories?: Array<{
id?: string;
repository_id: string;
base_branch?: string;
checkout_branch?: string;
position?: number;
}>;
primary_session_id?: string | null;
session_count?: number | null;
review_status?: "pending" | "approved" | "changes_requested" | "rejected" | null;
apps/web/lib/ws/handlers/kanban.ts
+8 -28
View File
@@ -2,6 +2,7 @@ import type { StoreApi } from "zustand";
import type { AppState } from "@/lib/state/store";
import type { WsHandlers } from "@/lib/ws/handlers/types";
import type { KanbanState } from "@/lib/state/slices/kanban/types";
import { mergeTaskRepositoryFields } from "@/lib/ws/handlers/task-repositories";
type KanbanTask = KanbanState["tasks"][number];
type KanbanStep = KanbanState["steps"][number];
@@ -18,23 +19,6 @@ type KanbanUpdateTask = {
is_ephemeral?: boolean;
};
function resolveRepositories(
task: KanbanUpdateTask,
existing: KanbanTask | undefined,
): KanbanTask["repositories"] {
if (task.repositories !== undefined) return task.repositories;
if (task.repository_id && task.repository_id !== existing?.repositoryId) return undefined;
return existing?.repositories;
}
function resolveRepositoryId(
task: KanbanUpdateTask,
repositories: KanbanTask["repositories"],
existing: KanbanTask | undefined,
): string | undefined {
return task.repository_id ?? repositories?.[0]?.repository_id ?? existing?.repositoryId;
}
export function registerKanbanHandlers(store: StoreApi<AppState>): WsHandlers {
return {
"kanban.update": (message) => {
@@ -60,7 +44,10 @@ export function registerKanbanHandlers(store: StoreApi<AppState>): WsHandlers {
.filter((task: KanbanUpdateTask) => !task.is_ephemeral)
.map((task: KanbanUpdateTask) => {
const existing = existingById.get(task.id);
const repositories = resolveRepositories(task, existing);
const repoFields = mergeTaskRepositoryFields(existing, {
repositoryId: task.repository_id,
repositories: task.repositories,
});
return {
id: task.id,
workflowStepId: task.workflowStepId,
@@ -68,8 +55,7 @@ export function registerKanbanHandlers(store: StoreApi<AppState>): WsHandlers {
description: task.description,
position: task.position ?? 0,
state: task.state,
repositoryId: resolveRepositoryId(task, repositories, existing),
repositories,
...repoFields,
primarySessionId: existing?.primarySessionId,
primarySessionState: existing?.primarySessionState,
};
@@ -86,20 +72,14 @@ export function registerKanbanHandlers(store: StoreApi<AppState>): WsHandlers {
const existingMultiById = new Map(snapshot.tasks.map((t) => [t.id, t]));
const multiTasks = tasks.map((t) => {
const fallback = existingMultiById.get(t.id);
const repositoryId =
t.repositoryId === undefined ? fallback?.repositoryId : t.repositoryId;
const repositories =
t.repositories !== undefined || repositoryId !== fallback?.repositoryId
? t.repositories
: fallback?.repositories;
const repoFields = mergeTaskRepositoryFields(fallback, t);
// Fall back to the multi-snapshot's own value only when the main
// kanban lookup returned `undefined` (task absent from kanban.tasks).
// An explicit `null` means the primary was intentionally cleared
// and must NOT be replaced by a stale snapshot value.
return {
...t,
repositoryId,
repositories,
...repoFields,
primarySessionId:
t.primarySessionId === undefined ? fallback?.primarySessionId : t.primarySessionId,
primarySessionState:
apps/web/lib/ws/handlers/task-repositories.test.ts
+68
View File
@@ -0,0 +1,68 @@
import { describe, expect, it } from "vitest";
import { mergeTaskRepositoryFields } from "./task-repositories";
const REPO_A = "repo-a";
const REPO_B = "repo-b";
function taskRepo(repositoryId: string) {
return {
id: `task-${repositoryId}`,
repository_id: repositoryId,
base_branch: "main",
position: 0,
};
}
describe("mergeTaskRepositoryFields", () => {
it("derives repositoryId from provided repositories when repositoryId is omitted", () => {
const repositories = [taskRepo(REPO_B)];
expect(
mergeTaskRepositoryFields(
{ repositoryId: REPO_A, repositories: undefined },
{ repositoryId: undefined, repositories },
),
).toEqual({ repositoryId: REPO_B, repositories });
});
it("clears repository fields when an empty repository list is provided", () => {
expect(
mergeTaskRepositoryFields(
{
repositoryId: REPO_A,
repositories: [taskRepo(REPO_A)],
},
{ repositoryId: undefined, repositories: [] },
),
).toEqual({ repositoryId: undefined, repositories: [] });
});
it("clears stale repositories when the primary repository changes", () => {
expect(
mergeTaskRepositoryFields(
{
repositoryId: REPO_A,
repositories: [taskRepo(REPO_A)],
},
{ repositoryId: REPO_B, repositories: undefined },
),
).toEqual({ repositoryId: REPO_B, repositories: undefined });
});
it("preserves existing repository fields when both fields are omitted", () => {
const existing = {
repositoryId: REPO_A,
repositories: [taskRepo(REPO_A)],
};
expect(
mergeTaskRepositoryFields(existing, { repositoryId: undefined, repositories: undefined }),
).toEqual(existing);
});
it("returns omitted fields when no existing task is available", () => {
expect(
mergeTaskRepositoryFields(undefined, { repositoryId: undefined, repositories: undefined }),
).toEqual({ repositoryId: undefined, repositories: undefined });
});
});
apps/web/lib/ws/handlers/task-repositories.ts
+23
View File
@@ -0,0 +1,23 @@
import type { KanbanState } from "@/lib/state/slices/kanban/types";
type KanbanTask = KanbanState["tasks"][number];
type TaskRepositoryFields = Pick<KanbanTask, "repositoryId" | "repositories">;
export function mergeTaskRepositoryFields(
existing: TaskRepositoryFields | undefined,
next: TaskRepositoryFields,
): TaskRepositoryFields {
const repositoriesProvided = next.repositories !== undefined;
const nextRepositoryId = next.repositoryId ?? next.repositories?.[0]?.repository_id;
const repositoryIdChanged =
nextRepositoryId !== undefined && nextRepositoryId !== existing?.repositoryId;
return {
repositoryId: repositoriesProvided
? nextRepositoryId
: (nextRepositoryId ?? existing?.repositoryId),
repositories:
repositoriesProvided || repositoryIdChanged ? next.repositories : existing?.repositories,
};
}
apps/web/lib/ws/handlers/tasks.test.ts
+129
View File
@@ -273,6 +273,135 @@ describe("task.updated cross-workflow placement", () => {
});
});
describe("task.updated repository preservation", () => {
it("preserves repository metadata when a rename update omits repo fields", () => {
const repo = {
id: "task-repo-1",
repository_id: "repo-a",
base_branch: "main",
checkout_branch: "feature/rename",
position: 0,
};
const existingTask = {
id: "t1",
workflowStepId: "step1",
title: "Old title",
position: 0,
repositoryId: "repo-a",
repositories: [repo],
};
const store = makeStore({
kanban: {
workflowId: "wf1",
steps: [],
tasks: [existingTask],
} as unknown as AppState["kanban"],
kanbanMulti: {
isLoading: false,
snapshots: {
wf1: { workflowId: "wf1", workflowName: "WF1", steps: [], tasks: [existingTask] },
},
} as unknown as AppState["kanbanMulti"],
});
const handlers = registerTasksHandlers(store);
handlers["task.updated"]!(
makeMessage({
...makeTask("t1", null),
title: "Renamed task",
repository_id: undefined,
repositories: undefined,
}),
);
const state = store.getState();
const kanbanTask = state.kanban.tasks.find((task) => task.id === "t1");
const snapshotTask = state.kanbanMulti.snapshots.wf1.tasks.find((task) => task.id === "t1");
expect(kanbanTask?.title).toBe("Renamed task");
expect(kanbanTask?.repositoryId).toBe("repo-a");
expect(kanbanTask?.repositories).toEqual([repo]);
expect(snapshotTask?.repositoryId).toBe("repo-a");
expect(snapshotTask?.repositories).toEqual([repo]);
});
it("does not preserve stale repository rows when the primary repository changes", () => {
const existingTask = {
id: "t1",
workflowStepId: "step1",
title: "Old title",
position: 0,
repositoryId: "repo-a",
repositories: [
{
id: "task-repo-1",
repository_id: "repo-a",
base_branch: "main",
position: 0,
},
],
};
const store = makeStore({
kanban: {
workflowId: "wf1",
steps: [],
tasks: [existingTask],
} as unknown as AppState["kanban"],
});
const handlers = registerTasksHandlers(store);
handlers["task.updated"]!(
makeMessage({
...makeTask("t1", null),
repository_id: "repo-b",
repositories: undefined,
}),
);
const task = store.getState().kanban.tasks.find((item) => item.id === "t1");
expect(task?.repositoryId).toBe("repo-b");
expect(task?.repositories).toBeUndefined();
});
});
describe("task.updated repository clearing", () => {
it("clears repository metadata when an update explicitly sends an empty repository list", () => {
const existingTask = {
id: "t1",
workflowStepId: "step1",
title: "Old title",
position: 0,
repositoryId: "repo-a",
repositories: [
{
id: "task-repo-1",
repository_id: "repo-a",
base_branch: "main",
position: 0,
},
],
};
const store = makeStore({
kanban: {
workflowId: "wf1",
steps: [],
tasks: [existingTask],
} as unknown as AppState["kanban"],
});
const handlers = registerTasksHandlers(store);
handlers["task.updated"]!(
makeMessage({
...makeTask("t1", null),
repositories: [],
}),
);
const task = store.getState().kanban.tasks.find((item) => item.id === "t1");
expect(task?.repositoryId).toBeUndefined();
expect(task?.repositories).toEqual([]);
});
});
describe("task.deleted cleanup", () => {
it("removes the deleted task from recent task history", () => {
const store = makeStore({
apps/web/lib/ws/handlers/tasks.ts
+14 -4
View File
@@ -7,14 +7,24 @@ import { removeRecentTask } from "@/lib/recent-tasks";
import { useContextFilesStore } from "@/lib/state/context-files-store";
import { toKanbanTask, type TaskLike } from "@/lib/kanban/map-task";
import { sessionId as toSessionId } from "@/lib/types/http";
import { mergeTaskRepositoryFields } from "@/lib/ws/handlers/task-repositories";
type KanbanTask = KanbanState["tasks"][number];
function mergeTaskUpdate(existing: KanbanTask | undefined, nextTask: KanbanTask): KanbanTask {
if (!existing) return nextTask;
return {
...nextTask,
...mergeTaskRepositoryFields(existing, nextTask),
};
}
function upsertTask(tasks: KanbanTask[], nextTask: KanbanTask): KanbanTask[] {
const exists = tasks.some((task) => task.id === nextTask.id);
return exists
? tasks.map((task) => (task.id === nextTask.id ? nextTask : task))
: [...tasks, nextTask];
const existing = tasks.find((task) => task.id === nextTask.id);
const merged = mergeTaskUpdate(existing, nextTask);
return existing
? tasks.map((task) => (task.id === nextTask.id ? merged : task))
: [...tasks, merged];
}
function upsertMultiTask(state: AppState, workflowId: string, task: KanbanTask): AppState {