From 1d70a0df6dd84bec434bd364d389b2fc249eacf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Sat, 1 Aug 2026 19:42:23 +0100 Subject: [PATCH] feat: enforce task title length limit (#2134) --- apps/backend/config/prompts/kandev-context.md | 2 +- .../mcp/handlers/config_handlers_test.go | 28 ++++ .../backend/internal/mcp/handlers/handlers.go | 5 + .../internal/mcp/handlers/handlers_test.go | 48 ++++++ .../internal/mcp/server/handlers_test.go | 25 +++ apps/backend/internal/mcp/server/server.go | 5 +- .../internal/office/onboarding/handler.go | 8 +- .../internal/office/onboarding/service.go | 4 + .../office/onboarding/service_test.go | 19 +++ .../internal/office/routines/service.go | 3 +- .../internal/office/routines/service_test.go | 34 +++++ .../orchestrator/event_handlers_automation.go | 7 +- .../event_handlers_automation_test.go | 17 +++ apps/backend/internal/task/handlers/errors.go | 3 + .../task/handlers/task_http_handlers_test.go | 58 +++++++ .../handlers/task_update_repositories_test.go | 30 ++++ .../task/handlers/task_ws_handlers.go | 3 + .../internal/task/service/service_tasks.go | 8 + .../internal/task/service/task_title.go | 36 +++++ .../internal/task/service/task_title_test.go | 143 ++++++++++++++++++ .../app/office/components/new-task-dialog.tsx | 3 +- .../app/office/components/new-task-draft.ts | 6 +- apps/web/app/office/setup/step-task.tsx | 3 +- .../automation-editor-sections.tsx | 5 +- .../azure-devops-task-launcher.tsx | 5 +- .../github/my-github/quick-task-launcher.tsx | 3 +- .../gitlab/my-gitlab/quick-task-launcher.tsx | 3 +- .../jira/my-jira/quick-task-launcher.tsx | 3 +- .../linear/linear-quick-task-launcher.tsx | 3 +- .../task-create-dialog-handlers.test.ts | 21 +++ .../components/task-create-dialog-handlers.ts | 6 +- .../task-create-dialog-selectors.tsx | 3 +- .../task-create-dialog-state.test.ts | 23 +++ .../components/task-create-dialog-state.ts | 20 ++- .../task-create-dialog-submit.test.tsx | 23 +++ .../components/task-create-dialog-submit.tsx | 3 +- apps/web/components/task-create-dialog.tsx | 24 +-- .../task/new-subtask-form-parts.tsx | 3 +- .../components/task/task-rename-dialog.tsx | 3 +- .../components/task/task-top-bar-title.tsx | 3 +- apps/web/e2e/helpers/topbar-fixtures.ts | 5 +- .../pr-action-create-task-dialog.spec.ts | 16 +- .../layout/compact-desktop-responsive.spec.ts | 2 +- .../mobile-create-task-remote-repo.spec.ts | 33 +++- .../task/sidebar-scroll-preservation.spec.ts | 3 +- apps/web/e2e/tests/task/topbar-rename.spec.ts | 3 +- apps/web/lib/task-title.test.ts | 37 +++++ apps/web/lib/task-title.ts | 17 +++ docs/plans/task-title-length-limit/plan.md | 82 ++++++++++ .../task-01-backend-mcp-contract.md | 67 ++++++++ .../task-02-frontend-title-limit.md | 69 +++++++++ .../task-03-e2e-docs.md | 63 ++++++++ docs/public/automation-and-mcp.md | 2 + docs/public/tasks-and-workflows.md | 2 +- docs/specs/INDEX.md | 1 + docs/specs/tasks/title-length-limit.md | 53 +++++++ 56 files changed, 1050 insertions(+), 57 deletions(-) create mode 100644 apps/backend/internal/task/service/task_title.go create mode 100644 apps/backend/internal/task/service/task_title_test.go create mode 100644 apps/web/lib/task-title.test.ts create mode 100644 apps/web/lib/task-title.ts create mode 100644 docs/plans/task-title-length-limit/plan.md create mode 100644 docs/plans/task-title-length-limit/task-01-backend-mcp-contract.md create mode 100644 docs/plans/task-title-length-limit/task-02-frontend-title-limit.md create mode 100644 docs/plans/task-title-length-limit/task-03-e2e-docs.md create mode 100644 docs/specs/tasks/title-length-limit.md diff --git a/apps/backend/config/prompts/kandev-context.md b/apps/backend/config/prompts/kandev-context.md index 287e68284f..e11840c561 100644 --- a/apps/backend/config/prompts/kandev-context.md +++ b/apps/backend/config/prompts/kandev-context.md @@ -14,7 +14,7 @@ Available tools: - list_workspaces_kandev: List all workspaces. - list_workflows_kandev: List workflows in a workspace. Required params: workspace_id. - list_tasks_kandev: List tasks in a workflow. Required params: workflow_id. -- create_task_kandev: Create a new task or subtask. Required params: title. For subtasks, set parent_id to the literal string "self" (the MCP server expands it to your current task ID) and omit workspace_id/workflow_id/workflow_step_id; they inherit from the parent. Pass workspace_id/workflow_id on a subtask only when deliberately targeting another task workspace/workflow; any supplied workflow_id must belong to the effective workspace_id. MCP subtasks reuse the parent's materialized workspace by default; set workspace_mode to "new_workspace" only when the subtask should launch in its own worktree/materialized workspace. For top-level tasks, provide workspace_id/workflow_id unless each can be auto-resolved uniquely. workflow_step_id is optional. +- create_task_kandev: Create a new task or subtask. Required params: title (keep it concise, a few words, and no more than 60 characters; put detailed context in description). For subtasks, set parent_id to the literal string "self" (the MCP server expands it to your current task ID) and omit workspace_id/workflow_id/workflow_step_id; they inherit from the parent. Pass workspace_id/workflow_id on a subtask only when deliberately targeting another task workspace/workflow; any supplied workflow_id must belong to the effective workspace_id. MCP subtasks reuse the parent's materialized workspace by default; set workspace_mode to "new_workspace" only when the subtask should launch in its own worktree/materialized workspace. For top-level tasks, provide workspace_id/workflow_id unless each can be auto-resolved uniquely. workflow_step_id is optional. - update_task_kandev: Update a task. Required params: task_id. - spawn_session_kandev: Spawn an ADDITIONAL agent session on your current task (no new task is created — it runs alongside your session in the same workspace). Required params: prompt (the new session's ONLY initial context). Optional: agent_profile_id (defaults to your profile; specify a different one to spawn a different agent), name (session tab label, e.g. "reviewer"), task_id (defaults to your task). Returns the new session_id. - message_task_kandev: Message another task's agent, or a specific session via optional session_id — including a sibling session on your OWN task. Required params: task_id, prompt.{coordinator_task_control_section} diff --git a/apps/backend/internal/mcp/handlers/config_handlers_test.go b/apps/backend/internal/mcp/handlers/config_handlers_test.go index 8471c40cb2..a00f4e3091 100644 --- a/apps/backend/internal/mcp/handlers/config_handlers_test.go +++ b/apps/backend/internal/mcp/handlers/config_handlers_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "strings" "testing" "time" @@ -567,6 +568,33 @@ func TestHandleUpdateTask_PersistsState(t *testing.T) { assert.Equal(t, v1.TaskStateCompleted, task.State) } +func TestHandleUpdateTask_TitleTooLongReturnsValidation(t *testing.T) { + svc, repo := newTestTaskService(t) + ctx := context.Background() + now := time.Now().UTC() + + require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ + ID: "ws-update-title", Name: "Update Title", CreatedAt: now, UpdatedAt: now, + })) + require.NoError(t, repo.CreateWorkflow(ctx, &models.Workflow{ + ID: "wf-update-title", WorkspaceID: "ws-update-title", Name: "Board", CreatedAt: now, UpdatedAt: now, + })) + require.NoError(t, repo.CreateTask(ctx, &models.Task{ + ID: "task-update-title", WorkspaceID: "ws-update-title", WorkflowID: "wf-update-title", + Title: "Original", State: v1.TaskStateTODO, CreatedAt: now, UpdatedAt: now, + })) + + h := &Handlers{taskSvc: svc, logger: testLogger(t).WithFields()} + msg := makeWSMessage(t, ws.ActionMCPUpdateTask, map[string]interface{}{ + "task_id": "task-update-title", + "title": strings.Repeat("x", service.TaskTitleMaxLength+1), + }) + + resp, err := h.handleUpdateTask(ctx, msg) + require.NoError(t, err) + assertWSError(t, resp, ws.ErrorCodeValidation) +} + // TestHandleMoveTask_ActiveSessionWithoutPrompt_DefersMove pins the production // bug where an agent on Work called move_task_kandev → Done without a prompt // mid-turn. The immediate path hit validateMoveSessions (RUNNING session) and diff --git a/apps/backend/internal/mcp/handlers/handlers.go b/apps/backend/internal/mcp/handlers/handlers.go index 699110c967..ce8d0ebf88 100644 --- a/apps/backend/internal/mcp/handlers/handlers.go +++ b/apps/backend/internal/mcp/handlers/handlers.go @@ -697,6 +697,8 @@ func classifyCreateTaskError(err error) string { switch { case errors.Is(err, service.ErrWIPLimitExceeded): return ws.ErrorCodeConflict + case errors.Is(err, service.ErrTaskTitleTooLong): + return ws.ErrorCodeValidation case errors.Is(err, service.ErrSubtaskDepthExceeded), errors.Is(err, service.ErrInvalidTaskWorkflow), isMCPWorkflowNotFoundError(err): @@ -1303,6 +1305,9 @@ func (h *Handlers) handleUpdateTask(ctx context.Context, msg *ws.Message) (*ws.M }) if err != nil { h.logger.Error("failed to update task", zap.Error(err)) + if errors.Is(err, service.ErrTaskTitleTooLong) { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil) + } return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "Failed to update task", nil) } diff --git a/apps/backend/internal/mcp/handlers/handlers_test.go b/apps/backend/internal/mcp/handlers/handlers_test.go index 606dc4ccbf..33224c7152 100644 --- a/apps/backend/internal/mcp/handlers/handlers_test.go +++ b/apps/backend/internal/mcp/handlers/handlers_test.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "path/filepath" + "strings" "sync" "testing" "time" @@ -203,6 +204,53 @@ func TestClassifyCreateTaskErrorMapsWIPLimitToConflict(t *testing.T) { } } +func TestClassifyCreateTaskErrorMapsOverlongTitleToValidation(t *testing.T) { + if got := classifyCreateTaskError(service.ErrTaskTitleTooLong); got != ws.ErrorCodeValidation { + t.Fatalf("expected ErrorCodeValidation, got %q", got) + } +} + +func TestHandleCreateTask_RejectsOverlongTitle(t *testing.T) { + svc, repo := newTestTaskService(t) + ctx := context.Background() + workspaces, err := svc.ListWorkspaces(ctx) + require.NoError(t, err) + require.Len(t, workspaces, 1) + workflows, err := svc.ListWorkflows(ctx, workspaces[0].ID, false) + require.NoError(t, err) + require.Len(t, workflows, 1) + + source, err := svc.CreateTask(ctx, &service.CreateTaskRequest{ + WorkspaceID: workspaces[0].ID, + WorkflowID: workflows[0].ID, + Title: "Source task", + }) + require.NoError(t, err) + require.NoError(t, repo.CreateTaskSession(ctx, &models.TaskSession{ + ID: "source-title-session", + TaskID: source.ID, + AgentProfileID: "source-profile", + ExecutorProfileID: "source-executor-profile", + State: models.TaskSessionStateWaitingForInput, + IsPrimary: true, + })) + + h := &Handlers{taskSvc: svc, logger: testLogger(t).WithFields()} + resp, err := h.handleCreateTask(ctx, makeWSMessage(t, ws.ActionMCPCreateTask, map[string]interface{}{ + "source_task_id": source.ID, + "workspace_id": workspaces[0].ID, + "workflow_id": workflows[0].ID, + "title": strings.Repeat("x", service.TaskTitleMaxLength+1), + "start_agent": false, + })) + require.NoError(t, err) + assertWSError(t, resp, ws.ErrorCodeValidation) + + tasks, err := svc.ListTasks(ctx, workflows[0].ID) + require.NoError(t, err) + assert.Len(t, tasks, 1, "validation failure must not persist a task") +} + // TestHandleAddBranchToTask_RejectsMultipleLocators pins the mutual-exclusion // check at the WS handler tier: supplying two of repository_id / // repository_url / local_path is an agent mistake that previously got diff --git a/apps/backend/internal/mcp/server/handlers_test.go b/apps/backend/internal/mcp/server/handlers_test.go index 7e1030c35f..f999cba0b7 100644 --- a/apps/backend/internal/mcp/server/handlers_test.go +++ b/apps/backend/internal/mcp/server/handlers_test.go @@ -5,6 +5,7 @@ import ( "errors" "testing" + "github.com/kandev/kandev/internal/task/service" ws "github.com/kandev/kandev/pkg/websocket" "github.com/mark3labs/mcp-go/mcp" "github.com/stretchr/testify/assert" @@ -35,6 +36,13 @@ func TestCreateTask_ToolSchema_HasParentID(t *testing.T) { require.True(t, ok, "schema should have properties") assert.Contains(t, props, "parent_id", "create_task schema must expose parent_id") assert.Contains(t, props, "title") + titleProp, ok := props["title"].(map[string]interface{}) + require.True(t, ok, "title should be an object") + assert.Equal(t, float64(service.TaskTitleMaxLength), titleProp["maxLength"]) + titleDesc, ok := titleProp["description"].(string) + require.True(t, ok, "title should have a description") + assert.Contains(t, titleDesc, "concise") + assert.Contains(t, titleDesc, "60") assert.Contains(t, props, "workspace_id") assert.Contains(t, props, "workflow_id") assert.Contains(t, props, "workflow_step_id") @@ -96,6 +104,23 @@ func TestCreateTask_ToolSchema_HasParentID(t *testing.T) { assert.False(t, requiredSet["workflow_id"], "workflow_id should not be required") } +func TestUpdateTask_ToolSchema_HasTitleMaxLength(t *testing.T) { + s := newTaskModeServer(t, &testBackend{}, "task-current") + + tool, ok := s.mcpServer.ListTools()["update_task_kandev"] + require.True(t, ok, "update_task tool not registered") + schema, err := json.Marshal(tool.Tool.InputSchema) + require.NoError(t, err) + + var parsed map[string]interface{} + require.NoError(t, json.Unmarshal(schema, &parsed)) + props, ok := parsed["properties"].(map[string]interface{}) + require.True(t, ok, "schema should have properties") + titleProp, ok := props["title"].(map[string]interface{}) + require.True(t, ok, "title should be an object") + assert.Equal(t, float64(service.TaskTitleMaxLength), titleProp["maxLength"]) +} + func TestCreateTask_PromptCanonical(t *testing.T) { backend := &testBackend{ response: map[string]interface{}{"id": "subtask-1", "parent_id": "task-current"}, diff --git a/apps/backend/internal/mcp/server/server.go b/apps/backend/internal/mcp/server/server.go index ea66c5f951..d75ca5201c 100644 --- a/apps/backend/internal/mcp/server/server.go +++ b/apps/backend/internal/mcp/server/server.go @@ -14,6 +14,7 @@ import ( "github.com/gin-gonic/gin" "github.com/kandev/kandev/internal/agentctl/types/streams" "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/task/service" ws "github.com/kandev/kandev/pkg/websocket" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" @@ -574,7 +575,7 @@ func (s *Server) registerKanbanTools() { mcp.NewTool("update_task_kandev", mcp.WithDescription("Update an existing task."), mcp.WithString("task_id", mcp.Required(), mcp.Description("The task ID")), - mcp.WithString("title", mcp.Description("New title")), + mcp.WithString("title", mcp.MaxLength(service.TaskTitleMaxLength), mcp.Description("New concise task title (maximum 60 characters)")), mcp.WithString("description", mcp.Description("New description")), mcp.WithString("state", mcp.Description("New state: not_started, in_progress, etc.")), ), @@ -741,7 +742,7 @@ IMPORTANT: mcp.WithString("workflow_id", mcp.Description("The workflow ID. Auto-resolved if the workspace has only one workflow. Defaulted from parent for subtasks when workspace_id is also omitted; if supplied, it must belong to the effective workspace_id.")), mcp.WithString("workflow_step_id", mcp.Description("The workflow step ID (optional, auto-resolved if omitted; for subtasks, pass only with an explicit workflow_id)")), mcp.WithString("workspace_mode", mcp.Description("Subtask materialized-workspace mode: inherit_parent reuses the parent's worktree/materialized workspace (default for subtasks); new_workspace launches the subtask in its own workspace/worktree.")), - mcp.WithString("title", mcp.Required(), mcp.Description("The task title")), + mcp.WithString("title", mcp.Required(), mcp.MaxLength(service.TaskTitleMaxLength), mcp.Description("A concise, few-word task title (maximum 60 characters).")), mcp.WithString("prompt", mcp.Description("The initial prompt for the sub-agent. This is the ONLY context the agent receives when it starts — treat it as the agent's first user message. For auto-started subtasks, provide a specific and detailed prompt; omitting it starts the sub-agent without task-specific context.")), mcp.WithString("agent_profile_id", mcp.Description(agentProfileDesc)), mcp.WithString("executor_profile_id", mcp.Description("Executor profile ID to use (determines the runtime environment: local, worktree, docker, etc.). For subtasks, inherited from the parent session. For top-level tasks, ask the user which executor profile they want if not already known.")), diff --git a/apps/backend/internal/office/onboarding/handler.go b/apps/backend/internal/office/onboarding/handler.go index d16bf24ce6..27a79bceb4 100644 --- a/apps/backend/internal/office/onboarding/handler.go +++ b/apps/backend/internal/office/onboarding/handler.go @@ -1,10 +1,12 @@ package onboarding import ( + "errors" "net/http" "github.com/gin-gonic/gin" "github.com/kandev/kandev/internal/common/logger" + taskservice "github.com/kandev/kandev/internal/task/service" "go.uber.org/zap" ) @@ -66,7 +68,11 @@ func (h *Handler) completeOnboarding(c *gin.Context) { result, err := h.svc.CompleteOnboarding(c.Request.Context(), CompleteRequest(req)) if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + status := http.StatusInternalServerError + if errors.Is(err, taskservice.ErrTaskTitleTooLong) { + status = http.StatusBadRequest + } + c.JSON(status, gin.H{"error": err.Error()}) return } c.JSON(http.StatusCreated, OnboardingCompleteResponse{ diff --git a/apps/backend/internal/office/onboarding/service.go b/apps/backend/internal/office/onboarding/service.go index 35417f6fe1..a085fc92e0 100644 --- a/apps/backend/internal/office/onboarding/service.go +++ b/apps/backend/internal/office/onboarding/service.go @@ -17,6 +17,7 @@ import ( "github.com/kandev/kandev/internal/office/repository/sqlite" "github.com/kandev/kandev/internal/office/routing" "github.com/kandev/kandev/internal/office/shared" + taskservice "github.com/kandev/kandev/internal/task/service" "go.uber.org/zap" ) @@ -265,6 +266,9 @@ func (s *OnboardingService) unimportedFSWorkspaces(ctx context.Context) []FSWork // and marks onboarding as finished. func (s *OnboardingService) CompleteOnboarding(ctx context.Context, req CompleteRequest) (*CompleteResult, error) { result := &CompleteResult{} + if err := taskservice.ValidateTaskTitle(req.TaskTitle); err != nil { + return nil, fmt.Errorf("invalid onboarding task title: %w", err) + } if err := s.createOnboardingWorkspace(ctx, req.WorkspaceName, req.TaskPrefix); err != nil { return nil, fmt.Errorf("create workspace: %w", err) diff --git a/apps/backend/internal/office/onboarding/service_test.go b/apps/backend/internal/office/onboarding/service_test.go index e1af94f3a1..e138fbf3c0 100644 --- a/apps/backend/internal/office/onboarding/service_test.go +++ b/apps/backend/internal/office/onboarding/service_test.go @@ -2,6 +2,7 @@ package onboarding import ( "context" + "errors" "os" "path/filepath" "runtime" @@ -18,6 +19,7 @@ import ( "github.com/kandev/kandev/internal/office/models" "github.com/kandev/kandev/internal/office/repository/sqlite" "github.com/kandev/kandev/internal/office/routing" + taskservice "github.com/kandev/kandev/internal/task/service" ) // --- mock implementations --- @@ -267,6 +269,23 @@ func TestCompleteOnboarding_CreatesEntities(t *testing.T) { } } +func TestCompleteOnboardingRejectsOverlongTaskTitleBeforeSideEffects(t *testing.T) { + svc, wsCreator, _ := newTestOnboardingService(t) + longTitle := strings.Repeat("x", taskservice.TaskTitleMaxLength+1) + + _, err := svc.CompleteOnboarding(context.Background(), CompleteRequest{ + WorkspaceName: "too-long-title", + AgentName: "CEO", + TaskTitle: longTitle, + }) + if !errors.Is(err, taskservice.ErrTaskTitleTooLong) { + t.Fatalf("CompleteOnboarding error = %v, want ErrTaskTitleTooLong", err) + } + if _, ok := wsCreator.workspaces["too-long-title"]; ok { + t.Fatal("CompleteOnboarding created a workspace before rejecting the title") + } +} + func TestCreateOnboardingAgent_KeepsRuntimeConfigurationIndirect(t *testing.T) { svc, _, _ := newTestOnboardingService(t) source := &models.AgentInstance{ diff --git a/apps/backend/internal/office/routines/service.go b/apps/backend/internal/office/routines/service.go index 8acce178b3..cae286413e 100644 --- a/apps/backend/internal/office/routines/service.go +++ b/apps/backend/internal/office/routines/service.go @@ -13,6 +13,7 @@ import ( "github.com/kandev/kandev/internal/common/logger" "github.com/kandev/kandev/internal/office/models" "github.com/kandev/kandev/internal/office/shared" + taskservice "github.com/kandev/kandev/internal/task/service" ) // Repository is the persistence interface required by RoutineService. @@ -499,7 +500,7 @@ func (s *RoutineService) dispatchRoutineRun( vars := shared.ResolveVariables(now, defaults, provided) tmpl := parseTaskTemplate(routine.TaskTemplate) - title := shared.InterpolateTemplate(tmpl.Title, vars) + title := taskservice.TruncateTaskTitle(shared.InterpolateTemplate(tmpl.Title, vars)) description := shared.InterpolateTemplate(tmpl.Description, vars) fingerprint := computeFingerprint(title, description, routine.AssigneeAgentProfileID) diff --git a/apps/backend/internal/office/routines/service_test.go b/apps/backend/internal/office/routines/service_test.go index 7dff1ada58..c5fea1629d 100644 --- a/apps/backend/internal/office/routines/service_test.go +++ b/apps/backend/internal/office/routines/service_test.go @@ -12,6 +12,7 @@ import ( "github.com/kandev/kandev/internal/office/models" "github.com/kandev/kandev/internal/office/repository/sqlite" "github.com/kandev/kandev/internal/office/routines" + taskservice "github.com/kandev/kandev/internal/task/service" ) // noopActivity implements shared.ActivityLogger for tests. @@ -327,6 +328,39 @@ func TestDispatch_HeavyRoutine_CreatesTaskInRoutineWorkflow(t *testing.T) { } } +func TestDispatch_HeavyRoutine_TruncatesRenderedTitle(t *testing.T) { + svc := newTestRoutineService(t) + ctx := context.Background() + longName := strings.Repeat("x", taskservice.TaskTitleMaxLength+10) + routine := &models.Routine{ + WorkspaceID: "ws-1", + Name: "Long title", + TaskTemplate: `{"title":"{{name}}","description":"Details: {{name}}"}`, + AssigneeAgentProfileID: "agent-1", + Status: "active", + ConcurrencyPolicy: "always_create", + Variables: `{"name":{"default":"short"}}`, + } + if err := svc.CreateRoutine(ctx, routine); err != nil { + t.Fatalf("create routine: %v", err) + } + + wf := &fakeWorkflowEnsurer{} + tc := &fakeTaskCreator{} + svc.SetWorkflowEnsurer(wf) + svc.SetTaskCreator(tc) + if _, err := svc.FireManual(ctx, routine.ID, map[string]string{"name": longName}); err != nil { + t.Fatalf("fire manual: %v", err) + } + + if gotLen := len([]rune(tc.captured.title)); gotLen != taskservice.TaskTitleMaxLength { + t.Fatalf("captured title rune length = %d, want %d", gotLen, taskservice.TaskTitleMaxLength) + } + if !strings.Contains(tc.captured.description, longName) { + t.Fatalf("description = %q, want full rendered value", tc.captured.description) + } +} + // TestDispatch_HeavyRoutine_FallsBackWithoutDeps confirms that a heavy // (task_template set) routine degrades to lightweight behaviour when // the workflow ensurer or task creator is not wired — no task is diff --git a/apps/backend/internal/orchestrator/event_handlers_automation.go b/apps/backend/internal/orchestrator/event_handlers_automation.go index b6eb6a770e..aeb95c3f23 100644 --- a/apps/backend/internal/orchestrator/event_handlers_automation.go +++ b/apps/backend/internal/orchestrator/event_handlers_automation.go @@ -14,6 +14,7 @@ import ( "github.com/kandev/kandev/internal/events/bus" "github.com/kandev/kandev/internal/github" "github.com/kandev/kandev/internal/task/models" + taskservice "github.com/kandev/kandev/internal/task/service" "github.com/kandev/kandev/internal/worktree" ) @@ -310,17 +311,17 @@ func (s *Service) resolveAutomationTaskTitle(a *automation.Automation, evt *auto if a.TaskTitleTemplate != "" { title := automation.InterpolatePrompt(a.TaskTitleTemplate, evt.TriggerType, evt.TriggerData) if title != "" { - return title + return taskservice.TruncateTaskTitle(title) } } // Fall back to trigger type default from registry. if info := automation.GetTriggerTypeInfo(evt.TriggerType); info != nil && info.DefaultTaskTitle != "" { title := automation.InterpolatePrompt(info.DefaultTaskTitle, evt.TriggerType, evt.TriggerData) if title != "" { - return title + return taskservice.TruncateTaskTitle(title) } } - return fmt.Sprintf("[Auto] %s", a.Name) + return taskservice.TruncateTaskTitle(fmt.Sprintf("[Auto] %s", a.Name)) } // associateAutomationPR links a task to a GitHub PR using trigger data. diff --git a/apps/backend/internal/orchestrator/event_handlers_automation_test.go b/apps/backend/internal/orchestrator/event_handlers_automation_test.go index 4072f7ad44..0904dec47a 100644 --- a/apps/backend/internal/orchestrator/event_handlers_automation_test.go +++ b/apps/backend/internal/orchestrator/event_handlers_automation_test.go @@ -3,12 +3,14 @@ package orchestrator import ( "context" "encoding/json" + "strings" "testing" "time" "github.com/kandev/kandev/internal/automation" "github.com/kandev/kandev/internal/task/models" sqliterepo "github.com/kandev/kandev/internal/task/repository/sqlite" + taskservice "github.com/kandev/kandev/internal/task/service" ) // seedAutomationWorkspaceRepos creates a workspace with the given repository @@ -89,6 +91,21 @@ func TestResolveAutomationRepository_EmptyListFallsBackToWorkspace(t *testing.T) } } +func TestResolveAutomationTaskTitleTruncatesRenderedTitle(t *testing.T) { + svc := &Service{} + longTitle := strings.Repeat("x", taskservice.TaskTitleMaxLength+20) + a := &automation.Automation{TaskTitleTemplate: "{{pr.title}}"} + evt := &automation.AutomationTriggeredEvent{ + TriggerType: automation.TriggerTypeGitHubPR, + TriggerData: json.RawMessage(`{"title":"` + longTitle + `"}`), + } + + got := svc.resolveAutomationTaskTitle(a, evt) + if got != strings.Repeat("x", taskservice.TaskTitleMaxLength-1)+"…" { + t.Fatalf("resolved title = %q, want rendered title truncated with ellipsis", got) + } +} + // TestResolveAutomationRepository_GitHubPRIgnoresConfiguredRepositoryIDs is // the regression guard for a CodeRabbit review finding on PR #2077: the // frontend disables (but does not clear) the repository picker for diff --git a/apps/backend/internal/task/handlers/errors.go b/apps/backend/internal/task/handlers/errors.go index 18ae1a8326..f9dc8f162f 100644 --- a/apps/backend/internal/task/handlers/errors.go +++ b/apps/backend/internal/task/handlers/errors.go @@ -79,6 +79,9 @@ func isValidationError(err error) bool { if errors.Is(err, service.ErrInvalidParent) { return true } + if errors.Is(err, service.ErrTaskTitleTooLong) { + return true + } msg := strings.ToLower(err.Error()) return strings.Contains(msg, "pending approval") || strings.Contains(msg, "validation") || diff --git a/apps/backend/internal/task/handlers/task_http_handlers_test.go b/apps/backend/internal/task/handlers/task_http_handlers_test.go index 166e0f4462..53ca515b2e 100644 --- a/apps/backend/internal/task/handlers/task_http_handlers_test.go +++ b/apps/backend/internal/task/handlers/task_http_handlers_test.go @@ -432,6 +432,34 @@ func TestHTTPCreateTask_ProjectIDReachesOfficePath(t *testing.T) { assert.Equal(t, "wf-office", repo.captured.WorkflowID, "office workflow should be auto-resolved") } +func TestHTTPCreateTaskRejectsOverlongTitle(t *testing.T) { + gin.SetMode(gin.TestMode) + log := newTestLogger(t) + repo := &captureCreateTaskRepo{} + 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{}) + h := &TaskHandlers{service: svc, logger: log} + + rec := httptest.NewRecorder() + c, _ := gin.CreateTestContext(rec) + c.Request = httptest.NewRequest(http.MethodPost, "/tasks", strings.NewReader(`{ + "workspace_id": "ws-1", + "workflow_id": "wf-1", + "title": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + }`)) + c.Request.Header.Set("Content-Type", "application/json") + + h.httpCreateTask(c) + + assert.Equal(t, http.StatusBadRequest, rec.Code, "body: %s", rec.Body.String()) + assert.Nil(t, repo.captured, "overlong title must not reach repository persistence") +} + func TestHTTPCreateTaskRecordsFinalLastUsedSelections(t *testing.T) { gin.SetMode(gin.TestMode) log := newTestLogger(t) @@ -642,6 +670,36 @@ func TestWSCreateTaskRecordsFinalLastUsedSelections(t *testing.T) { }, recorder.got) } +func TestWSCreateTaskReturnsValidationErrorForOverlongTitle(t *testing.T) { + log := newTestLogger(t) + repo := &captureCreateTaskRepo{} + 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{}) + h := &TaskHandlers{service: svc, logger: log} + + msg, err := ws.NewRequest("msg-title", ws.ActionTaskCreate, map[string]any{ + "workspace_id": "ws-1", + "workflow_id": "wf-1", + "title": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", + }) + require.NoError(t, err) + + resp, err := h.wsCreateTask(context.Background(), msg) + + require.NoError(t, err) + require.Equal(t, ws.MessageTypeError, resp.Type) + var payload ws.ErrorPayload + require.NoError(t, json.Unmarshal(resp.Payload, &payload)) + assert.Equal(t, ws.ErrorCodeValidation, payload.Code) + assert.Contains(t, payload.Message, "60") + assert.Nil(t, repo.captured, "overlong title must not reach repository persistence") +} + func TestWSCreateTaskRecordsFreshBranchRequestBase(t *testing.T) { log := newTestLogger(t) diff --git a/apps/backend/internal/task/handlers/task_update_repositories_test.go b/apps/backend/internal/task/handlers/task_update_repositories_test.go index 45ecd4b0e0..580c86d283 100644 --- a/apps/backend/internal/task/handlers/task_update_repositories_test.go +++ b/apps/backend/internal/task/handlers/task_update_repositories_test.go @@ -133,6 +133,36 @@ func TestWSUpdateTaskTitleOnlyPreservesRepositories(t *testing.T) { assert.Equal(t, []string{"repo-1"}, decodeTaskRepositoryIDs(t, resp.Payload)) } +func TestHTTPUpdateTaskRejectsOverlongTitle(t *testing.T) { + gin.SetMode(gin.TestMode) + h, repo := newUpdateTaskHandlers(t) + + rec := doUpdateTaskRequest(t, h, `{"title":"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"}`) + + assert.Equal(t, http.StatusBadRequest, rec.Code, "body: %s", rec.Body.String()) + assert.False(t, repo.deleteReposCalled, "rejected title must not touch task repositories") +} + +func TestWSUpdateTaskReturnsValidationErrorForOverlongTitle(t *testing.T) { + h, repo := newUpdateTaskHandlers(t) + + msg, err := ws.NewRequest("msg-title", ws.ActionTaskUpdate, map[string]any{ + "id": "task-1", + "title": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", + }) + require.NoError(t, err) + + resp, err := h.wsUpdateTask(context.Background(), msg) + + require.NoError(t, err) + require.Equal(t, ws.MessageTypeError, resp.Type) + var payload ws.ErrorPayload + require.NoError(t, json.Unmarshal(resp.Payload, &payload)) + assert.Equal(t, ws.ErrorCodeValidation, payload.Code) + assert.Contains(t, payload.Message, "60") + assert.False(t, repo.deleteReposCalled, "rejected title must not touch task repositories") +} + func TestConvertUpdateRepositories(t *testing.T) { assert.Nil(t, convertUpdateRepositories(false, nil), "absent field must stay nil") diff --git a/apps/backend/internal/task/handlers/task_ws_handlers.go b/apps/backend/internal/task/handlers/task_ws_handlers.go index 0d3a88ac1d..8c194c3311 100644 --- a/apps/backend/internal/task/handlers/task_ws_handlers.go +++ b/apps/backend/internal/task/handlers/task_ws_handlers.go @@ -331,6 +331,9 @@ func (h *TaskHandlers) wsUpdateTask(ctx context.Context, msg *ws.Message) (*ws.M }) if err != nil { h.logger.Error("failed to update task", zap.Error(err)) + if isValidationError(err) { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil) + } return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "Failed to update task", nil) } return ws.NewResponse(msg.ID, msg.Action, dto.FromTask(task)) diff --git a/apps/backend/internal/task/service/service_tasks.go b/apps/backend/internal/task/service/service_tasks.go index 7a706b850f..ec9ea5b187 100644 --- a/apps/backend/internal/task/service/service_tasks.go +++ b/apps/backend/internal/task/service/service_tasks.go @@ -285,6 +285,9 @@ func (s *Service) inheritParentRepositories(ctx context.Context, req *CreateTask // validateCreateTaskRequest validates constraints for task creation. func (s *Service) validateCreateTaskRequest(req *CreateTaskRequest) error { + if err := validateTaskTitle(req.Title); err != nil { + return err + } isOffice := isOfficeRequest(req) if !req.IsEphemeral && !isOffice && req.WorkflowID == "" { return fmt.Errorf("workflow_id is required for non-ephemeral tasks") @@ -1126,6 +1129,11 @@ func (s *Service) UpdateTask(ctx context.Context, id string, req *UpdateTaskRequ if err := s.authorizeTaskID(ctx, id); err != nil { return nil, err } + if req.Title != nil { + if err := validateTaskTitle(*req.Title); err != nil { + return nil, err + } + } task, err := s.tasks.GetTask(ctx, id) if err != nil { return nil, err diff --git a/apps/backend/internal/task/service/task_title.go b/apps/backend/internal/task/service/task_title.go new file mode 100644 index 0000000000..b183d1c2c7 --- /dev/null +++ b/apps/backend/internal/task/service/task_title.go @@ -0,0 +1,36 @@ +package service + +import ( + "errors" + "fmt" + "unicode/utf8" +) + +// TaskTitleMaxLength is the maximum number of characters allowed in a new or +// replacement task title. +const TaskTitleMaxLength = 60 + +// ErrTaskTitleTooLong identifies a task title that exceeds TaskTitleMaxLength. +var ErrTaskTitleTooLong = errors.New("task title is too long") + +// ValidateTaskTitle checks whether title fits the task title character limit. +func ValidateTaskTitle(title string) error { + if utf8.RuneCountInString(title) <= TaskTitleMaxLength { + return nil + } + return fmt.Errorf("%w: task titles must be %d characters or fewer", ErrTaskTitleTooLong, TaskTitleMaxLength) +} + +// TruncateTaskTitle shortens title to the task title limit, reserving the last +// character for an ellipsis when truncation is required. +func TruncateTaskTitle(title string) string { + characters := []rune(title) + if len(characters) <= TaskTitleMaxLength { + return title + } + return string(characters[:TaskTitleMaxLength-1]) + "…" +} + +func validateTaskTitle(title string) error { + return ValidateTaskTitle(title) +} diff --git a/apps/backend/internal/task/service/task_title_test.go b/apps/backend/internal/task/service/task_title_test.go new file mode 100644 index 0000000000..8b49a00c17 --- /dev/null +++ b/apps/backend/internal/task/service/task_title_test.go @@ -0,0 +1,143 @@ +package service + +import ( + "context" + "strings" + "testing" + + "github.com/kandev/kandev/internal/task/models" +) + +func TestCreateTaskRejectsTitleLongerThanLimit(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-title", Name: "Titles"}); err != nil { + t.Fatalf("CreateWorkspace: %v", err) + } + if err := repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-title", WorkspaceID: "ws-title", Name: "Workflow"}); err != nil { + t.Fatalf("CreateWorkflow: %v", err) + } + + _, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: "ws-title", + WorkflowID: "wf-title", + Title: strings.Repeat("x", 61), + }) + if err == nil { + t.Fatal("CreateTask accepted a title longer than 60 characters") + } + if !strings.Contains(err.Error(), "60") { + t.Fatalf("CreateTask error = %q, want 60-character validation", err) + } + + tasks, listErr := repo.ListTasks(ctx, "wf-title") + if listErr != nil { + t.Fatalf("ListTasks: %v", listErr) + } + if len(tasks) != 0 { + t.Fatalf("persisted %d tasks after title validation failure, want none", len(tasks)) + } +} + +func TestCreateTaskAcceptsTitleAtLimit(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-title", Name: "Titles"}); err != nil { + t.Fatalf("CreateWorkspace: %v", err) + } + if err := repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-title", WorkspaceID: "ws-title", Name: "Workflow"}); err != nil { + t.Fatalf("CreateWorkflow: %v", err) + } + + _, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: "ws-title", + WorkflowID: "wf-title", + Title: strings.Repeat("x", TaskTitleMaxLength), + }) + if err != nil { + t.Fatalf("CreateTask rejected a title at the limit: %v", err) + } +} + +func TestUpdateTaskRejectsTitleLongerThanLimit(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-title", Name: "Titles"}); err != nil { + t.Fatalf("CreateWorkspace: %v", err) + } + if err := repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-title", WorkspaceID: "ws-title", Name: "Workflow"}); err != nil { + t.Fatalf("CreateWorkflow: %v", err) + } + if err := repo.CreateTask(ctx, &models.Task{ + ID: "task-title", WorkspaceID: "ws-title", WorkflowID: "wf-title", Title: "Original", Priority: "medium", + }); err != nil { + t.Fatalf("CreateTask: %v", err) + } + + title := strings.Repeat("x", 61) + _, err := svc.UpdateTask(ctx, "task-title", &UpdateTaskRequest{Title: &title}) + if err == nil { + t.Fatal("UpdateTask accepted a title longer than 60 characters") + } + if !strings.Contains(err.Error(), "60") { + t.Fatalf("UpdateTask error = %q, want 60-character validation", err) + } + + stored, getErr := repo.GetTask(ctx, "task-title") + if getErr != nil { + t.Fatalf("GetTask: %v", getErr) + } + if stored.Title != "Original" { + t.Fatalf("stored title = %q, want unchanged original title", stored.Title) + } +} + +func TestUpdateTaskWithoutTitlePreservesLegacyOverlongTitle(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-title", Name: "Titles"}); err != nil { + t.Fatalf("CreateWorkspace: %v", err) + } + if err := repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-title", WorkspaceID: "ws-title", Name: "Workflow"}); err != nil { + t.Fatalf("CreateWorkflow: %v", err) + } + legacyTitle := strings.Repeat("legacy ", 12) + if err := repo.CreateTask(ctx, &models.Task{ + ID: "legacy-title", WorkspaceID: "ws-title", WorkflowID: "wf-title", Title: legacyTitle, Priority: "medium", + }); err != nil { + t.Fatalf("CreateTask: %v", err) + } + + description := "updated description" + if _, err := svc.UpdateTask(ctx, "legacy-title", &UpdateTaskRequest{Description: &description}); err != nil { + t.Fatalf("UpdateTask: %v", err) + } + + stored, err := repo.GetTask(ctx, "legacy-title") + if err != nil { + t.Fatalf("GetTask: %v", err) + } + if stored.Title != legacyTitle { + t.Fatalf("stored title = %q, want legacy title preserved", stored.Title) + } +} + +func TestTruncateTaskTitlePreservesCharacterLimit(t *testing.T) { + longTitle := strings.Repeat("😀", TaskTitleMaxLength+5) + + got := TruncateTaskTitle(longTitle) + if got != strings.Repeat("😀", TaskTitleMaxLength-1)+"…" { + t.Fatalf("TruncateTaskTitle = %q, want a 60-character ellipsis title", got) + } + if gotLen := len([]rune(got)); gotLen != TaskTitleMaxLength { + t.Fatalf("TruncateTaskTitle rune length = %d, want %d", gotLen, TaskTitleMaxLength) + } +} + +func TestTruncateTaskTitleLeavesShortTitlesUnchanged(t *testing.T) { + for _, title := range []string{"", "short", strings.Repeat("x", TaskTitleMaxLength)} { + if got := TruncateTaskTitle(title); got != title { + t.Errorf("TruncateTaskTitle(%q) = %q, want unchanged title", title, got) + } + } +} diff --git a/apps/web/app/office/components/new-task-dialog.tsx b/apps/web/app/office/components/new-task-dialog.tsx index a9497ce157..c6c886ba19 100644 --- a/apps/web/app/office/components/new-task-dialog.tsx +++ b/apps/web/app/office/components/new-task-dialog.tsx @@ -18,6 +18,7 @@ import { EMPTY_STAGES, type StagesDraft, } from "./new-task-stages"; +import { clampTaskTitleInput } from "@/lib/task-title"; function buildMetadata(draft: IssueDraft): Record | undefined { const meta: Record = {}; @@ -148,7 +149,7 @@ function NewIssueDialogBody({