diff --git a/apps/backend/internal/task/dto/dto.go b/apps/backend/internal/task/dto/dto.go index 46272c96ae..52adafefda 100644 --- a/apps/backend/internal/task/dto/dto.go +++ b/apps/backend/internal/task/dto/dto.go @@ -743,13 +743,16 @@ type ApproveSessionResponse struct { // TaskPlanDTO represents a task plan for API responses type TaskPlanDTO struct { - ID string `json:"id"` - TaskID string `json:"task_id"` - Title string `json:"title"` - Content string `json:"content"` - CreatedBy string `json:"created_by"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` + ID string `json:"id"` + TaskID string `json:"task_id"` + Title string `json:"title"` + Content string `json:"content"` + CreatedBy string `json:"created_by"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` + ImplementationStartedAt *time.Time `json:"implementation_started_at,omitempty"` + ImplementationStartedSessionID *string `json:"implementation_started_session_id,omitempty"` + ImplementationStartedBy *string `json:"implementation_started_by,omitempty"` } // TaskPlanFromModel converts a TaskPlan model to a TaskPlanDTO. @@ -758,13 +761,16 @@ func TaskPlanFromModel(plan *models.TaskPlan) *TaskPlanDTO { return nil } return &TaskPlanDTO{ - ID: plan.ID, - TaskID: plan.TaskID, - Title: plan.Title, - Content: plan.Content, - CreatedBy: plan.CreatedBy, - CreatedAt: plan.CreatedAt, - UpdatedAt: plan.UpdatedAt, + ID: plan.ID, + TaskID: plan.TaskID, + Title: plan.Title, + Content: plan.Content, + CreatedBy: plan.CreatedBy, + CreatedAt: plan.CreatedAt, + UpdatedAt: plan.UpdatedAt, + ImplementationStartedAt: plan.ImplementationStartedAt, + ImplementationStartedSessionID: plan.ImplementationStartedSessionID, + ImplementationStartedBy: plan.ImplementationStartedBy, } } diff --git a/apps/backend/internal/task/dto/task_plan_test.go b/apps/backend/internal/task/dto/task_plan_test.go new file mode 100644 index 0000000000..3fb9ca014a --- /dev/null +++ b/apps/backend/internal/task/dto/task_plan_test.go @@ -0,0 +1,37 @@ +package dto + +import ( + "testing" + "time" + + "github.com/kandev/kandev/internal/task/models" +) + +func TestTaskPlanFromModelIncludesImplementationMarker(t *testing.T) { + startedAt := time.Date(2026, 7, 9, 12, 0, 0, 0, time.UTC) + sessionID := "session-1" + actor := "user" + + out := TaskPlanFromModel(&models.TaskPlan{ + ID: "plan-1", + TaskID: "task-1", + Title: "Plan", + Content: "Implement it", + CreatedBy: "user", + CreatedAt: startedAt.Add(-time.Hour), + UpdatedAt: startedAt, + ImplementationStartedAt: &startedAt, + ImplementationStartedSessionID: &sessionID, + ImplementationStartedBy: &actor, + }) + + if out.ImplementationStartedAt == nil || !out.ImplementationStartedAt.Equal(startedAt) { + t.Fatalf("expected implementation_started_at %s, got %v", startedAt, out.ImplementationStartedAt) + } + if out.ImplementationStartedSessionID == nil || *out.ImplementationStartedSessionID != sessionID { + t.Fatalf("expected implementation_started_session_id %q, got %v", sessionID, out.ImplementationStartedSessionID) + } + if out.ImplementationStartedBy == nil || *out.ImplementationStartedBy != actor { + t.Fatalf("expected implementation_started_by %q, got %v", actor, out.ImplementationStartedBy) + } +} diff --git a/apps/backend/internal/task/handlers/task_handlers.go b/apps/backend/internal/task/handlers/task_handlers.go index 1a0fcdb8f4..3b08b01426 100644 --- a/apps/backend/internal/task/handlers/task_handlers.go +++ b/apps/backend/internal/task/handlers/task_handlers.go @@ -148,6 +148,7 @@ func (h *TaskHandlers) registerWS(dispatcher *ws.Dispatcher) { dispatcher.RegisterFunc(ws.ActionTaskPlanRevisionsList, h.wsListTaskPlanRevisions) dispatcher.RegisterFunc(ws.ActionTaskPlanRevisionGet, h.wsGetTaskPlanRevision) dispatcher.RegisterFunc(ws.ActionTaskPlanRevert, h.wsRevertTaskPlan) + dispatcher.RegisterFunc(ws.ActionTaskPlanImplement, h.wsMarkTaskPlanImplementationStarted) } // convertToServiceRepos converts dto.TaskRepositoryInput slice to service.TaskRepositoryInput slice. diff --git a/apps/backend/internal/task/handlers/task_plan_handlers.go b/apps/backend/internal/task/handlers/task_plan_handlers.go index 653c969439..164e9fc780 100644 --- a/apps/backend/internal/task/handlers/task_plan_handlers.go +++ b/apps/backend/internal/task/handlers/task_plan_handlers.go @@ -135,6 +135,28 @@ func (h *TaskHandlers) wsDeleteTaskPlan(ctx context.Context, msg *ws.Message) (* return ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{responseKeySuccess: true}) } +func (h *TaskHandlers) wsMarkTaskPlanImplementationStarted(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + var req struct { + TaskID string `json:"task_id"` + SessionID string `json:"session_id"` + Actor string `json:"actor"` + } + if err := json.Unmarshal(msg.Payload, &req); err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil) + } + + plan, err := h.planService.MarkImplementationStarted(ctx, service.MarkImplementationStartedRequest{ + TaskID: req.TaskID, + SessionID: req.SessionID, + Actor: req.Actor, + }) + if err != nil { + return taskPlanServiceError(msg, err, "Failed to mark task plan implementation started") + } + + return ws.NewResponse(msg.ID, msg.Action, dto.TaskPlanFromModel(plan)) +} + // wsListTaskPlanRevisions returns revision metadata newest-first (no content). func (h *TaskHandlers) wsListTaskPlanRevisions(ctx context.Context, msg *ws.Message) (*ws.Message, error) { var req struct { @@ -210,17 +232,27 @@ func (h *TaskHandlers) wsRevertTaskPlan(ctx context.Context, msg *ws.Message) (* AuthorName: req.AuthorName, }) if err != nil { - switch { - case errors.Is(err, service.ErrTaskIDRequired): - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "task_id is required", nil) - case errors.Is(err, service.ErrRevisionIDRequired): - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "revision_id is required", nil) - case errors.Is(err, service.ErrRevisionNotFound): - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeNotFound, "Revision not found", nil) - case errors.Is(err, service.ErrRevisionTaskMismatch): - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "Revision does not belong to task", nil) - } - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "Failed to revert plan: "+err.Error(), nil) + return taskPlanServiceError(msg, err, "Failed to revert plan") } return ws.NewResponse(msg.ID, msg.Action, dto.TaskPlanRevisionFromModel(rev)) } + +func taskPlanServiceError(msg *ws.Message, err error, fallback string) (*ws.Message, error) { + switch { + case errors.Is(err, service.ErrTaskIDRequired): + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "task_id is required", nil) + case errors.Is(err, service.ErrSessionIDRequired): + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "session_id is required", nil) + case errors.Is(err, service.ErrSessionTaskMismatch): + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "Session does not belong to task", nil) + case errors.Is(err, service.ErrTaskPlanNotFound): + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeNotFound, "Task plan not found", nil) + case errors.Is(err, service.ErrRevisionIDRequired): + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "revision_id is required", nil) + case errors.Is(err, service.ErrRevisionNotFound): + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeNotFound, "Revision not found", nil) + case errors.Is(err, service.ErrRevisionTaskMismatch): + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "Revision does not belong to task", nil) + } + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, fallback+": "+err.Error(), nil) +} diff --git a/apps/backend/internal/task/models/models.go b/apps/backend/internal/task/models/models.go index 253d02ae96..ffe7763b98 100644 --- a/apps/backend/internal/task/models/models.go +++ b/apps/backend/internal/task/models/models.go @@ -1131,13 +1131,16 @@ func (r *TaskEnvironmentRepo) ToAPI() map[string]interface{} { // TaskPlan represents a plan associated with a task type TaskPlan struct { - ID string `json:"id"` - TaskID string `json:"task_id"` - Title string `json:"title"` - Content string `json:"content"` - CreatedBy string `json:"created_by"` // "agent" or "user" - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` + ID string `json:"id"` + TaskID string `json:"task_id"` + Title string `json:"title"` + Content string `json:"content"` + CreatedBy string `json:"created_by"` // "agent" or "user" + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` + ImplementationStartedAt *time.Time `json:"implementation_started_at,omitempty"` + ImplementationStartedSessionID *string `json:"implementation_started_session_id,omitempty"` + ImplementationStartedBy *string `json:"implementation_started_by,omitempty"` } // TaskPlanRevision is one immutable snapshot in the revision history of a task plan. diff --git a/apps/backend/internal/task/repository/interface.go b/apps/backend/internal/task/repository/interface.go index 0ecb3de935..d300232498 100644 --- a/apps/backend/internal/task/repository/interface.go +++ b/apps/backend/internal/task/repository/interface.go @@ -13,6 +13,7 @@ import ( var ErrWorkspaceNameMismatch = repoerrors.ErrWorkspaceNameMismatch var ErrWorkspaceNotFound = repoerrors.ErrWorkspaceNotFound var ErrTaskNotFound = repoerrors.ErrTaskNotFound +var ErrTaskPlanNotFound = repoerrors.ErrTaskPlanNotFound // WorkspaceRepository handles workspace CRUD. type WorkspaceRepository interface { @@ -318,6 +319,7 @@ type PlanRepository interface { CreateTaskPlan(ctx context.Context, plan *models.TaskPlan) error GetTaskPlan(ctx context.Context, taskID string) (*models.TaskPlan, error) UpdateTaskPlan(ctx context.Context, plan *models.TaskPlan) error + MarkTaskPlanImplementationStarted(ctx context.Context, taskID, sessionID, actor string) (*models.TaskPlan, error) DeleteTaskPlan(ctx context.Context, taskID string) error // Revision history diff --git a/apps/backend/internal/task/repository/repoerrors/errors.go b/apps/backend/internal/task/repository/repoerrors/errors.go index 29a7144328..b958d2c63b 100644 --- a/apps/backend/internal/task/repository/repoerrors/errors.go +++ b/apps/backend/internal/task/repository/repoerrors/errors.go @@ -11,3 +11,6 @@ var ErrWorkspaceNotFound = errors.New("workspace not found") // ErrTaskNotFound reports that no task row matched the supplied id. var ErrTaskNotFound = errors.New("task not found") + +// ErrTaskPlanNotFound reports that no task plan row matched the supplied task id. +var ErrTaskPlanNotFound = errors.New("task plan not found") diff --git a/apps/backend/internal/task/repository/sqlite/base_migrations.go b/apps/backend/internal/task/repository/sqlite/base_migrations.go index ab037e40d3..95494837f1 100644 --- a/apps/backend/internal/task/repository/sqlite/base_migrations.go +++ b/apps/backend/internal/task/repository/sqlite/base_migrations.go @@ -98,6 +98,9 @@ func (r *Repository) runMigrations() error { r.migrate.Apply("repositories.copy_files", `ALTER TABLE repositories ADD COLUMN copy_files TEXT DEFAULT ''`) r.migrate.Apply("repositories.worktree_branch_template", `ALTER TABLE repositories ADD COLUMN worktree_branch_template TEXT DEFAULT 'feature/{title}-{suffix}'`) r.migrate.Apply("repositories.worktree_branch_template.backfill", `UPDATE repositories SET worktree_branch_template = COALESCE(NULLIF(TRIM(worktree_branch_prefix), ''), 'feature/') || '{title}-{suffix}'`) + r.migrate.Apply("task_plans.implementation_started_at", `ALTER TABLE task_plans ADD COLUMN implementation_started_at TIMESTAMP`) + r.migrate.Apply("task_plans.implementation_started_session_id", `ALTER TABLE task_plans ADD COLUMN implementation_started_session_id TEXT`) + r.migrate.Apply("task_plans.implementation_started_by", `ALTER TABLE task_plans ADD COLUMN implementation_started_by TEXT`) // Authoritative per-message change signal (chat render-perf). SQLite forbids a // non-constant default on ADD COLUMN, so the column is added nullable and diff --git a/apps/backend/internal/task/repository/sqlite/base_schema.go b/apps/backend/internal/task/repository/sqlite/base_schema.go index 16665d8b69..3302bbe2b5 100644 --- a/apps/backend/internal/task/repository/sqlite/base_schema.go +++ b/apps/backend/internal/task/repository/sqlite/base_schema.go @@ -327,6 +327,9 @@ func (r *Repository) initPlansSchema() error { created_by TEXT NOT NULL DEFAULT 'agent', created_at TIMESTAMP NOT NULL, updated_at TIMESTAMP NOT NULL, + implementation_started_at TIMESTAMP, + implementation_started_session_id TEXT, + implementation_started_by TEXT, FOREIGN KEY (task_id) REFERENCES tasks(id) ON DELETE CASCADE ); CREATE INDEX IF NOT EXISTS idx_task_plans_task_id ON task_plans(task_id); diff --git a/apps/backend/internal/task/repository/sqlite/errors.go b/apps/backend/internal/task/repository/sqlite/errors.go index 5db0f1a040..7a6424df1f 100644 --- a/apps/backend/internal/task/repository/sqlite/errors.go +++ b/apps/backend/internal/task/repository/sqlite/errors.go @@ -16,6 +16,10 @@ var ErrTaskNotFound = repoerrors.ErrTaskNotFound // matches the supplied id. var ErrWorkspaceNotFound = repoerrors.ErrWorkspaceNotFound +// ErrTaskPlanNotFound is returned by Repository task-plan methods when no row +// matches the supplied task id. +var ErrTaskPlanNotFound = repoerrors.ErrTaskPlanNotFound + // ErrNoPrimarySession is returned by GetPrimarySessionByTaskID when the task // has no primary session row. Callers should use errors.Is to distinguish this // "not found" case from genuine backend/DB errors. diff --git a/apps/backend/internal/task/repository/sqlite/plan.go b/apps/backend/internal/task/repository/sqlite/plan.go index 66b1f466f4..05a75fd13c 100644 --- a/apps/backend/internal/task/repository/sqlite/plan.go +++ b/apps/backend/internal/task/repository/sqlite/plan.go @@ -19,6 +19,8 @@ const revisionSelectCols = `id, task_id, revision_number, title, content, author // and is the fallback for unknown values when persisting plan history rows. const authorKindAgent = "agent" +const planSelectCols = `id, task_id, title, content, created_by, created_at, updated_at, implementation_started_at, implementation_started_session_id, implementation_started_by` + // CreateTaskPlan creates a new task plan. func (r *Repository) CreateTaskPlan(ctx context.Context, plan *models.TaskPlan) error { if plan.ID == "" { @@ -44,15 +46,10 @@ func (r *Repository) CreateTaskPlan(ctx context.Context, plan *models.TaskPlan) // GetTaskPlan retrieves a task plan by task ID. func (r *Repository) GetTaskPlan(ctx context.Context, taskID string) (*models.TaskPlan, error) { - plan := &models.TaskPlan{} - err := r.ro.QueryRowContext(ctx, r.ro.Rebind(` - SELECT id, task_id, title, content, created_by, created_at, updated_at + plan, err := scanPlanRow(r.ro.QueryRowContext(ctx, r.ro.Rebind(` + SELECT `+planSelectCols+` FROM task_plans WHERE task_id = ? - `), taskID).Scan(&plan.ID, &plan.TaskID, &plan.Title, &plan.Content, &plan.CreatedBy, &plan.CreatedAt, &plan.UpdatedAt) - - if err == sql.ErrNoRows { - return nil, nil // Return nil, nil when no plan exists - } + `), taskID)) if err != nil { return nil, fmt.Errorf("failed to get task plan: %w", err) } @@ -78,6 +75,38 @@ func (r *Repository) UpdateTaskPlan(ctx context.Context, plan *models.TaskPlan) return nil } +// MarkTaskPlanImplementationStarted records the first accepted implementation start. +// It is idempotent: later calls return the existing marker without changing it. +func (r *Repository) MarkTaskPlanImplementationStarted(ctx context.Context, taskID, sessionID, actor string) (*models.TaskPlan, error) { + now := time.Now().UTC() + result, err := r.db.ExecContext(ctx, r.db.Rebind(` + UPDATE task_plans + SET + implementation_started_at = COALESCE(implementation_started_at, ?), + implementation_started_session_id = CASE + WHEN implementation_started_at IS NULL THEN ? + ELSE implementation_started_session_id + END, + implementation_started_by = CASE + WHEN implementation_started_at IS NULL THEN ? + ELSE implementation_started_by + END, + updated_at = CASE + WHEN implementation_started_at IS NULL THEN ? + ELSE updated_at + END + WHERE task_id = ? + `), now, sessionID, actor, now, taskID) + if err != nil { + return nil, fmt.Errorf("failed to mark task plan implementation started: %w", err) + } + rows, _ := result.RowsAffected() + if rows == 0 { + return nil, fmt.Errorf("%w: %s", ErrTaskPlanNotFound, taskID) + } + return r.GetTaskPlan(ctx, taskID) +} + // DeleteTaskPlan deletes a task plan by task ID. func (r *Repository) DeleteTaskPlan(ctx context.Context, taskID string) error { result, err := r.db.ExecContext(ctx, r.db.Rebind(`DELETE FROM task_plans WHERE task_id = ?`), taskID) @@ -246,6 +275,41 @@ func (r *Repository) WritePlanRevision( return tx.Commit() } +func scanPlanRow(row *sql.Row) (*models.TaskPlan, error) { + plan := &models.TaskPlan{} + var startedAt sql.NullTime + var sessionID sql.NullString + var actor sql.NullString + err := row.Scan( + &plan.ID, + &plan.TaskID, + &plan.Title, + &plan.Content, + &plan.CreatedBy, + &plan.CreatedAt, + &plan.UpdatedAt, + &startedAt, + &sessionID, + &actor, + ) + if err == sql.ErrNoRows { + return nil, nil + } + if err != nil { + return nil, err + } + if startedAt.Valid { + plan.ImplementationStartedAt = &startedAt.Time + } + if sessionID.Valid { + plan.ImplementationStartedSessionID = &sessionID.String + } + if actor.Valid { + plan.ImplementationStartedBy = &actor.String + } + return plan, nil +} + func upsertPlanHead(ctx context.Context, tx *sqlx.Tx, db *sqlx.DB, head *models.TaskPlan, now time.Time) error { if head.ID == "" { head.ID = uuid.New().String() diff --git a/apps/backend/internal/task/service/plan_service.go b/apps/backend/internal/task/service/plan_service.go index 6ed4933ede..c692020ed0 100644 --- a/apps/backend/internal/task/service/plan_service.go +++ b/apps/backend/internal/task/service/plan_service.go @@ -20,9 +20,11 @@ var ( ErrTaskPlanNotFound = errors.New("task plan not found") ErrTaskIDRequired = errors.New("task_id is required") ErrContentRequired = errors.New("content is required") + ErrSessionIDRequired = errors.New("session_id is required") ErrRevisionNotFound = errors.New("task plan revision not found") ErrRevisionIDRequired = errors.New("target_revision_id is required") ErrRevisionTaskMismatch = errors.New("revision does not belong to given task") + ErrSessionTaskMismatch = errors.New("session does not belong to given task") ) const ( @@ -42,6 +44,7 @@ type planRepo interface { repository.PlanRepository GetActiveTaskSessionByTaskID(ctx context.Context, taskID string) (*models.TaskSession, error) GetTaskSessionByTaskID(ctx context.Context, taskID string) (*models.TaskSession, error) + GetTaskSession(ctx context.Context, id string) (*models.TaskSession, error) } // PlanService provides task plan business logic. @@ -211,9 +214,16 @@ func (s *PlanService) upsertPlan(ctx context.Context, req CreatePlanRequest) (*m return nil, err } - s.publishPlanEvent(ctx, eventType, plan) + saved, err := s.repo.GetTaskPlan(ctx, req.TaskID) + if err != nil { + return nil, err + } + if saved == nil { + return nil, ErrTaskPlanNotFound + } + s.publishPlanEvent(ctx, eventType, saved) s.publishRevisionEvent(ctx, rev, coalesce) - return plan, nil + return saved, nil } // resolveAgentDisplayName returns the agent profile's display name for the @@ -302,6 +312,47 @@ func (s *PlanService) GetPlan(ctx context.Context, taskID string) (*models.TaskP return s.repo.GetTaskPlan(ctx, taskID) } +type MarkImplementationStartedRequest struct { + TaskID string + SessionID string + Actor string +} + +func (s *PlanService) MarkImplementationStarted(ctx context.Context, req MarkImplementationStartedRequest) (*models.TaskPlan, error) { + if req.TaskID == "" { + return nil, ErrTaskIDRequired + } + if req.SessionID == "" { + return nil, ErrSessionIDRequired + } + session, err := s.repo.GetTaskSession(ctx, req.SessionID) + if err != nil { + if errors.Is(err, models.ErrTaskSessionNotFound) { + return nil, ErrSessionTaskMismatch + } + return nil, err + } + if session == nil || session.TaskID != req.TaskID { + return nil, ErrSessionTaskMismatch + } + actor := req.Actor + if actor == "" { + actor = createdByUser + } + plan, err := s.repo.MarkTaskPlanImplementationStarted(ctx, req.TaskID, req.SessionID, actor) + if err != nil { + if errors.Is(err, repository.ErrTaskPlanNotFound) { + return nil, ErrTaskPlanNotFound + } + return nil, err + } + if plan == nil { + return nil, ErrTaskPlanNotFound + } + s.publishPlanEvent(ctx, events.TaskPlanUpdated, plan) + return plan, nil +} + // DeletePlan removes a plan and all its revisions (cascade via FK when task goes; here we delete only HEAD). // Historical revisions remain for audit; callers wanting a full wipe should delete the task. func (s *PlanService) DeletePlan(ctx context.Context, taskID string) error { @@ -403,7 +454,14 @@ func (s *PlanService) RevertPlan(ctx context.Context, req RevertPlanRequest) (*m return nil, err } - s.publishPlanEvent(ctx, events.TaskPlanUpdated, plan) + saved, err := s.repo.GetTaskPlan(ctx, req.TaskID) + if err != nil { + return nil, err + } + if saved == nil { + return nil, ErrTaskPlanNotFound + } + s.publishPlanEvent(ctx, events.TaskPlanUpdated, saved) s.publishRevisionEvent(ctx, rev, false) s.publishReverted(ctx, rev) return rev, nil @@ -422,6 +480,15 @@ func (s *PlanService) publishPlanEvent(ctx context.Context, eventType string, pl "created_at": plan.CreatedAt, "updated_at": plan.UpdatedAt, } + if plan.ImplementationStartedAt != nil { + payload["implementation_started_at"] = *plan.ImplementationStartedAt + } + if plan.ImplementationStartedSessionID != nil { + payload["implementation_started_session_id"] = *plan.ImplementationStartedSessionID + } + if plan.ImplementationStartedBy != nil { + payload["implementation_started_by"] = *plan.ImplementationStartedBy + } if err := s.eventBus.Publish(ctx, eventType, bus.NewEvent(eventType, "plan-service", payload)); err != nil { s.logger.Error("publish plan event", zap.String("event_type", eventType), zap.Error(err)) } diff --git a/apps/backend/internal/task/service/plan_service_test.go b/apps/backend/internal/task/service/plan_service_test.go index 1686f91603..3eb2219f9e 100644 --- a/apps/backend/internal/task/service/plan_service_test.go +++ b/apps/backend/internal/task/service/plan_service_test.go @@ -36,6 +36,13 @@ func seedTask(t *testing.T, ctx context.Context, repo *sqliterepo.Repository, ta }) } +func seedSession(t *testing.T, ctx context.Context, repo *sqliterepo.Repository, taskID, sessionID string) { + t.Helper() + if err := repo.CreateTaskSession(ctx, &models.TaskSession{ID: sessionID, TaskID: taskID}); err != nil { + t.Fatalf("CreateTaskSession(%s): %v", sessionID, err) + } +} + func createTestPlanService(t *testing.T) (*PlanService, *MockEventBus, *sqliterepo.Repository) { t.Helper() _, eventBus, repo := createTestService(t) @@ -44,6 +51,14 @@ func createTestPlanService(t *testing.T) (*PlanService, *MockEventBus, *sqlitere return svc, eventBus, repo } +type nilMarkPlanRepo struct { + *sqliterepo.Repository +} + +func (r *nilMarkPlanRepo) MarkTaskPlanImplementationStarted(ctx context.Context, taskID, sessionID, actor string) (*models.TaskPlan, error) { + return nil, nil +} + func TestPlanService_CreatePlan(t *testing.T) { svc, _, repo := createTestPlanService(t) ctx := context.Background() @@ -162,6 +177,154 @@ func TestPlanService_UpdatePlan(t *testing.T) { } } +func TestPlanService_MarkImplementationStartedIsDurableAndIdempotent(t *testing.T) { + svc, _, repo := createTestPlanService(t) + ctx := context.Background() + seedTask(t, ctx, repo, "task-impl") + seedSession(t, ctx, repo, "task-impl", "session-1") + seedSession(t, ctx, repo, "task-impl", "session-2") + + _, err := svc.CreatePlan(ctx, CreatePlanRequest{ + TaskID: "task-impl", + Title: "Plan", + Content: "Ship the toolbar", + CreatedBy: "user", + }) + if err != nil { + t.Fatalf("CreatePlan failed: %v", err) + } + + marked, err := svc.MarkImplementationStarted(ctx, MarkImplementationStartedRequest{ + TaskID: "task-impl", + SessionID: "session-1", + Actor: "user", + }) + if err != nil { + t.Fatalf("MarkImplementationStarted failed: %v", err) + } + if marked.ImplementationStartedAt == nil { + t.Fatal("expected implementation_started_at to be set") + } + if marked.ImplementationStartedSessionID == nil || *marked.ImplementationStartedSessionID != "session-1" { + t.Fatalf("expected session marker session-1, got %v", marked.ImplementationStartedSessionID) + } + if marked.ImplementationStartedBy == nil || *marked.ImplementationStartedBy != "user" { + t.Fatalf("expected actor marker user, got %v", marked.ImplementationStartedBy) + } + + firstStartedAt := *marked.ImplementationStartedAt + idempotent, err := svc.MarkImplementationStarted(ctx, MarkImplementationStartedRequest{ + TaskID: "task-impl", + SessionID: "session-2", + Actor: "agent", + }) + if err != nil { + t.Fatalf("second MarkImplementationStarted failed: %v", err) + } + if !idempotent.ImplementationStartedAt.Equal(firstStartedAt) { + t.Fatalf("expected started_at to remain %s, got %s", firstStartedAt, *idempotent.ImplementationStartedAt) + } + if idempotent.ImplementationStartedSessionID == nil || *idempotent.ImplementationStartedSessionID != "session-1" { + t.Fatalf("expected idempotent session marker session-1, got %v", idempotent.ImplementationStartedSessionID) + } + if idempotent.ImplementationStartedBy == nil || *idempotent.ImplementationStartedBy != "user" { + t.Fatalf("expected idempotent actor marker user, got %v", idempotent.ImplementationStartedBy) + } + + updated, err := svc.UpdatePlan(ctx, UpdatePlanRequest{ + TaskID: "task-impl", + Content: "Ship the toolbar after review", + CreatedBy: "user", + }) + if err != nil { + t.Fatalf("UpdatePlan failed: %v", err) + } + if updated.ImplementationStartedAt == nil || !updated.ImplementationStartedAt.Equal(firstStartedAt) { + t.Fatalf("expected update to preserve implementation marker, got %v", updated.ImplementationStartedAt) + } +} + +func TestPlanService_MarkImplementationStartedRejectsCrossTaskSession(t *testing.T) { + svc, _, repo := createTestPlanService(t) + ctx := context.Background() + seedTask(t, ctx, repo, "task-impl") + seedTask(t, ctx, repo, "task-other") + seedSession(t, ctx, repo, "task-other", "session-other") + + _, err := svc.CreatePlan(ctx, CreatePlanRequest{ + TaskID: "task-impl", + Title: "Plan", + Content: "Ship the toolbar", + }) + if err != nil { + t.Fatalf("CreatePlan failed: %v", err) + } + + _, err = svc.MarkImplementationStarted(ctx, MarkImplementationStartedRequest{ + TaskID: "task-impl", + SessionID: "session-other", + Actor: "user", + }) + if err != ErrSessionTaskMismatch { + t.Fatalf("expected ErrSessionTaskMismatch, got %v", err) + } +} + +func TestPlanService_MarkImplementationStartedRejectsMissingSession(t *testing.T) { + svc, _, repo := createTestPlanService(t) + ctx := context.Background() + seedTask(t, ctx, repo, "task-impl") + + _, err := svc.CreatePlan(ctx, CreatePlanRequest{ + TaskID: "task-impl", + Title: "Plan", + Content: "Ship the toolbar", + }) + if err != nil { + t.Fatalf("CreatePlan failed: %v", err) + } + + _, err = svc.MarkImplementationStarted(ctx, MarkImplementationStartedRequest{ + TaskID: "task-impl", + SessionID: "missing-session", + Actor: "user", + }) + if err != ErrSessionTaskMismatch { + t.Fatalf("expected ErrSessionTaskMismatch, got %v", err) + } +} + +func TestPlanService_MarkImplementationStartedHandlesNilPlanAfterWrite(t *testing.T) { + _, eventBus, repo := createTestService(t) + log, _ := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json", OutputPath: "stdout"}) + svc := NewPlanService(&nilMarkPlanRepo{Repository: repo}, eventBus, log) + ctx := context.Background() + seedTask(t, ctx, repo, "task-impl") + seedSession(t, ctx, repo, "task-impl", "session-1") + + _, err := svc.CreatePlan(ctx, CreatePlanRequest{ + TaskID: "task-impl", + Title: "Plan", + Content: "Ship the toolbar", + }) + if err != nil { + t.Fatalf("CreatePlan failed: %v", err) + } + eventBus.ClearEvents() + + _, err = svc.MarkImplementationStarted(ctx, MarkImplementationStartedRequest{ + TaskID: "task-impl", + SessionID: "session-1", + Actor: "user", + }) + if err != ErrTaskPlanNotFound { + t.Fatalf("expected ErrTaskPlanNotFound, got %v", err) + } + if got := len(eventBus.GetPublishedEvents()); got != 0 { + t.Fatalf("expected no events after nil plan marker, got %d", got) + } +} + func TestPlanService_UpdatePlanNotFound(t *testing.T) { svc, _, repo := createTestPlanService(t) ctx := context.Background() diff --git a/apps/backend/pkg/websocket/actions.go b/apps/backend/pkg/websocket/actions.go index 5bfcf4154a..8b26c204d0 100644 --- a/apps/backend/pkg/websocket/actions.go +++ b/apps/backend/pkg/websocket/actions.go @@ -73,6 +73,7 @@ const ( ActionTaskPlanRevisionsList = "task.plan.revisions.list" ActionTaskPlanRevisionGet = "task.plan.revision.get" ActionTaskPlanRevert = "task.plan.revert" + ActionTaskPlanImplement = "task.plan.implementation_started" ActionTaskSessionList = "task.session.list" ActionTaskSessionStatus = "task.session.status" diff --git a/apps/web/components/task/chat/implement-plan-button.tsx b/apps/web/components/task/chat/implement-plan-button.tsx index 1ac7c6ed12..f97dc67b96 100644 --- a/apps/web/components/task/chat/implement-plan-button.tsx +++ b/apps/web/components/task/chat/implement-plan-button.tsx @@ -1,5 +1,6 @@ "use client"; +import type { ReactNode } from "react"; import { IconChevronDown, IconPlus, IconRocket } from "@tabler/icons-react"; import { Button } from "@kandev/ui/button"; import { @@ -9,6 +10,7 @@ import { DropdownMenuTrigger, } from "@kandev/ui/dropdown-menu"; import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; +import { cn } from "@kandev/ui/lib/utils"; /** * Split-button: primary "Implement" runs the plan inline in the current @@ -16,27 +18,93 @@ import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; * a new session with a clean context window. `onClick(fresh)` is invoked * with `false` for the primary path and `true` for the fresh-agent path. */ -export function ImplementPlanButton({ onClick }: { onClick: (fresh: boolean) => void }) { - const primary = ( +type ImplementPlanButtonProps = { + onClick: (fresh: boolean) => void | Promise; + disabled?: boolean; + disabledReason?: string; + framed?: boolean; + testIds?: { + root?: string; + button?: string; + menuTrigger?: string; + freshItem?: string; + }; +}; + +function legacyRootTestId(rootId: string) { + return rootId === "implement-plan-control" ? undefined : "implement-plan-control"; +} + +function DisabledImplementTooltip({ + disabledReason, + children, +}: { + disabledReason: string; + children: ReactNode; +}) { + return ( - + + {children} + + {disabledReason} + + ); +} + +export function ImplementPlanButton({ + onClick, + disabled, + disabledReason, + framed, + testIds, +}: ImplementPlanButtonProps) { + const ids = { + root: testIds?.root ?? "implement-plan-control", + button: testIds?.button ?? "implement-plan-button", + menuTrigger: testIds?.menuTrigger ?? "implement-plan-menu-trigger", + freshItem: testIds?.freshItem ?? "implement-fresh-menu-item", + }; + const primaryButton = ( + + + + ); + const primary = disabledReason ? ( + primaryButton + ) : ( + + {primaryButton} Implement the plan in this session ); - return ( -
+ const splitButton = ( +
{primary} @@ -44,16 +112,22 @@ export function ImplementPlanButton({ onClick }: { onClick: (fresh: boolean) => type="button" variant="ghost" size="sm" - data-testid="implement-plan-menu-trigger" + data-testid={ids.menuTrigger} aria-label="More implement options" - className="h-7 px-1 cursor-pointer hover:bg-muted/40 text-violet-400 rounded-l-none border-l border-violet-400/20" + aria-disabled={disabled} + disabled={disabled} + className={cn( + "h-7 px-1 text-violet-400 rounded-l-none border-y-0 border-r-0 border-l border-violet-400/20", + "hover:bg-muted/40 focus-visible:border-y-0 focus-visible:border-r-0 focus-visible:border-l-violet-400/20 focus-visible:ring-0", + disabled ? "pointer-events-none cursor-not-allowed" : "cursor-pointer", + )} > onClick(true)} className="cursor-pointer" > @@ -69,4 +143,12 @@ export function ImplementPlanButton({ onClick }: { onClick: (fresh: boolean) =>
); + + if (!disabledReason) return splitButton; + + return ( + + {splitButton} + + ); } diff --git a/apps/web/components/task/task-plan-implement.test.ts b/apps/web/components/task/task-plan-implement.test.ts new file mode 100644 index 0000000000..1ddee08a4a --- /dev/null +++ b/apps/web/components/task/task-plan-implement.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from "vitest"; +import type { TaskPlan } from "@/lib/types/http"; +import { getPlanToolbarImplementState } from "./task-plan-implement"; + +function plan(overrides: Partial = {}): TaskPlan { + return { + id: "plan-1", + task_id: "task-1", + title: "Plan", + content: "Persisted plan", + created_by: "user", + created_at: "2026-07-09T12:00:00Z", + updated_at: "2026-07-09T12:00:00Z", + ...overrides, + }; +} + +describe("getPlanToolbarImplementState", () => { + it("hides the button when the editor draft is empty", () => { + expect(getPlanToolbarImplementState({ draftContent: " \n", plan: null })).toEqual({ + visible: false, + disabled: false, + }); + }); + + it("enables the button for non-empty unimplemented plan content", () => { + expect(getPlanToolbarImplementState({ draftContent: "Ship it", plan: plan() })).toEqual({ + visible: true, + disabled: false, + }); + }); + + it("keeps the button visible and disabled once implementation has started", () => { + expect( + getPlanToolbarImplementState({ + draftContent: "Ship it", + plan: plan({ implementation_started_at: "2026-07-09T12:30:00Z" }), + }), + ).toEqual({ + visible: true, + disabled: true, + disabledReason: "This plan has already been sent for implementation.", + }); + }); + + it("keeps the disabled marker affordance visible even when the draft is empty", () => { + expect( + getPlanToolbarImplementState({ + draftContent: " \n", + plan: plan({ implementation_started_at: "2026-07-09T12:30:00Z" }), + }), + ).toEqual({ + visible: true, + disabled: true, + disabledReason: "This plan has already been sent for implementation.", + }); + }); +}); diff --git a/apps/web/components/task/task-plan-implement.ts b/apps/web/components/task/task-plan-implement.ts new file mode 100644 index 0000000000..e07831fe00 --- /dev/null +++ b/apps/web/components/task/task-plan-implement.ts @@ -0,0 +1,29 @@ +import type { TaskPlan } from "@/lib/types/http"; + +type PlanToolbarImplementArgs = { + draftContent: string; + plan: TaskPlan | null; +}; + +export type PlanToolbarImplementState = { + visible: boolean; + disabled: boolean; + disabledReason?: string; +}; + +export function getPlanToolbarImplementState({ + draftContent, + plan, +}: PlanToolbarImplementArgs): PlanToolbarImplementState { + if (plan?.implementation_started_at) { + return { + visible: true, + disabled: true, + disabledReason: "This plan has already been sent for implementation.", + }; + } + if (draftContent.trim() === "") { + return { visible: false, disabled: false }; + } + return { visible: true, disabled: false }; +} diff --git a/apps/web/components/task/task-plan-panel-header.tsx b/apps/web/components/task/task-plan-panel-header.tsx new file mode 100644 index 0000000000..710ffb110e --- /dev/null +++ b/apps/web/components/task/task-plan-panel-header.tsx @@ -0,0 +1,128 @@ +"use client"; + +import { useCallback, useState } from "react"; +import { PanelHeaderBarSplit } from "./panel-primitives"; +import { TaskPlanRevisions } from "./task-plan-revisions"; +import { getPlanToolbarImplementState } from "./task-plan-implement"; +import { ImplementPlanButton } from "./chat/implement-plan-button"; +import { useImplementPlanRunner } from "@/hooks/domains/kanban/use-plan-actions"; +import type { TaskPlan, TaskPlanRevision } from "@/lib/types/http"; + +type PlanPanelHeaderProps = { + taskId: string; + plan: TaskPlan | null; + draftContent: string; + hasUnsavedChanges: boolean; + activeSessionId: string | null; + revisions: TaskPlanRevision[]; + isLoadingRevisions: boolean; + isSaving: boolean; + isAgentBusy: boolean; + savePlan: (content: string, title?: string) => Promise; + onOpenRevisions: () => void; + onRevert: (id: string) => Promise; + loadRevisionContent: (revisionId: string) => Promise; + previewRevisionId: string | null; + setPreviewRevision: (revisionId: string | null) => void; + comparePair: [string | null, string | null]; + toggleCompareSelection: (revisionId: string) => void; + clearComparePair: () => void; +}; + +// Header bar lives only to host plan actions. The plan title is already shown +// by the dockview/mobile tab above the panel, so we don't repeat it here. +export function PlanPanelHeader({ + taskId, + plan, + draftContent, + hasUnsavedChanges, + activeSessionId, + revisions, + isLoadingRevisions, + isSaving, + isAgentBusy, + savePlan, + onOpenRevisions, + onRevert, + loadRevisionContent, + previewRevisionId, + setPreviewRevision, + comparePair, + toggleCompareSelection, + clearComparePair, +}: PlanPanelHeaderProps) { + const [isImplementing, setIsImplementing] = useState(false); + const implementPlan = useImplementPlanRunner({ + resolvedSessionId: activeSessionId, + taskId, + clearPlanModeAfterSend: false, + }); + const implementState = getPlanToolbarImplementState({ draftContent, plan }); + const implementDisabled = + implementState.disabled || isSaving || isImplementing || isAgentBusy || !activeSessionId; + const handleImplement = useCallback( + async (fresh: boolean) => { + if (implementDisabled) return; + setIsImplementing(true); + try { + let savedPlan: TaskPlan; + if (hasUnsavedChanges || !plan) { + const nextPlan = await savePlan(draftContent, plan?.title); + if (!nextPlan) return; + savedPlan = nextPlan; + } else { + savedPlan = plan; + } + const savedState = getPlanToolbarImplementState({ + draftContent: savedPlan.content, + plan: savedPlan, + }); + if (!savedState.visible || savedState.disabled) { + return; + } + await implementPlan(fresh); + } finally { + setIsImplementing(false); + } + }, + [draftContent, hasUnsavedChanges, implementDisabled, implementPlan, plan, savePlan], + ); + + return ( + + {implementState.visible && ( + + )} + + + } + /> + ); +} diff --git a/apps/web/components/task/task-plan-panel.tsx b/apps/web/components/task/task-plan-panel.tsx index 8ddf7b67ce..f3a3b7b870 100644 --- a/apps/web/components/task/task-plan-panel.tsx +++ b/apps/web/components/task/task-plan-panel.tsx @@ -1,8 +1,8 @@ "use client"; import { memo, useState, useCallback, useEffect, useRef, useMemo } from "react"; -import { PanelRoot, PanelBody, PanelHeaderBarSplit } from "./panel-primitives"; -import { TaskPlanRevisions } from "./task-plan-revisions"; +import { PanelRoot, PanelBody } from "./panel-primitives"; +import { PlanPanelHeader } from "./task-plan-panel-header"; import dynamic from "@/lib/routing/client-dynamic"; import { IconLoader2, IconFileText, IconRobot, IconMessage, IconClick } from "@tabler/icons-react"; import { cn } from "@/lib/utils"; @@ -16,7 +16,6 @@ import type { TextSelection, CommentForEditor, } from "@/components/editors/tiptap/tiptap-plan-editor"; -import type { TaskPlanRevision } from "@/lib/types/http"; import type { Editor } from "@tiptap/core"; import { PanelSearchBar } from "@/components/search/panel-search-bar"; import { usePlanFindShortcut } from "./use-plan-find-shortcut"; @@ -192,9 +191,15 @@ function PlanPanelContent({ ); } - -type PlanPanelHeaderProps = { - taskId: string; - revisions: TaskPlanRevision[]; - isLoadingRevisions: boolean; - isSaving: boolean; - onOpenRevisions: () => void; - onRevert: (id: string) => Promise; - loadRevisionContent: (revisionId: string) => Promise; - previewRevisionId: string | null; - setPreviewRevision: (revisionId: string | null) => void; - comparePair: [string | null, string | null]; - toggleCompareSelection: (revisionId: string) => void; - clearComparePair: () => void; -}; - -// Header bar lives only to host the rewind button. The plan's title (default -// "Plan") is already shown by the dockview/mobile tab above the panel, so we -// don't repeat it here. -function PlanPanelHeader({ - taskId, - revisions, - isLoadingRevisions, - isSaving, - onOpenRevisions, - onRevert, - loadRevisionContent, - previewRevisionId, - setPreviewRevision, - comparePair, - toggleCompareSelection, - clearComparePair, -}: PlanPanelHeaderProps) { - return ( - - } - /> - ); -} diff --git a/apps/web/e2e/helpers/api-client.ts b/apps/web/e2e/helpers/api-client.ts index 496b81c3d6..c16e980a3b 100644 --- a/apps/web/e2e/helpers/api-client.ts +++ b/apps/web/e2e/helpers/api-client.ts @@ -1232,6 +1232,9 @@ export class ApiClient { title?: string; created_by: string; updated_at: string; + implementation_started_at?: string | null; + implementation_started_session_id?: string | null; + implementation_started_by?: string | null; } | null> { try { return await this.wsRequest("task.plan.get", { task_id: taskId }); diff --git a/apps/web/e2e/tests/task/mobile-plan-toolbar-implement.spec.ts b/apps/web/e2e/tests/task/mobile-plan-toolbar-implement.spec.ts new file mode 100644 index 0000000000..4aafb4bc0a --- /dev/null +++ b/apps/web/e2e/tests/task/mobile-plan-toolbar-implement.spec.ts @@ -0,0 +1,85 @@ +// Filename starts with "mobile-" so this runs under the mobile-chrome project. +import { type Page } from "@playwright/test"; +import { test, expect } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; +import type { SeedData } from "../../fixtures/test-base"; +import { planScript } from "../../helpers/seed-session-messages"; +import { SessionPage } from "../../pages/session-page"; + +async function seedMobileTaskWithPlan(testPage: Page, apiClient: ApiClient, seedData: SeedData) { + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Plan toolbar implement mobile", + seedData.agentProfileId, + { + description: planScript("## Plan\n\n1. Build the mobile toolbar implement action"), + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await expect.poll(() => apiClient.getTaskPlan(task.id), { timeout: 30_000 }).not.toBeNull(); + await session.waitForChatIdle({ timeout: 45_000 }); + await openMobilePlanPanel(testPage, session); + return { taskId: task.id, sessionId: task.session_id!, session }; +} + +async function openMobilePlanPanel(testPage: Page, session: SessionPage) { + await session.togglePlanMode(); + await testPage.getByRole("button", { name: "Plan" }).tap(); + await expect(session.planPanel).toBeVisible({ timeout: 10_000 }); +} + +test.describe("mobile: Plan toolbar implement", () => { + test.describe.configure({ retries: 1 }); + + test("marks the plan implemented and remains disabled after refresh", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + const { taskId, sessionId, session } = await seedMobileTaskWithPlan( + testPage, + apiClient, + seedData, + ); + + const toolbarButton = testPage.getByTestId("plan-toolbar-implement-button"); + await expect(toolbarButton).toBeVisible({ timeout: 10_000 }); + await expect(toolbarButton).toBeInViewport(); + + const overflow = await testPage.evaluate(() => { + const root = document.scrollingElement ?? document.documentElement; + return root.scrollWidth > root.clientWidth + 1; + }); + expect(overflow).toBe(false); + + await toolbarButton.tap(); + + await expect + .poll( + async () => { + const plan = await apiClient.getTaskPlan(taskId); + return plan?.implementation_started_session_id ?? null; + }, + { timeout: 30_000 }, + ) + .toBe(sessionId); + + await expect(toolbarButton).toBeVisible({ timeout: 10_000 }); + await expect(toolbarButton).toBeDisabled(); + + await testPage.reload(); + await session.waitForLoad(); + await openMobilePlanPanel(testPage, session); + await expect(testPage.getByTestId("plan-toolbar-implement-button")).toBeVisible({ + timeout: 10_000, + }); + await expect(testPage.getByTestId("plan-toolbar-implement-button")).toBeDisabled(); + }); +}); diff --git a/apps/web/e2e/tests/task/plan-toolbar-implement.spec.ts b/apps/web/e2e/tests/task/plan-toolbar-implement.spec.ts new file mode 100644 index 0000000000..ec115cd1b4 --- /dev/null +++ b/apps/web/e2e/tests/task/plan-toolbar-implement.spec.ts @@ -0,0 +1,99 @@ +import { type Page } from "@playwright/test"; +import { test, expect } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; +import type { SeedData } from "../../fixtures/test-base"; +import { planScript } from "../../helpers/seed-session-messages"; +import { SessionPage } from "../../pages/session-page"; + +async function seedTaskWithPlan( + testPage: Page, + apiClient: ApiClient, + seedData: SeedData, + title: string, +) { + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + title, + seedData.agentProfileId, + { + description: planScript("## Plan\n\n1. Build the toolbar implement action"), + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await expect.poll(() => apiClient.getTaskPlan(task.id), { timeout: 30_000 }).not.toBeNull(); + await session.waitForChatIdle({ timeout: 45_000 }); + await openPlanPanel(session); + return { taskId: task.id, sessionId: task.session_id!, session }; +} + +async function openPlanPanel(session: SessionPage) { + if (await session.planPanel.isVisible()) return; + await session.togglePlanMode(); + await expect(session.planPanel).toBeVisible({ timeout: 10_000 }); +} + +test.describe("Plan toolbar implement", () => { + test.describe.configure({ retries: 1 }); + + test("saves unsaved plan text, starts implementation, and stays disabled after refresh", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + const { taskId, sessionId, session } = await seedTaskWithPlan( + testPage, + apiClient, + seedData, + "Plan toolbar implement desktop", + ); + + const toolbarButton = testPage.getByTestId("plan-toolbar-implement-button"); + await expect(toolbarButton).toBeVisible({ timeout: 10_000 }); + + const uniqueEdit = `toolbar marker ${Date.now()}`; + await session.planEditor().click(); + await testPage.keyboard.press(process.platform === "darwin" ? "Meta+End" : "Control+End"); + await testPage.keyboard.type(`\n\n${uniqueEdit}`); + + await toolbarButton.click(); + + await expect + .poll( + async () => { + const plan = await apiClient.getTaskPlan(taskId); + return { + hasEdit: plan?.content.includes(uniqueEdit) ?? false, + markerSession: plan?.implementation_started_session_id ?? null, + }; + }, + { timeout: 30_000 }, + ) + .toEqual({ hasEdit: true, markerSession: sessionId }); + + const { messages } = await apiClient.listSessionMessages(sessionId); + const latestUserMessage = messages.filter((m) => m.author_type === "user").pop(); + expect(latestUserMessage?.content).toContain("Implement the plan"); + await expect(toolbarButton).toBeVisible({ timeout: 10_000 }); + await expect(toolbarButton).toBeDisabled(); + + await testPage.getByTestId("plan-toolbar-implement-control").hover(); + await expect( + testPage.getByText("This plan has already been sent for implementation."), + ).toBeVisible(); + + await testPage.reload(); + await session.waitForLoad(); + await openPlanPanel(session); + await expect(testPage.getByTestId("plan-toolbar-implement-button")).toBeVisible({ + timeout: 10_000, + }); + await expect(testPage.getByTestId("plan-toolbar-implement-button")).toBeDisabled(); + }); +}); diff --git a/apps/web/hooks/domains/kanban/use-implement-fresh.test.ts b/apps/web/hooks/domains/kanban/use-implement-fresh.test.ts index c7947023ad..7ea2e8525c 100644 --- a/apps/web/hooks/domains/kanban/use-implement-fresh.test.ts +++ b/apps/web/hooks/domains/kanban/use-implement-fresh.test.ts @@ -7,19 +7,24 @@ import { type TaskSession, } from "@/lib/types/http"; import type { MessageAttachment } from "@/components/task/chat/chat-input-container"; +import type { TaskPlan } from "@/lib/types/http-agents"; const mockLaunchSession = vi.fn(); +const mockMarkPlanImplementationStarted = vi.fn(); const mockSetChatDraftContent = vi.fn(); const mockToast = vi.fn(); const mockWsRequest = vi.fn(); const mockSetActiveSession = vi.fn(); +const mockSetTaskPlan = vi.fn(); let mockStoreState: { taskSessions: { items: Record }; setActiveSession: typeof mockSetActiveSession; + setTaskPlan: typeof mockSetTaskPlan; } = { taskSessions: { items: {} }, setActiveSession: mockSetActiveSession, + setTaskPlan: mockSetTaskPlan, }; vi.mock("@/components/state-provider", () => ({ @@ -34,6 +39,10 @@ vi.mock("@/lib/services/session-launch-service", () => ({ launchSession: (...args: unknown[]) => mockLaunchSession(...args), })); +vi.mock("@/lib/api/domains/plan-api", () => ({ + markPlanImplementationStarted: (...args: unknown[]) => mockMarkPlanImplementationStarted(...args), +})); + vi.mock("@/lib/ws/connection", () => ({ getWebSocketClient: () => ({ request: mockWsRequest }), })); @@ -52,6 +61,22 @@ const TASK_ID = "task-1"; const SESS_PLAN = "sess-plan"; const SESS_FRESH = "sess-fresh"; +function makePlan(overrides: Partial = {}): TaskPlan { + return { + id: "plan-1", + task_id: TASK_ID, + title: "Plan", + content: "## Plan", + created_by: "agent", + created_at: "", + updated_at: "", + implementation_started_at: "2026-07-09T12:00:00Z", + implementation_started_session_id: SESS_FRESH, + implementation_started_by: "user", + ...overrides, + }; +} + function makeSession(overrides: Partial = {}): TaskSession { return { id: toSessionId(SESS_PLAN), @@ -90,9 +115,11 @@ function setup(session: TaskSession | undefined = makeSession()) { state: "RUNNING", }); mockWsRequest.mockResolvedValue({ success: true }); + mockMarkPlanImplementationStarted.mockResolvedValue(makePlan()); mockStoreState = { taskSessions: { items: session ? { [session.id]: session } : {} }, setActiveSession: mockSetActiveSession, + setTaskPlan: mockSetTaskPlan, }; } @@ -161,6 +188,39 @@ describe("useImplementFresh", () => { ); }); + it("marks the task plan as implementation-started after launch", async () => { + const markedPlan = makePlan({ implementation_started_session_id: SESS_FRESH }); + mockMarkPlanImplementationStarted.mockResolvedValueOnce(markedPlan); + const { ref } = makeChatRef({ value: "implement" }); + const { result } = renderHook(() => useImplementFresh(SESS_PLAN, TASK_ID, ref)); + + await act(async () => { + await result.current(); + }); + + expect(mockMarkPlanImplementationStarted).toHaveBeenCalledWith(TASK_ID, SESS_FRESH); + expect(mockSetTaskPlan).toHaveBeenCalledWith(TASK_ID, markedPlan); + }); + + it("continues focusing and clearing when the marker write fails after launch", async () => { + mockMarkPlanImplementationStarted.mockRejectedValueOnce(new Error("marker offline")); + const { ref, clear } = makeChatRef({ value: "implement" }); + const { result } = renderHook(() => useImplementFresh(SESS_PLAN, TASK_ID, ref)); + + await act(async () => { + await result.current(); + }); + + expect(mockSetActiveSession).toHaveBeenCalledWith(TASK_ID, SESS_FRESH); + expect(clear).toHaveBeenCalledTimes(1); + expect(mockSetChatDraftContent).toHaveBeenCalledWith(SESS_PLAN, null); + expect(mockToast).not.toHaveBeenCalled(); + }); +}); + +describe("useImplementFresh post-launch side effects", () => { + beforeEach(() => setup()); + it("focuses the fresh session as active in the UI after launch", async () => { const { ref } = makeChatRef({ value: "implement" }); const { result } = renderHook(() => useImplementFresh(SESS_PLAN, TASK_ID, ref)); @@ -278,7 +338,11 @@ describe("useImplementFresh guards", () => { }); it("no-ops when planning session is not in the store", async () => { - mockStoreState = { taskSessions: { items: {} }, setActiveSession: mockSetActiveSession }; // Empty store + mockStoreState = { + taskSessions: { items: {} }, + setActiveSession: mockSetActiveSession, + setTaskPlan: mockSetTaskPlan, + }; // Empty store mockLaunchSession.mockClear(); const { ref } = makeChatRef(); const { result } = renderHook(() => useImplementFresh(SESS_PLAN, TASK_ID, ref)); diff --git a/apps/web/hooks/domains/kanban/use-implement-fresh.ts b/apps/web/hooks/domains/kanban/use-implement-fresh.ts index 9951652d48..70bd9e5029 100644 --- a/apps/web/hooks/domains/kanban/use-implement-fresh.ts +++ b/apps/web/hooks/domains/kanban/use-implement-fresh.ts @@ -5,7 +5,11 @@ import { useToast } from "@/components/toast-provider"; import { setChatDraftContent } from "@/lib/local-storage"; import { launchSession } from "@/lib/services/session-launch-service"; import { getWebSocketClient } from "@/lib/ws/connection"; -import { buildImplementPlanContent } from "./use-plan-actions"; +import { + buildImplementPlanContent, + collectImplementPlanInput, + markPlanImplementationStartedBestEffort, +} from "./use-plan-actions"; import type { ChatInputContainerHandle } from "@/components/task/chat/chat-input-container"; async function setupFreshSession(newSessionId: string): Promise { @@ -39,21 +43,24 @@ async function setupFreshSession(newSessionId: string): Promise { export function useImplementFresh( resolvedSessionId: string | null, taskId: string | null, - chatInputRef: React.RefObject, + chatInputRef?: React.RefObject, ) { const planningSession = useAppStore((s) => resolvedSessionId ? s.taskSessions.items[resolvedSessionId] : undefined, ); const setActiveSession = useAppStore((s) => s.setActiveSession); + const setTaskPlan = useAppStore((s) => s.setTaskPlan); const { toast } = useToast(); return useCallback(async () => { if (!taskId || !resolvedSessionId || !planningSession?.agent_profile_id) { - return; + return false; } - const userText = chatInputRef.current?.getValue() ?? ""; - const attachments = chatInputRef.current?.getAttachments() ?? []; + const { userText, attachments } = collectImplementPlanInput( + chatInputRef?.current, + resolvedSessionId, + ); const prompt = buildImplementPlanContent(userText); try { @@ -68,17 +75,28 @@ export function useImplementFresh( }); const newSessionId = response.session_id; - if (!newSessionId) return; + if (!newSessionId) return false; await setupFreshSession(newSessionId); + await markPlanImplementationStartedBestEffort(taskId, newSessionId, setTaskPlan); setActiveSession(taskId, newSessionId); // Clear composer + draft only when a fresh session was actually created. - chatInputRef.current?.clear(); + chatInputRef?.current?.clear(); setChatDraftContent(resolvedSessionId, null); + return true; } catch (err) { console.error("Failed to launch fresh implementation session:", err); toast({ description: "Failed to start implementation session", variant: "error" }); + return false; } - }, [taskId, resolvedSessionId, planningSession, chatInputRef, setActiveSession, toast]); + }, [ + taskId, + resolvedSessionId, + planningSession, + chatInputRef, + setTaskPlan, + setActiveSession, + toast, + ]); } diff --git a/apps/web/hooks/domains/kanban/use-plan-actions.runner.test.ts b/apps/web/hooks/domains/kanban/use-plan-actions.runner.test.ts new file mode 100644 index 0000000000..3ab3ad455f --- /dev/null +++ b/apps/web/hooks/domains/kanban/use-plan-actions.runner.test.ts @@ -0,0 +1,238 @@ +import { act, renderHook } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { TaskPlan } from "@/lib/types/http-agents"; +import type { TaskSession } from "@/lib/types/http"; + +const mockSetTaskPlan = vi.fn(); +const mockSetActiveSession = vi.fn(); +const mockToast = vi.fn(); +const mockWsRequest = vi.fn(); +const mockSetChatDraftContent = vi.fn(); +const mockMarkPlanImplementationStarted = vi.fn(); +const mockLaunchSession = vi.fn(); + +let mockStoreState: { + taskSessions: { items: Record }; + setActiveSession: typeof mockSetActiveSession; + setTaskPlan: typeof mockSetTaskPlan; +} = { + taskSessions: { items: {} }, + setActiveSession: mockSetActiveSession, + setTaskPlan: mockSetTaskPlan, +}; + +vi.mock("@/components/state-provider", () => ({ + useAppStore: (selector: (s: typeof mockStoreState) => unknown) => selector(mockStoreState), +})); + +vi.mock("@/components/toast-provider", () => ({ + useToast: () => ({ toast: mockToast }), +})); + +vi.mock("@/lib/ws/connection", () => ({ + getWebSocketClient: () => ({ request: mockWsRequest }), +})); + +vi.mock("@/lib/local-storage", async (importOriginal) => { + const actual = (await importOriginal()) as Record; + return { + ...actual, + setChatDraftContent: (...args: unknown[]) => mockSetChatDraftContent(...args), + }; +}); + +vi.mock("@/lib/api/domains/plan-api", () => ({ + markPlanImplementationStarted: (...args: unknown[]) => mockMarkPlanImplementationStarted(...args), +})); + +vi.mock("@/lib/services/session-launch-service", () => ({ + launchSession: (...args: unknown[]) => mockLaunchSession(...args), +})); + +vi.mock("@/lib/state/context-files-store", () => ({ + useContextFilesStore: { + getState: () => ({ filesBySessionId: {} }), + }, +})); + +import { useImplementPlanRunner } from "./use-plan-actions"; + +const TASK_ID = "task-1"; +const SESSION_ID = "session-1"; + +function makePlan(overrides: Partial = {}): TaskPlan { + return { + id: "plan-1", + task_id: TASK_ID, + title: "Plan", + content: "## Plan", + created_by: "agent", + created_at: "", + updated_at: "", + implementation_started_at: "2026-07-09T12:00:00Z", + implementation_started_session_id: SESSION_ID, + implementation_started_by: "user", + ...overrides, + }; +} + +function makeChatRef(value = "ship it") { + const clear = vi.fn(); + return { + ref: { + current: { + focusInput: vi.fn(), + getTextareaElement: () => null, + getValue: () => value, + getSelectionStart: () => 0, + insertText: vi.fn(), + getAttachments: () => [], + clear, + }, + }, + clear, + }; +} + +function setup() { + vi.clearAllMocks(); + mockStoreState = { + taskSessions: { items: {} }, + setActiveSession: mockSetActiveSession, + setTaskPlan: mockSetTaskPlan, + }; + mockWsRequest.mockResolvedValue({ success: true }); + mockMarkPlanImplementationStarted.mockResolvedValue(makePlan()); +} + +describe("useImplementPlanRunner same-session path", () => { + beforeEach(() => setup()); + + it("sends the implementation prompt, marks the plan, and clears plan mode", async () => { + const plan = makePlan(); + mockMarkPlanImplementationStarted.mockResolvedValueOnce(plan); + const handlePlanModeChange = vi.fn(); + const { ref, clear } = makeChatRef(); + const { result } = renderHook(() => + useImplementPlanRunner({ + resolvedSessionId: SESSION_ID, + taskId: TASK_ID, + handlePlanModeChange, + chatInputRef: ref, + }), + ); + + let ok = false; + await act(async () => { + ok = await result.current(false); + }); + + expect(ok).toBe(true); + expect(mockWsRequest).toHaveBeenNthCalledWith( + 1, + "message.add", + expect.objectContaining({ + task_id: TASK_ID, + session_id: SESSION_ID, + content: expect.stringContaining("ship it"), + plan_mode: false, + }), + 10000, + ); + expect(mockMarkPlanImplementationStarted).toHaveBeenCalledWith(TASK_ID, SESSION_ID); + expect(mockSetTaskPlan).toHaveBeenCalledWith(TASK_ID, plan); + expect(handlePlanModeChange).toHaveBeenCalledWith(false); + expect(clear).toHaveBeenCalledTimes(1); + expect(mockSetChatDraftContent).toHaveBeenCalledWith(SESSION_ID, null); + expect(mockWsRequest).toHaveBeenNthCalledWith( + 2, + "session.set_plan_mode", + { session_id: SESSION_ID, enabled: false }, + 5000, + ); + }); + + it("still clears plan mode when the durable marker write fails after send", async () => { + mockMarkPlanImplementationStarted.mockRejectedValueOnce(new Error("marker offline")); + const handlePlanModeChange = vi.fn(); + const { ref, clear } = makeChatRef(); + const { result } = renderHook(() => + useImplementPlanRunner({ + resolvedSessionId: SESSION_ID, + taskId: TASK_ID, + handlePlanModeChange, + chatInputRef: ref, + }), + ); + + let ok = false; + await act(async () => { + ok = await result.current(false); + }); + + expect(ok).toBe(true); + expect(handlePlanModeChange).toHaveBeenCalledWith(false); + expect(clear).toHaveBeenCalledTimes(1); + expect(mockToast).not.toHaveBeenCalled(); + expect(mockWsRequest).toHaveBeenCalledWith( + "session.set_plan_mode", + { session_id: SESSION_ID, enabled: false }, + 5000, + ); + }); + + it("returns false and preserves composer state when sending fails", async () => { + mockWsRequest.mockRejectedValueOnce(new Error("send failed")); + const handlePlanModeChange = vi.fn(); + const { ref, clear } = makeChatRef(); + const { result } = renderHook(() => + useImplementPlanRunner({ + resolvedSessionId: SESSION_ID, + taskId: TASK_ID, + handlePlanModeChange, + chatInputRef: ref, + }), + ); + + let ok = true; + await act(async () => { + ok = await result.current(false); + }); + + expect(ok).toBe(false); + expect(mockMarkPlanImplementationStarted).not.toHaveBeenCalled(); + expect(handlePlanModeChange).not.toHaveBeenCalled(); + expect(clear).not.toHaveBeenCalled(); + expect(mockToast).toHaveBeenCalledWith(expect.objectContaining({ variant: "error" })); + }); +}); + +describe("useImplementPlanRunner toolbar path", () => { + beforeEach(() => setup()); + + it("can keep plan mode open for toolbar implementation", async () => { + const plan = makePlan(); + mockMarkPlanImplementationStarted.mockResolvedValueOnce(plan); + const handlePlanModeChange = vi.fn(); + const { result } = renderHook(() => + useImplementPlanRunner({ + resolvedSessionId: SESSION_ID, + taskId: TASK_ID, + handlePlanModeChange, + clearPlanModeAfterSend: false, + }), + ); + + let ok = false; + await act(async () => { + ok = await result.current(false); + }); + + expect(ok).toBe(true); + expect(mockMarkPlanImplementationStarted).toHaveBeenCalledWith(TASK_ID, SESSION_ID); + expect(mockSetTaskPlan).toHaveBeenCalledWith(TASK_ID, plan); + expect(handlePlanModeChange).not.toHaveBeenCalled(); + expect(mockSetChatDraftContent).not.toHaveBeenCalled(); + expect(mockWsRequest).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/web/hooks/domains/kanban/use-plan-actions.test.ts b/apps/web/hooks/domains/kanban/use-plan-actions.test.ts index b277a9a22b..dfddf54d7c 100644 --- a/apps/web/hooks/domains/kanban/use-plan-actions.test.ts +++ b/apps/web/hooks/domains/kanban/use-plan-actions.test.ts @@ -7,7 +7,11 @@ vi.mock("@/lib/state/context-files-store", () => ({ useContextFilesStore: { getState: () => mockGetState() }, })); -import { buildImplementPlanContent, readContextFilesMeta } from "./use-plan-actions"; +import { + buildImplementPlanContent, + collectImplementPlanInput, + readContextFilesMeta, +} from "./use-plan-actions"; describe("buildImplementPlanContent", () => { it("appends the kandev-system block to user text", () => { @@ -77,3 +81,14 @@ describe("readContextFilesMeta", () => { expect(readContextFilesMeta(sessionId)).toEqual([{ path: appFilePath, name: appFileName }]); }); }); + +describe("collectImplementPlanInput", () => { + it("uses default empty input when no chat input handle exists", () => { + mockGetState.mockReturnValue({ filesBySessionId: {} }); + expect(collectImplementPlanInput(null, "sess-1")).toEqual({ + userText: "", + attachments: [], + contextFilesMeta: [], + }); + }); +}); diff --git a/apps/web/hooks/domains/kanban/use-plan-actions.ts b/apps/web/hooks/domains/kanban/use-plan-actions.ts index 4579b37925..79c44afd89 100644 --- a/apps/web/hooks/domains/kanban/use-plan-actions.ts +++ b/apps/web/hooks/domains/kanban/use-plan-actions.ts @@ -8,7 +8,11 @@ import { useContextFilesStore } from "@/lib/state/context-files-store"; import { useLayoutStore } from "@/lib/state/layout-store"; import { useDockviewStore } from "@/lib/state/dockview-store"; import { useImplementFresh } from "./use-implement-fresh"; -import type { ChatInputContainerHandle } from "@/components/task/chat/chat-input-container"; +import { markPlanImplementationStarted } from "@/lib/api/domains/plan-api"; +import type { + ChatInputContainerHandle, + MessageAttachment, +} from "@/components/task/chat/chat-input-container"; const PLAN_CONTEXT_PATH = "plan:context"; @@ -108,26 +112,64 @@ export function readContextFilesMeta(sessionId: string): Array<{ path: string; n .map((f) => ({ path: f.path, name: f.name })); } +export function collectImplementPlanInput( + chatInput: ChatInputContainerHandle | null | undefined, + sessionId: string | null, +): { + userText: string; + attachments: MessageAttachment[]; + contextFilesMeta: Array<{ path: string; name: string }>; +} { + if (!chatInput || !sessionId) { + return { userText: "", attachments: [], contextFilesMeta: [] }; + } + return { + userText: chatInput.getValue(), + attachments: chatInput.getAttachments(), + contextFilesMeta: readContextFilesMeta(sessionId), + }; +} + +export async function markPlanImplementationStartedBestEffort( + taskId: string, + sessionId: string, + setTaskPlan: ( + taskId: string, + plan: Awaited>, + ) => void, +) { + try { + const markedPlan = await markPlanImplementationStarted(taskId, sessionId); + setTaskPlan(taskId, markedPlan); + } catch (err) { + console.error("Failed to mark plan implementation started:", err); + } +} + function useImplementPlan( resolvedSessionId: string | null, taskId: string | null, - handlePlanModeChange: (enabled: boolean) => void, - chatInputRef: React.RefObject, + handlePlanModeChange: ((enabled: boolean) => void) | undefined, + clearPlanModeAfterSend: boolean, + chatInputRef?: React.RefObject, ) { - return useCallback(() => { - if (!resolvedSessionId || !taskId) return; + const setTaskPlan = useAppStore((s) => s.setTaskPlan); + const { toast } = useToast(); + return useCallback(async (): Promise => { + if (!resolvedSessionId || !taskId) return false; const client = getWebSocketClient(); - if (!client) return; + if (!client) return false; - const userText = chatInputRef.current?.getValue() ?? ""; - const attachments = chatInputRef.current?.getAttachments() ?? []; - const contextFilesMeta = readContextFilesMeta(resolvedSessionId); + const { userText, attachments, contextFilesMeta } = collectImplementPlanInput( + chatInputRef?.current, + resolvedSessionId, + ); const content = buildImplementPlanContent(userText); - client - .request( + try { + await client.request( "message.add", { task_id: taskId, @@ -138,25 +180,43 @@ function useImplementPlan( ...(contextFilesMeta.length > 0 && { context_files: contextFilesMeta }), }, attachments.length > 0 ? 30000 : 10000, - ) - .then(() => { - // Exit plan mode + clear composer only on success so a failed send - // leaves the layout and input intact for retry. - handlePlanModeChange(false); + ); + await markPlanImplementationStartedBestEffort(taskId, resolvedSessionId, setTaskPlan); + // Exit plan mode + clear composer only on success so a failed send + // leaves the layout and input intact for retry. + if (clearPlanModeAfterSend) { + handlePlanModeChange?.(false); + } + if (chatInputRef) { chatInputRef.current?.clear(); setChatDraftContent(resolvedSessionId, null); - // Authoritatively clear plan_mode in session metadata so a refresh - // mid-implementation cannot re-hydrate plan mode from the server. - // Run as a separate request with its own catch so a set_plan_mode - // failure doesn't masquerade as a message send failure. + } + // Authoritatively clear plan_mode in session metadata so a refresh + // mid-implementation cannot re-hydrate plan mode from the server. + // Run as a separate request with its own catch so a set_plan_mode + // failure doesn't masquerade as a message send failure. + if (clearPlanModeAfterSend) { client .request("session.set_plan_mode", { session_id: resolvedSessionId, enabled: false }, 5000) .catch((err: unknown) => console.error("Failed to clear plan mode after implement:", err), ); - }) - .catch((err: unknown) => console.error("Failed to send implement plan message:", err)); - }, [resolvedSessionId, taskId, handlePlanModeChange, chatInputRef]); + } + return true; + } catch (err) { + console.error("Failed to start implementation:", err); + toast({ description: "Failed to start implementing the plan", variant: "error" }); + return false; + } + }, [ + resolvedSessionId, + taskId, + chatInputRef, + setTaskPlan, + clearPlanModeAfterSend, + handlePlanModeChange, + toast, + ]); } /** Directly disable plan mode state + layout, bypassing the MCP availability guard. */ @@ -191,17 +251,12 @@ export function usePlanActions(opts: { handlePlanModeChange: (enabled: boolean) => void; chatInputRef: React.RefObject; }) { - const handleImplementPlan = useImplementPlan( - opts.resolvedSessionId, - opts.taskId, - opts.handlePlanModeChange, - opts.chatInputRef, - ); - const handleImplementFresh = useImplementFresh( - opts.resolvedSessionId, - opts.taskId, - opts.chatInputRef, - ); + const implementPlan = useImplementPlanRunner({ + resolvedSessionId: opts.resolvedSessionId, + taskId: opts.taskId, + handlePlanModeChange: opts.handlePlanModeChange, + chatInputRef: opts.chatInputRef, + }); const { proceedStepName, nextStepIsWorkStep, @@ -221,8 +276,31 @@ export function usePlanActions(opts: { }, [planModeEnabled, disablePlanMode, rawProceed]); const showImplement = opts.planModeEnabled && !nextStepIsWorkStep; - const implementPlanHandler = showImplement - ? (fresh: boolean) => (fresh ? handleImplementFresh() : handleImplementPlan()) - : undefined; + const implementPlanHandler = showImplement ? implementPlan : undefined; return { implementPlanHandler, proceedStepName, proceed, isMoving }; } + +export function useImplementPlanRunner(opts: { + resolvedSessionId: string | null; + taskId: string | null; + handlePlanModeChange?: (enabled: boolean) => void; + clearPlanModeAfterSend?: boolean; + chatInputRef?: React.RefObject; +}) { + const handleImplementPlan = useImplementPlan( + opts.resolvedSessionId, + opts.taskId, + opts.handlePlanModeChange, + opts.clearPlanModeAfterSend ?? true, + opts.chatInputRef, + ); + const handleImplementFresh = useImplementFresh( + opts.resolvedSessionId, + opts.taskId, + opts.chatInputRef, + ); + return useCallback( + (fresh: boolean) => (fresh ? handleImplementFresh() : handleImplementPlan()), + [handleImplementFresh, handleImplementPlan], + ); +} diff --git a/apps/web/lib/api/domains/plan-api.ts b/apps/web/lib/api/domains/plan-api.ts index 6da9cc43f0..488e1bf055 100644 --- a/apps/web/lib/api/domains/plan-api.ts +++ b/apps/web/lib/api/domains/plan-api.ts @@ -70,6 +70,22 @@ export async function updateTaskPlan( return response as TaskPlan; } +export async function markPlanImplementationStarted( + taskId: string, + sessionId: string, +): Promise { + const client = getWebSocketClient(); + if (!client) { + throw new Error(WS_CLIENT_UNAVAILABLE); + } + const response = await client.request("task.plan.implementation_started", { + task_id: taskId, + session_id: sessionId, + actor: "user", + }); + return response as TaskPlan; +} + /** * Delete a task plan. */ diff --git a/apps/web/lib/types/backend.ts b/apps/web/lib/types/backend.ts index b0938c8f4e..2df097e7df 100644 --- a/apps/web/lib/types/backend.ts +++ b/apps/web/lib/types/backend.ts @@ -1,3 +1,5 @@ +import type { TaskPlanEventPayload, TaskPlanRevisionEventPayload } from "./task-plan-events"; + export type BackendMessageType = keyof BackendMessageMap; export type { BackendMessage } from "./backend-message"; @@ -409,28 +411,7 @@ export { type SessionTodosPayload, } from "./session-runtime-payloads"; -export type TaskPlanEventPayload = { - id: string; - task_id: string; - title: string; - content: string; - created_by: "agent" | "user"; - created_at: string; - updated_at: string; -}; - -export type TaskPlanRevisionEventPayload = { - id: string; - task_id: string; - revision_number: number; - title: string; - author_kind: "agent" | "user"; - author_name: string; - revert_of_revision_id?: string | null; - coalesced?: boolean; - created_at: string; - updated_at: string; -}; +export type { TaskPlanEventPayload, TaskPlanRevisionEventPayload } from "./task-plan-events"; export type QueuedMessagePayload = { content: string; @@ -566,10 +547,7 @@ export type BackendMessageMap = OfficeBackendMessageMap & "run.event.appended": BackendMessage<"run.event.appended", RunEventAppendedPayload>; }; -// Run event payload — the WS gateway forwards the bus event verbatim, -// which means the run id sits at the top level and the full RunEvent -// row sits under `event`. The handler dispatches to subscribers keyed -// on `run_id`. +// The WS gateway forwards run events verbatim: id at top level, row under `event`. export type RunEventAppendedPayload = { run_id: string; event: { diff --git a/apps/web/lib/types/http-agents.ts b/apps/web/lib/types/http-agents.ts index 1fbc9e8c3a..7089588b63 100644 --- a/apps/web/lib/types/http-agents.ts +++ b/apps/web/lib/types/http-agents.ts @@ -285,6 +285,9 @@ export type TaskPlan = { created_by: "agent" | "user"; created_at: string; updated_at: string; + implementation_started_at?: string | null; + implementation_started_session_id?: string | null; + implementation_started_by?: string | null; }; export type TaskPlanResponse = { @@ -332,6 +335,7 @@ export type TaskPlanRevision = { author_kind: "agent" | "user"; author_name: string; revert_of_revision_id?: string | null; + coalesced?: boolean; created_at: string; updated_at: string; }; diff --git a/apps/web/lib/types/task-plan-events.ts b/apps/web/lib/types/task-plan-events.ts new file mode 100644 index 0000000000..fd29e43a32 --- /dev/null +++ b/apps/web/lib/types/task-plan-events.ts @@ -0,0 +1,25 @@ +export type TaskPlanEventPayload = { + id: string; + task_id: string; + title: string; + content: string; + created_by: "agent" | "user"; + created_at: string; + updated_at: string; + implementation_started_at?: string | null; + implementation_started_session_id?: string | null; + implementation_started_by?: string | null; +}; + +export type TaskPlanRevisionEventPayload = { + id: string; + task_id: string; + revision_number: number; + title: string; + author_kind: "agent" | "user"; + author_name: string; + revert_of_revision_id?: string | null; + coalesced?: boolean; + created_at: string; + updated_at: string; +}; diff --git a/apps/web/lib/ws/handlers/task-plans.ts b/apps/web/lib/ws/handlers/task-plans.ts index 81b4c10945..68b1c50ebb 100644 --- a/apps/web/lib/ws/handlers/task-plans.ts +++ b/apps/web/lib/ws/handlers/task-plans.ts @@ -10,7 +10,18 @@ type RevisionMessage = | BackendMessageMap["task.plan.reverted"]; function handlePlanUpsert(store: StoreApi, message: PlanMessage) { - const { task_id, id, title, content, created_by, created_at, updated_at } = message.payload; + const { + task_id, + id, + title, + content, + created_by, + created_at, + updated_at, + implementation_started_at, + implementation_started_session_id, + implementation_started_by, + } = message.payload; const prevPlan = store.getState().taskPlans.byTaskId[task_id]; store.getState().setTaskPlan(task_id, { id, @@ -20,6 +31,9 @@ function handlePlanUpsert(store: StoreApi, message: PlanMessage) { created_by, created_at, updated_at, + implementation_started_at, + implementation_started_session_id, + implementation_started_by, }); // User-authored writes mark the plan as seen — but only when the content @@ -42,6 +56,7 @@ function handleRevisionPush(store: StoreApi, message: RevisionMessage) author_kind: p.author_kind, author_name: p.author_name, revert_of_revision_id: p.revert_of_revision_id ?? null, + coalesced: p.coalesced, created_at: p.created_at, updated_at: p.updated_at, }; diff --git a/docs/decisions/0033-durable-plan-implementation-start.md b/docs/decisions/0033-durable-plan-implementation-start.md new file mode 100644 index 0000000000..4fc438ba7e --- /dev/null +++ b/docs/decisions/0033-durable-plan-implementation-start.md @@ -0,0 +1,27 @@ +# 0033: Durable Plan Implementation Start Marker + +**Status:** accepted +**Date:** 2026-07-09 +**Area:** backend, frontend + +## Context + +Plan mode can create a reviewed task plan, then the user may start implementation from the chat composer, the plan toolbar, or by typing an equivalent instruction directly. The UI needs to know whether to keep showing a prominent Implement action after the user starts implementation. Inferring that from messages is ambiguous because the agent could be asked to implement without using a button, messages can be edited or summarized, and page refreshes must preserve the answer. + +## Decision + +`task_plans` stores an implementation-start marker: timestamp, session id, and actor. The marker is set by an explicit backend action when the UI starts implementation from an Implement button. The write is idempotent: the first implementation start wins, and later clicks or plan edits do not clear or replace it. + +Task plan reads, WebSocket plan events, and frontend task-plan state include the marker. The plan toolbar shows its accent Implement action whenever the current draft has non-empty content, and disables it with an explanatory tooltip once the persisted marker is present. Before starting implementation from the toolbar, the frontend saves any unsaved plan edits, sends the normal implement prompt to the target session, then marks the plan implemented so the button remains visible but disabled after refresh. + +## Consequences + +The UI no longer needs to infer implementation state from chat history, so refreshes and multi-client updates behave consistently. Keeping the marker on `task_plans` also avoids coupling the toolbar to any one agent/session message format. + +Direct typed implementation requests that bypass the button are not detected automatically; they remain intentionally outside this marker because the system cannot reliably distinguish them from discussion or partial implementation requests. + +## Alternatives Considered + +1. **Infer from session messages.** Rejected because prompt text, hidden system blocks, summaries, and manual user instructions make detection unreliable. +2. **Use browser-local state.** Rejected because it would not survive another client, DB-backed task reload, or durable page refresh semantics. +3. **Clear the marker when the plan changes.** Rejected because editing the plan after starting implementation should not reintroduce the primary Implement CTA for the same task. diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index d95fc5c14e..fc51039dbe 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -38,3 +38,4 @@ Read individual ADRs for full context. Create new ones via `/record decision` or | 0030 | [Workspace-scoped integration settings](0030-workspace-scoped-integration-settings.md) | accepted | backend, frontend | 2026-07-01 | | 0031 | [Office skill reference files](0031-office-skill-reference-files.md) | accepted | backend | 2026-07-06 | | 0032 | [Configurable worktree branch names](0032-configurable-worktree-branch-names.md) | accepted | backend, frontend | 2026-07-07 | +| 0033 | [Durable plan implementation start marker](0033-durable-plan-implementation-start.md) | accepted | backend, frontend | 2026-07-09 |