diff --git a/apps/backend/internal/events/types.go b/apps/backend/internal/events/types.go index 06febd1272..80f5e54fd3 100644 --- a/apps/backend/internal/events/types.go +++ b/apps/backend/internal/events/types.go @@ -49,9 +49,11 @@ const ( // Event types for task plans const ( - TaskPlanCreated = "task_plan.created" - TaskPlanUpdated = "task_plan.updated" - TaskPlanDeleted = "task_plan.deleted" + TaskPlanCreated = "task_plan.created" + TaskPlanUpdated = "task_plan.updated" + TaskPlanDeleted = "task_plan.deleted" + TaskPlanRevisionCreated = "task_plan.revision.created" + TaskPlanReverted = "task_plan.reverted" ) // Event types for session turns diff --git a/apps/backend/internal/gateway/websocket/task_notifications.go b/apps/backend/internal/gateway/websocket/task_notifications.go index f2e12e054c..5956fb0974 100644 --- a/apps/backend/internal/gateway/websocket/task_notifications.go +++ b/apps/backend/internal/gateway/websocket/task_notifications.go @@ -44,6 +44,8 @@ func RegisterTaskNotifications(ctx context.Context, eventBus bus.EventBus, hub * b.subscribe(eventBus, events.TaskPlanCreated, ws.ActionTaskPlanCreated) b.subscribe(eventBus, events.TaskPlanUpdated, ws.ActionTaskPlanUpdated) b.subscribe(eventBus, events.TaskPlanDeleted, ws.ActionTaskPlanDeleted) + b.subscribe(eventBus, events.TaskPlanRevisionCreated, ws.ActionTaskPlanRevisionCreated) + b.subscribe(eventBus, events.TaskPlanReverted, ws.ActionTaskPlanReverted) b.subscribe(eventBus, events.RepositoryCreated, ws.ActionRepositoryCreated) b.subscribe(eventBus, events.RepositoryUpdated, ws.ActionRepositoryUpdated) b.subscribe(eventBus, events.RepositoryDeleted, ws.ActionRepositoryDeleted) diff --git a/apps/backend/internal/gateway/websocket/task_notifications_test.go b/apps/backend/internal/gateway/websocket/task_notifications_test.go index 27d5d4a143..3efae8079c 100644 --- a/apps/backend/internal/gateway/websocket/task_notifications_test.go +++ b/apps/backend/internal/gateway/websocket/task_notifications_test.go @@ -39,7 +39,7 @@ func TestTaskEventBroadcaster_NoDuplicateSubscriptions(t *testing.T) { // // Update this number when adding or removing event subscriptions in // RegisterTaskNotifications — it is intentionally exact. - const wantSubscriptions = 46 + const wantSubscriptions = 48 if got := len(b.subscriptions); got != wantSubscriptions { t.Errorf("RegisterTaskNotifications created %d subscriptions, want %d — "+ "did an event get subscribed twice?", got, wantSubscriptions) diff --git a/apps/backend/internal/task/dto/dto.go b/apps/backend/internal/task/dto/dto.go index f03c2ba6dd..6af7d29386 100644 --- a/apps/backend/internal/task/dto/dto.go +++ b/apps/backend/internal/task/dto/dto.go @@ -697,6 +697,49 @@ func TaskPlanFromModel(plan *models.TaskPlan) *TaskPlanDTO { } } +// TaskPlanRevisionDTO represents a plan revision for API responses. +// Content is optional so list responses can omit it (fetched on demand). +type TaskPlanRevisionDTO struct { + ID string `json:"id"` + TaskID string `json:"task_id"` + RevisionNumber int `json:"revision_number"` + Title string `json:"title"` + Content string `json:"content,omitempty"` + AuthorKind string `json:"author_kind"` + AuthorName string `json:"author_name"` + RevertOfRevisionID *string `json:"revert_of_revision_id,omitempty"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` +} + +// TaskPlanRevisionFromModel converts a TaskPlanRevision model with content. +func TaskPlanRevisionFromModel(rev *models.TaskPlanRevision) *TaskPlanRevisionDTO { + if rev == nil { + return nil + } + return &TaskPlanRevisionDTO{ + ID: rev.ID, + TaskID: rev.TaskID, + RevisionNumber: rev.RevisionNumber, + Title: rev.Title, + Content: rev.Content, + AuthorKind: rev.AuthorKind, + AuthorName: rev.AuthorName, + RevertOfRevisionID: rev.RevertOfRevisionID, + CreatedAt: rev.CreatedAt, + UpdatedAt: rev.UpdatedAt, + } +} + +// TaskPlanRevisionMetaFromModel converts without content (for list payloads/WS broadcasts). +func TaskPlanRevisionMetaFromModel(rev *models.TaskPlanRevision) *TaskPlanRevisionDTO { + meta := TaskPlanRevisionFromModel(rev) + if meta != nil { + meta.Content = "" + } + return meta +} + // FromTurn converts a Turn model to a TurnDTO. func FromTurn(turn *models.Turn) TurnDTO { var completedAt *string diff --git a/apps/backend/internal/task/handlers/task_handlers.go b/apps/backend/internal/task/handlers/task_handlers.go index e28e541838..b7e060a82f 100644 --- a/apps/backend/internal/task/handlers/task_handlers.go +++ b/apps/backend/internal/task/handlers/task_handlers.go @@ -109,6 +109,9 @@ func (h *TaskHandlers) registerWS(dispatcher *ws.Dispatcher) { dispatcher.RegisterFunc(ws.ActionTaskPlanGet, h.wsGetTaskPlan) dispatcher.RegisterFunc(ws.ActionTaskPlanUpdate, h.wsUpdateTaskPlan) dispatcher.RegisterFunc(ws.ActionTaskPlanDelete, h.wsDeleteTaskPlan) + dispatcher.RegisterFunc(ws.ActionTaskPlanRevisionsList, h.wsListTaskPlanRevisions) + dispatcher.RegisterFunc(ws.ActionTaskPlanRevisionGet, h.wsGetTaskPlanRevision) + dispatcher.RegisterFunc(ws.ActionTaskPlanRevert, h.wsRevertTaskPlan) } // 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 2a74171231..a9a3b92de0 100644 --- a/apps/backend/internal/task/handlers/task_plan_handlers.go +++ b/apps/backend/internal/task/handlers/task_plan_handlers.go @@ -15,26 +15,29 @@ import ( // wsCreateTaskPlan creates a new task plan func (h *TaskHandlers) wsCreateTaskPlan(ctx context.Context, msg *ws.Message) (*ws.Message, error) { var req struct { - TaskID string `json:"task_id"` - Title string `json:"title"` - Content string `json:"content"` - CreatedBy string `json:"created_by"` + TaskID string `json:"task_id"` + Title string `json:"title"` + Content string `json:"content"` + CreatedBy string `json:"created_by"` + AuthorKind string `json:"author_kind"` + AuthorName string `json:"author_name"` } if err := json.Unmarshal(msg.Payload, &req); err != nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil) } - // Default to "user" for frontend requests createdBy := req.CreatedBy if createdBy == "" { createdBy = "user" } plan, err := h.planService.CreatePlan(ctx, service.CreatePlanRequest{ - TaskID: req.TaskID, - Title: req.Title, - Content: req.Content, - CreatedBy: createdBy, + TaskID: req.TaskID, + Title: req.Title, + Content: req.Content, + CreatedBy: createdBy, + AuthorKind: req.AuthorKind, + AuthorName: req.AuthorName, }) if err != nil { if errors.Is(err, service.ErrTaskIDRequired) { @@ -72,26 +75,29 @@ func (h *TaskHandlers) wsGetTaskPlan(ctx context.Context, msg *ws.Message) (*ws. // wsUpdateTaskPlan updates an existing task plan func (h *TaskHandlers) wsUpdateTaskPlan(ctx context.Context, msg *ws.Message) (*ws.Message, error) { var req struct { - TaskID string `json:"task_id"` - Title string `json:"title"` - Content string `json:"content"` - CreatedBy string `json:"created_by"` + TaskID string `json:"task_id"` + Title string `json:"title"` + Content string `json:"content"` + CreatedBy string `json:"created_by"` + AuthorKind string `json:"author_kind"` + AuthorName string `json:"author_name"` } if err := json.Unmarshal(msg.Payload, &req); err != nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil) } - // Default to "user" for frontend requests createdBy := req.CreatedBy if createdBy == "" { createdBy = "user" } plan, err := h.planService.UpdatePlan(ctx, service.UpdatePlanRequest{ - TaskID: req.TaskID, - Title: req.Title, - Content: req.Content, - CreatedBy: createdBy, + TaskID: req.TaskID, + Title: req.Title, + Content: req.Content, + CreatedBy: createdBy, + AuthorKind: req.AuthorKind, + AuthorName: req.AuthorName, }) if err != nil { if errors.Is(err, service.ErrTaskIDRequired) { @@ -128,3 +134,93 @@ func (h *TaskHandlers) wsDeleteTaskPlan(ctx context.Context, msg *ws.Message) (* return ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{"success": true}) } + +// wsListTaskPlanRevisions returns revision metadata newest-first (no content). +func (h *TaskHandlers) wsListTaskPlanRevisions(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + var req struct { + TaskID string `json:"task_id"` + } + if err := json.Unmarshal(msg.Payload, &req); err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil) + } + + revs, err := h.planService.ListRevisions(ctx, req.TaskID) + if err != nil { + if errors.Is(err, service.ErrTaskIDRequired) { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "task_id is required", nil) + } + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "Failed to list revisions: "+err.Error(), nil) + } + + out := make([]*dto.TaskPlanRevisionDTO, 0, len(revs)) + for _, r := range revs { + out = append(out, dto.TaskPlanRevisionMetaFromModel(r)) + } + return ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{"revisions": out}) +} + +// wsGetTaskPlanRevision returns a single revision with content. +func (h *TaskHandlers) wsGetTaskPlanRevision(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + var req struct { + TaskID string `json:"task_id"` + RevisionID string `json:"revision_id"` + } + if err := json.Unmarshal(msg.Payload, &req); err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil) + } + if req.RevisionID == "" { + // Surface missing argument as a 400, matching wsRevertTaskPlan's + // ErrRevisionIDRequired branch — without this guard, an empty id + // falls through to the service and returns 404 (lookup miss), + // which is misleading for the caller. + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "revision_id is required", nil) + } + + rev, err := h.planService.GetRevision(ctx, req.RevisionID) + if err != nil { + if errors.Is(err, service.ErrRevisionNotFound) { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeNotFound, "Revision not found", nil) + } + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "Failed to get revision: "+err.Error(), nil) + } + // Match wsRevertTaskPlan's ownership check so a caller can only read + // content for revisions belonging to the task they hold a reference to. + // Optional task_id keeps existing callers compatible while letting the + // frontend tighten the request shape. + if req.TaskID != "" && rev.TaskID != req.TaskID { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "Revision does not belong to task", nil) + } + return ws.NewResponse(msg.ID, msg.Action, dto.TaskPlanRevisionFromModel(rev)) +} + +// wsRevertTaskPlan reverts the plan HEAD to a target revision by appending a new revision. +func (h *TaskHandlers) wsRevertTaskPlan(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + var req struct { + TaskID string `json:"task_id"` + RevisionID string `json:"revision_id"` + AuthorName string `json:"author_name"` + } + if err := json.Unmarshal(msg.Payload, &req); err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil) + } + + rev, err := h.planService.RevertPlan(ctx, service.RevertPlanRequest{ + TaskID: req.TaskID, + TargetRevisionID: req.RevisionID, + 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 ws.NewResponse(msg.ID, msg.Action, dto.TaskPlanRevisionFromModel(rev)) +} diff --git a/apps/backend/internal/task/models/models.go b/apps/backend/internal/task/models/models.go index 946905d7fe..cbe087eb17 100644 --- a/apps/backend/internal/task/models/models.go +++ b/apps/backend/internal/task/models/models.go @@ -611,6 +611,21 @@ type TaskPlan struct { UpdatedAt time.Time `json:"updated_at"` } +// TaskPlanRevision is one immutable snapshot in the revision history of a task plan. +// Revisions are the source of truth for history; TaskPlan stores the latest revision's content as HEAD. +type TaskPlanRevision struct { + ID string `json:"id"` + TaskID string `json:"task_id"` + RevisionNumber int `json:"revision_number"` + Title string `json:"title"` + Content string `json:"content"` + AuthorKind string `json:"author_kind"` // "agent" | "user" + AuthorName string `json:"author_name"` // display snapshot (agent profile name or user identifier) + RevertOfRevisionID *string `json:"revert_of_revision_id,omitempty"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` // bumps on coalesce merge +} + // SessionFileReview tracks per-file review state within a session type SessionFileReview struct { ID string `json:"id"` diff --git a/apps/backend/internal/task/repository/interface.go b/apps/backend/internal/task/repository/interface.go index ccb22c2a54..d3d04887ea 100644 --- a/apps/backend/internal/task/repository/interface.go +++ b/apps/backend/internal/task/repository/interface.go @@ -198,10 +198,22 @@ type ReviewRepository interface { DeleteSessionFileReviews(ctx context.Context, sessionID string) error } -// PlanRepository handles task plan CRUD. +// PlanRepository handles task plan CRUD and its revision history. 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 DeleteTaskPlan(ctx context.Context, taskID string) error + + // Revision history + InsertTaskPlanRevision(ctx context.Context, rev *models.TaskPlanRevision) error + UpdateTaskPlanRevision(ctx context.Context, rev *models.TaskPlanRevision) error + GetTaskPlanRevision(ctx context.Context, id string) (*models.TaskPlanRevision, error) + GetLatestTaskPlanRevision(ctx context.Context, taskID string) (*models.TaskPlanRevision, error) + ListTaskPlanRevisions(ctx context.Context, taskID string, limit int) ([]*models.TaskPlanRevision, error) + NextTaskPlanRevisionNumber(ctx context.Context, taskID string) (int, error) + // WritePlanRevision atomically upserts the HEAD plan and writes/merges a revision in a + // single transaction. Pass a non-nil coalesceLatestID to merge into an existing revision; + // otherwise a new revision is appended with revision_number computed inside the tx. + WritePlanRevision(ctx context.Context, head *models.TaskPlan, rev *models.TaskPlanRevision, coalesceLatestID *string) error } diff --git a/apps/backend/internal/task/repository/plan_revisions_test.go b/apps/backend/internal/task/repository/plan_revisions_test.go new file mode 100644 index 0000000000..fbe59aa833 --- /dev/null +++ b/apps/backend/internal/task/repository/plan_revisions_test.go @@ -0,0 +1,249 @@ +package repository + +import ( + "context" + "path/filepath" + "testing" + "time" + + "github.com/jmoiron/sqlx" + "github.com/kandev/kandev/internal/db" + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/repository/sqlite" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +func seedPlanRevisionTask(t *testing.T, ctx context.Context, repo interface { + CreateWorkspace(context.Context, *models.Workspace) error + CreateWorkflow(context.Context, *models.Workflow) error + CreateTask(context.Context, *models.Task) error +}, taskID string) { + t.Helper() + _ = repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-rev", Name: "Rev WS"}) + _ = repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-rev", WorkspaceID: "ws-rev", Name: "WF"}) + now := time.Now().UTC() + _ = repo.CreateTask(ctx, &models.Task{ + ID: taskID, WorkspaceID: "ws-rev", WorkflowID: "wf-rev", + Title: "T", State: v1.TaskStateCreated, CreatedAt: now, UpdatedAt: now, + }) +} + +func TestPlanRevisions_InsertAndList(t *testing.T) { + repo, cleanup := createTestSQLiteRepo(t) + defer cleanup() + ctx := context.Background() + seedPlanRevisionTask(t, ctx, repo, "task-r1") + + for i := 1; i <= 3; i++ { + rev := &models.TaskPlanRevision{ + TaskID: "task-r1", RevisionNumber: i, + Title: "Plan", Content: "c", + AuthorKind: "agent", AuthorName: "Agent", + } + if err := repo.InsertTaskPlanRevision(ctx, rev); err != nil { + t.Fatalf("insert %d: %v", i, err) + } + } + + list, err := repo.ListTaskPlanRevisions(ctx, "task-r1", 0) + if err != nil { + t.Fatalf("list: %v", err) + } + if len(list) != 3 { + t.Fatalf("expected 3, got %d", len(list)) + } + if list[0].RevisionNumber != 3 || list[2].RevisionNumber != 1 { + t.Errorf("expected newest-first ordering, got %d..%d", list[0].RevisionNumber, list[2].RevisionNumber) + } +} + +func TestPlanRevisions_NextRevisionNumber(t *testing.T) { + repo, cleanup := createTestSQLiteRepo(t) + defer cleanup() + ctx := context.Background() + seedPlanRevisionTask(t, ctx, repo, "task-r2") + + n, err := repo.NextTaskPlanRevisionNumber(ctx, "task-r2") + if err != nil { + t.Fatalf("next: %v", err) + } + if n != 1 { + t.Errorf("expected 1, got %d", n) + } + + _ = repo.InsertTaskPlanRevision(ctx, &models.TaskPlanRevision{ + TaskID: "task-r2", RevisionNumber: 1, AuthorKind: "agent", + }) + _ = repo.InsertTaskPlanRevision(ctx, &models.TaskPlanRevision{ + TaskID: "task-r2", RevisionNumber: 2, AuthorKind: "agent", + }) + + n, _ = repo.NextTaskPlanRevisionNumber(ctx, "task-r2") + if n != 3 { + t.Errorf("expected 3, got %d", n) + } +} + +func TestPlanRevisions_UniqueConstraint(t *testing.T) { + repo, cleanup := createTestSQLiteRepo(t) + defer cleanup() + ctx := context.Background() + seedPlanRevisionTask(t, ctx, repo, "task-r3") + + if err := repo.InsertTaskPlanRevision(ctx, &models.TaskPlanRevision{ + TaskID: "task-r3", RevisionNumber: 1, AuthorKind: "agent", + }); err != nil { + t.Fatalf("first insert: %v", err) + } + err := repo.InsertTaskPlanRevision(ctx, &models.TaskPlanRevision{ + TaskID: "task-r3", RevisionNumber: 1, AuthorKind: "agent", + }) + if err == nil { + t.Fatal("expected UNIQUE violation on duplicate (task_id, revision_number)") + } +} + +func TestPlanRevisions_GetLatest(t *testing.T) { + repo, cleanup := createTestSQLiteRepo(t) + defer cleanup() + ctx := context.Background() + seedPlanRevisionTask(t, ctx, repo, "task-r4") + + latest, _ := repo.GetLatestTaskPlanRevision(ctx, "task-r4") + if latest != nil { + t.Errorf("expected nil before any insert, got %+v", latest) + } + + _ = repo.InsertTaskPlanRevision(ctx, &models.TaskPlanRevision{TaskID: "task-r4", RevisionNumber: 1, AuthorKind: "agent", Content: "a"}) + _ = repo.InsertTaskPlanRevision(ctx, &models.TaskPlanRevision{TaskID: "task-r4", RevisionNumber: 2, AuthorKind: "user", Content: "b"}) + + latest, err := repo.GetLatestTaskPlanRevision(ctx, "task-r4") + if err != nil { + t.Fatalf("latest: %v", err) + } + if latest == nil || latest.RevisionNumber != 2 || latest.Content != "b" { + t.Errorf("expected newest rev 2 with content 'b', got %+v", latest) + } +} + +func TestPlanRevisions_UpdateCoalesceMerge(t *testing.T) { + repo, cleanup := createTestSQLiteRepo(t) + defer cleanup() + ctx := context.Background() + seedPlanRevisionTask(t, ctx, repo, "task-r5") + + rev := &models.TaskPlanRevision{ + TaskID: "task-r5", RevisionNumber: 1, + Title: "Plan", Content: "v1", + AuthorKind: "agent", AuthorName: "Agent", + } + _ = repo.InsertTaskPlanRevision(ctx, rev) + originalID := rev.ID + + rev.Content = "v2" + if err := repo.UpdateTaskPlanRevision(ctx, rev); err != nil { + t.Fatalf("update: %v", err) + } + + got, _ := repo.GetTaskPlanRevision(ctx, originalID) + if got.Content != "v2" { + t.Errorf("expected merged content 'v2', got %q", got.Content) + } + if !got.UpdatedAt.After(got.CreatedAt) && !got.UpdatedAt.Equal(got.CreatedAt) { + t.Errorf("updated_at should be >= created_at") + } + + list, _ := repo.ListTaskPlanRevisions(ctx, "task-r5", 0) + if len(list) != 1 { + t.Errorf("coalesce merge should not add rows; have %d", len(list)) + } +} + +func TestPlanRevisions_RevertOfRevisionID(t *testing.T) { + repo, cleanup := createTestSQLiteRepo(t) + defer cleanup() + ctx := context.Background() + seedPlanRevisionTask(t, ctx, repo, "task-r6") + + target := &models.TaskPlanRevision{ + TaskID: "task-r6", RevisionNumber: 1, + Content: "original", AuthorKind: "agent", AuthorName: "Agent", + } + _ = repo.InsertTaskPlanRevision(ctx, target) + + refID := target.ID + revertRev := &models.TaskPlanRevision{ + TaskID: "task-r6", RevisionNumber: 2, + Content: "original", AuthorKind: "user", AuthorName: "User", + RevertOfRevisionID: &refID, + } + _ = repo.InsertTaskPlanRevision(ctx, revertRev) + + got, _ := repo.GetTaskPlanRevision(ctx, revertRev.ID) + if got.RevertOfRevisionID == nil || *got.RevertOfRevisionID != refID { + t.Errorf("expected revert_of_revision_id=%q, got %v", refID, got.RevertOfRevisionID) + } +} + +func TestPlanRevisions_BackfillsExistingPlan(t *testing.T) { + ctx := context.Background() + tmpDir := t.TempDir() + dbPath := filepath.Join(tmpDir, "backfill.db") + + // First open: seed a task + plan, then close (simulates pre-migration state plus + // a subsequent reopen — backfill runs once per newRepository call). + firstConn, err := db.OpenSQLite(dbPath) + if err != nil { + t.Fatalf("open 1: %v", err) + } + firstDB := sqlx.NewDb(firstConn, "sqlite3") + firstRepo, err := sqlite.NewWithDB(firstDB, firstDB) + if err != nil { + t.Fatalf("repo 1: %v", err) + } + seedPlanRevisionTask(t, ctx, firstRepo, "task-r7") + _ = firstRepo.CreateTaskPlan(ctx, &models.TaskPlan{ + TaskID: "task-r7", Title: "Legacy", Content: "legacy content", CreatedBy: "agent", + }) + // Remove the revision the plan creation would have left (if any) so backfill has real work. + _, _ = firstDB.Exec(`DELETE FROM task_plan_revisions WHERE task_id = ?`, "task-r7") + if err := firstDB.Close(); err != nil { + t.Fatalf("close 1: %v", err) + } + + // Second open: backfill should run during initSchema and synthesize a revision. + secondConn, err := db.OpenSQLite(dbPath) + if err != nil { + t.Fatalf("open 2: %v", err) + } + secondDB := sqlx.NewDb(secondConn, "sqlite3") + secondRepo, err := sqlite.NewWithDB(secondDB, secondDB) + if err != nil { + t.Fatalf("repo 2: %v", err) + } + defer func() { + if err := secondDB.Close(); err != nil { + t.Errorf("close 2: %v", err) + } + }() + + list, err := secondRepo.ListTaskPlanRevisions(ctx, "task-r7", 0) + if err != nil { + t.Fatalf("list: %v", err) + } + if len(list) != 1 { + t.Fatalf("expected 1 backfilled revision, got %d", len(list)) + } + if list[0].Content != "legacy content" { + t.Errorf("expected backfilled content 'legacy content', got %q", list[0].Content) + } + if list[0].AuthorName != "legacy" { + t.Errorf("expected author_name 'legacy', got %q", list[0].AuthorName) + } + if list[0].AuthorKind != "agent" { + t.Errorf("expected author_kind 'agent' (carried over from created_by), got %q", list[0].AuthorKind) + } + if list[0].RevisionNumber != 1 { + t.Errorf("expected revision_number=1, got %d", list[0].RevisionNumber) + } +} diff --git a/apps/backend/internal/task/repository/sqlite/base.go b/apps/backend/internal/task/repository/sqlite/base.go index 0d2c59e9c1..d66f6a2ebf 100644 --- a/apps/backend/internal/task/repository/sqlite/base.go +++ b/apps/backend/internal/task/repository/sqlite/base.go @@ -581,7 +581,7 @@ func (r *Repository) initCoreIndexes() error { } func (r *Repository) initPlansSchema() error { - _, err := r.db.Exec(` + if _, err := r.db.Exec(` CREATE TABLE IF NOT EXISTS task_plans ( id TEXT PRIMARY KEY, task_id TEXT NOT NULL UNIQUE, @@ -593,8 +593,85 @@ func (r *Repository) initPlansSchema() error { 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); - `) - return err + `); err != nil { + return err + } + if _, err := r.db.Exec(` + CREATE TABLE IF NOT EXISTS task_plan_revisions ( + id TEXT PRIMARY KEY, + task_id TEXT NOT NULL, + revision_number INTEGER NOT NULL, + title TEXT NOT NULL DEFAULT 'Plan', + content TEXT NOT NULL DEFAULT '', + author_kind TEXT NOT NULL DEFAULT 'agent', + author_name TEXT NOT NULL DEFAULT '', + revert_of_revision_id TEXT, + created_at TIMESTAMP NOT NULL, + updated_at TIMESTAMP NOT NULL, + FOREIGN KEY (task_id) REFERENCES tasks(id) ON DELETE CASCADE, + UNIQUE (task_id, revision_number) + ); + CREATE INDEX IF NOT EXISTS idx_task_plan_revisions_task_created + ON task_plan_revisions(task_id, created_at DESC); + -- Hot-path index: GetLatestTaskPlanRevision (called on every plan write + -- as part of the coalesce check), ListTaskPlanRevisions, and the + -- MAX(revision_number) lookup in WritePlanRevision all order/scan by + -- (task_id, revision_number DESC). With this index the latest-row lookup + -- is O(1) instead of an O(N) scan + sort per task. + CREATE INDEX IF NOT EXISTS idx_task_plan_revisions_task_number + ON task_plan_revisions(task_id, revision_number DESC); + `); err != nil { + return err + } + return r.backfillInitialPlanRevisions() +} + +// backfillInitialPlanRevisions ensures every existing task_plans row has at least +// one corresponding revision. Runs once at startup and is idempotent. +func (r *Repository) backfillInitialPlanRevisions() error { + rows, err := r.db.Query(` + SELECT p.id, p.task_id, p.title, p.content, p.created_by, p.created_at, p.updated_at + FROM task_plans p + WHERE NOT EXISTS ( + SELECT 1 FROM task_plan_revisions r WHERE r.task_id = p.task_id + )`) + if err != nil { + return fmt.Errorf("query plans missing revisions: %w", err) + } + defer func() { _ = rows.Close() }() + + type row struct { + id, taskID, title, content, createdBy string + createdAt, updatedAt interface{} + } + var pending []row + for rows.Next() { + var x row + if err := rows.Scan(&x.id, &x.taskID, &x.title, &x.content, &x.createdBy, &x.createdAt, &x.updatedAt); err != nil { + return fmt.Errorf("scan plan for backfill: %w", err) + } + pending = append(pending, x) + } + if err := rows.Err(); err != nil { + return fmt.Errorf("iterate plans for backfill: %w", err) + } + + for _, x := range pending { + authorKind := x.createdBy + // Match CreateTaskPlan (plan.go) and the task_plan_revisions column DEFAULT 'agent'. + if authorKind != "user" && authorKind != "agent" { + authorKind = "agent" + } + _, err := r.db.Exec(r.db.Rebind(` + INSERT INTO task_plan_revisions + (id, task_id, revision_number, title, content, author_kind, author_name, revert_of_revision_id, created_at, updated_at) + VALUES (?, ?, 1, ?, ?, ?, 'legacy', NULL, ?, ?) + `), uuid.New().String(), x.taskID, x.title, x.content, authorKind, x.createdAt, x.updatedAt) + if err != nil { + return fmt.Errorf("backfill revision for task %s: %w", x.taskID, err) + } + } + return nil } func (r *Repository) initSessionSchema() error { diff --git a/apps/backend/internal/task/repository/sqlite/plan.go b/apps/backend/internal/task/repository/sqlite/plan.go index c528081c4d..6e8edb43cc 100644 --- a/apps/backend/internal/task/repository/sqlite/plan.go +++ b/apps/backend/internal/task/repository/sqlite/plan.go @@ -7,9 +7,14 @@ import ( "time" "github.com/google/uuid" + "github.com/jmoiron/sqlx" "github.com/kandev/kandev/internal/task/models" ) +// revisionSelectCols lists the task_plan_revisions columns in the fixed order used by +// every SELECT in this file (and by scanRevisionRow / scanRevisionRows). +const revisionSelectCols = `id, task_id, revision_number, title, content, author_kind, author_name, revert_of_revision_id, created_at, updated_at` + // CreateTaskPlan creates a new task plan. func (r *Repository) CreateTaskPlan(ctx context.Context, plan *models.TaskPlan) error { if plan.ID == "" { @@ -82,3 +87,253 @@ func (r *Repository) DeleteTaskPlan(ctx context.Context, taskID string) error { } return nil } + +// Revision history + +// InsertTaskPlanRevision inserts a new revision row. +func (r *Repository) InsertTaskPlanRevision(ctx context.Context, rev *models.TaskPlanRevision) error { + if rev.ID == "" { + rev.ID = uuid.New().String() + } + now := time.Now().UTC() + if rev.CreatedAt.IsZero() { + rev.CreatedAt = now + } + rev.UpdatedAt = now + + _, err := r.db.ExecContext(ctx, r.db.Rebind(` + INSERT INTO task_plan_revisions + (id, task_id, revision_number, title, content, author_kind, author_name, revert_of_revision_id, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `), + rev.ID, rev.TaskID, rev.RevisionNumber, rev.Title, rev.Content, + rev.AuthorKind, rev.AuthorName, rev.RevertOfRevisionID, rev.CreatedAt, rev.UpdatedAt) + if err != nil { + return fmt.Errorf("failed to insert task plan revision: %w", err) + } + return nil +} + +// UpdateTaskPlanRevision updates title/content/updated_at on an existing revision (coalesce merge). +func (r *Repository) UpdateTaskPlanRevision(ctx context.Context, rev *models.TaskPlanRevision) error { + rev.UpdatedAt = time.Now().UTC() + result, err := r.db.ExecContext(ctx, r.db.Rebind(` + UPDATE task_plan_revisions + SET title = ?, content = ?, updated_at = ? + WHERE id = ? + `), rev.Title, rev.Content, rev.UpdatedAt, rev.ID) + if err != nil { + return fmt.Errorf("failed to update task plan revision: %w", err) + } + rows, _ := result.RowsAffected() + if rows == 0 { + return fmt.Errorf("task plan revision not found: %s", rev.ID) + } + return nil +} + +// GetTaskPlanRevision fetches a single revision by ID. +func (r *Repository) GetTaskPlanRevision(ctx context.Context, id string) (*models.TaskPlanRevision, error) { + return r.scanRevisionRow(r.ro.QueryRowContext(ctx, r.ro.Rebind( + `SELECT `+revisionSelectCols+` FROM task_plan_revisions WHERE id = ?`, + ), id)) +} + +// GetLatestTaskPlanRevision returns the newest revision for a task (by revision_number DESC). +func (r *Repository) GetLatestTaskPlanRevision(ctx context.Context, taskID string) (*models.TaskPlanRevision, error) { + return r.scanRevisionRow(r.ro.QueryRowContext(ctx, r.ro.Rebind( + `SELECT `+revisionSelectCols+` FROM task_plan_revisions WHERE task_id = ? ORDER BY revision_number DESC LIMIT 1`, + ), taskID)) +} + +// ListTaskPlanRevisions returns revisions newest-first. limit <= 0 returns all. +func (r *Repository) ListTaskPlanRevisions(ctx context.Context, taskID string, limit int) ([]*models.TaskPlanRevision, error) { + query := `SELECT ` + revisionSelectCols + ` FROM task_plan_revisions WHERE task_id = ? ORDER BY revision_number DESC` + args := []interface{}{taskID} + if limit > 0 { + query += ` LIMIT ?` + args = append(args, limit) + } + rows, err := r.ro.QueryContext(ctx, r.ro.Rebind(query), args...) + if err != nil { + return nil, fmt.Errorf("failed to list task plan revisions: %w", err) + } + defer func() { _ = rows.Close() }() + + var out []*models.TaskPlanRevision + for rows.Next() { + rev := &models.TaskPlanRevision{} + var revertOf sql.NullString + if err := rows.Scan( + &rev.ID, &rev.TaskID, &rev.RevisionNumber, &rev.Title, &rev.Content, + &rev.AuthorKind, &rev.AuthorName, &revertOf, &rev.CreatedAt, &rev.UpdatedAt, + ); err != nil { + return nil, fmt.Errorf("failed to scan task plan revision: %w", err) + } + if revertOf.Valid { + v := revertOf.String + rev.RevertOfRevisionID = &v + } + out = append(out, rev) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("iterate task plan revisions: %w", err) + } + return out, nil +} + +// NextTaskPlanRevisionNumber returns max(revision_number)+1 for a task, or 1 if none exist. +// +// Note: prefer WritePlanRevision when writing a new revision — it computes the next number +// atomically with the insert. This helper reads via the RO replica and is only safe against +// TOCTOU when followed by a write under a BEGIN IMMEDIATE-equivalent (e.g., within +// WritePlanRevision). Direct callers should be confined to read-only inspection / tests. +func (r *Repository) NextTaskPlanRevisionNumber(ctx context.Context, taskID string) (int, error) { + var maxNum sql.NullInt64 + err := r.ro.QueryRowContext(ctx, r.ro.Rebind(` + SELECT MAX(revision_number) FROM task_plan_revisions WHERE task_id = ? + `), taskID).Scan(&maxNum) + if err != nil { + return 0, fmt.Errorf("failed to get next revision number: %w", err) + } + if !maxNum.Valid { + return 1, nil + } + return int(maxNum.Int64) + 1, nil +} + +// WritePlanRevision atomically upserts HEAD (task_plans) and either appends a new revision +// or merges into an existing one in a single write transaction keyed by task_id. This closes +// the TOCTOU window on revision_number (MAX+1 is computed inside the tx) and prevents HEAD +// from disagreeing with history on partial failure. +// +// Coalesce behavior: when coalesceLatestID is non-nil and non-empty, the revision with that +// ID has title/content/updated_at merged in-place and its other fields (revision_number, +// author, created_at) are preserved. When nil or empty, a new revision row is inserted with +// revision_number = MAX(existing)+1 and populated from rev. +// +// On success, rev is mutated to reflect the persisted state (ID, RevisionNumber, CreatedAt, +// UpdatedAt). +func (r *Repository) WritePlanRevision( + ctx context.Context, + head *models.TaskPlan, + rev *models.TaskPlanRevision, + coalesceLatestID *string, +) error { + tx, err := r.db.BeginTxx(ctx, nil) + if err != nil { + return fmt.Errorf("begin plan revision tx: %w", err) + } + defer func() { _ = tx.Rollback() }() + + now := time.Now().UTC() + if err := upsertPlanHead(ctx, tx, r.db, head, now); err != nil { + return err + } + if coalesceLatestID != nil && *coalesceLatestID != "" { + if err := mergeRevisionInTx(ctx, tx, r.db, rev, *coalesceLatestID, now); err != nil { + return err + } + } else { + if err := insertNewRevisionInTx(ctx, tx, r.db, rev, now); err != nil { + return err + } + } + return tx.Commit() +} + +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() + } + if head.Title == "" { + head.Title = "Plan" + } + if head.CreatedBy == "" { + head.CreatedBy = "agent" + } + if head.CreatedAt.IsZero() { + head.CreatedAt = now + } + head.UpdatedAt = now + if _, err := tx.ExecContext(ctx, db.Rebind(` + INSERT INTO task_plans (id, task_id, title, content, created_by, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?) + ON CONFLICT(task_id) DO UPDATE SET + title = excluded.title, + content = excluded.content, + created_by = excluded.created_by, + updated_at = excluded.updated_at + `), head.ID, head.TaskID, head.Title, head.Content, head.CreatedBy, head.CreatedAt, head.UpdatedAt); err != nil { + return fmt.Errorf("upsert task plan head: %w", err) + } + return nil +} + +func mergeRevisionInTx(ctx context.Context, tx *sqlx.Tx, db *sqlx.DB, rev *models.TaskPlanRevision, latestID string, now time.Time) error { + result, err := tx.ExecContext(ctx, db.Rebind(` + UPDATE task_plan_revisions + SET title = ?, content = ?, updated_at = ? + WHERE id = ? + `), rev.Title, rev.Content, now, latestID) + if err != nil { + return fmt.Errorf("merge plan revision: %w", err) + } + rows, _ := result.RowsAffected() + if rows == 0 { + return fmt.Errorf("task plan revision not found: %s", latestID) + } + rev.ID = latestID + rev.UpdatedAt = now + return nil +} + +func insertNewRevisionInTx(ctx context.Context, tx *sqlx.Tx, db *sqlx.DB, rev *models.TaskPlanRevision, now time.Time) error { + var maxNum sql.NullInt64 + if err := tx.QueryRowContext(ctx, db.Rebind(` + SELECT MAX(revision_number) FROM task_plan_revisions WHERE task_id = ? + `), rev.TaskID).Scan(&maxNum); err != nil { + return fmt.Errorf("compute next revision number: %w", err) + } + rev.RevisionNumber = 1 + if maxNum.Valid { + rev.RevisionNumber = int(maxNum.Int64) + 1 + } + if rev.ID == "" { + rev.ID = uuid.New().String() + } + if rev.CreatedAt.IsZero() { + rev.CreatedAt = now + } + rev.UpdatedAt = now + if _, err := tx.ExecContext(ctx, db.Rebind(` + INSERT INTO task_plan_revisions + (`+revisionSelectCols+`) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `), + rev.ID, rev.TaskID, rev.RevisionNumber, rev.Title, rev.Content, + rev.AuthorKind, rev.AuthorName, rev.RevertOfRevisionID, rev.CreatedAt, rev.UpdatedAt); err != nil { + return fmt.Errorf("insert plan revision: %w", err) + } + return nil +} + +func (r *Repository) scanRevisionRow(row *sql.Row) (*models.TaskPlanRevision, error) { + rev := &models.TaskPlanRevision{} + var revertOf sql.NullString + err := row.Scan( + &rev.ID, &rev.TaskID, &rev.RevisionNumber, &rev.Title, &rev.Content, + &rev.AuthorKind, &rev.AuthorName, &revertOf, &rev.CreatedAt, &rev.UpdatedAt, + ) + if err == sql.ErrNoRows { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("failed to scan task plan revision: %w", err) + } + if revertOf.Valid { + v := revertOf.String + rev.RevertOfRevisionID = &v + } + return rev, nil +} diff --git a/apps/backend/internal/task/service/plan_service.go b/apps/backend/internal/task/service/plan_service.go index 5a14d9c504..6ed4933ede 100644 --- a/apps/backend/internal/task/service/plan_service.go +++ b/apps/backend/internal/task/service/plan_service.go @@ -3,6 +3,9 @@ package service import ( "context" "errors" + "os" + "strconv" + "time" "go.uber.org/zap" @@ -14,127 +17,103 @@ import ( ) var ( - ErrTaskPlanNotFound = errors.New("task plan not found") - ErrTaskIDRequired = errors.New("task_id is required") - ErrContentRequired = errors.New("content is required") + ErrTaskPlanNotFound = errors.New("task plan not found") + ErrTaskIDRequired = errors.New("task_id is required") + ErrContentRequired = errors.New("content 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") ) -// createdByAgent is the default creator for agent-created plans. -const createdByAgent = "agent" +const ( + createdByAgent = "agent" + createdByUser = "user" + defaultCoalesceWindow = 5 * time.Minute + coalesceWindowEnvVar = "KANDEV_PLAN_COALESCE_WINDOW_MS" + defaultAgentAuthorFallback = "Agent" + defaultUserAuthorFallback = "User" +) + +// planRepo is the repository surface this service depends on. It combines the +// plan-revision storage with a tiny slice of session lookups used to resolve +// the active session's agent profile name when the MCP path doesn't provide +// an explicit author_name. +type planRepo interface { + repository.PlanRepository + GetActiveTaskSessionByTaskID(ctx context.Context, taskID string) (*models.TaskSession, error) + GetTaskSessionByTaskID(ctx context.Context, taskID string) (*models.TaskSession, error) +} // PlanService provides task plan business logic. type PlanService struct { - repo repository.PlanRepository - eventBus bus.EventBus - logger *logger.Logger + repo planRepo + eventBus bus.EventBus + logger *logger.Logger + coalesceWindow time.Duration } -// NewPlanService creates a new task plan service. -func NewPlanService(repo repository.PlanRepository, eventBus bus.EventBus, log *logger.Logger) *PlanService { +// NewPlanService creates a new task plan service. The concrete repository +// passed by callers must implement both PlanRepository and the session-lookup +// methods on planRepo (the SQLite repository does both). +func NewPlanService(repo planRepo, eventBus bus.EventBus, log *logger.Logger) *PlanService { return &PlanService{ - repo: repo, - eventBus: eventBus, - logger: log.WithFields(zap.String("component", "plan-service")), + repo: repo, + eventBus: eventBus, + logger: log.WithFields(zap.String("component", "plan-service")), + coalesceWindow: readCoalesceWindow(), } } -// CreatePlanRequest contains parameters for creating a task plan. +func readCoalesceWindow() time.Duration { + raw := os.Getenv(coalesceWindowEnvVar) + if raw == "" { + return defaultCoalesceWindow + } + ms, err := strconv.Atoi(raw) + if err != nil || ms < 0 { + return defaultCoalesceWindow + } + return time.Duration(ms) * time.Millisecond +} + +// CreatePlanRequest contains parameters for creating/updating a task plan. +// AuthorKind and AuthorName are optional; when absent they are derived from CreatedBy. type CreatePlanRequest struct { - TaskID string - Title string - Content string - CreatedBy string // "agent" or "user" + TaskID string + Title string + Content string + CreatedBy string // "agent" | "user" + AuthorKind string // optional explicit override + AuthorName string // optional; display snapshot } -// CreatePlan creates a new task plan, or updates the existing one if a plan -// already exists for the given task (upsert semantics). +// CreatePlan upserts a plan and appends or coalesces a revision. func (s *PlanService) CreatePlan(ctx context.Context, req CreatePlanRequest) (*models.TaskPlan, error) { - if req.TaskID == "" { - return nil, ErrTaskIDRequired - } - - title := req.Title - if title == "" { - title = "Plan" - } - createdBy := req.CreatedBy - if createdBy == "" { - createdBy = createdByAgent - } - - // If a plan already exists for this task, update it instead of inserting. - existing, err := s.repo.GetTaskPlan(ctx, req.TaskID) - if err != nil { - s.logger.Error("failed to check existing task plan", zap.String("task_id", req.TaskID), zap.Error(err)) - return nil, err - } - if existing != nil { - return s.UpdatePlan(ctx, UpdatePlanRequest{ - TaskID: req.TaskID, - Title: title, - Content: req.Content, - CreatedBy: createdBy, - }) - } - - plan := &models.TaskPlan{ - TaskID: req.TaskID, - Title: title, - Content: req.Content, - CreatedBy: createdBy, - } - - if err := s.repo.CreateTaskPlan(ctx, plan); err != nil { - s.logger.Error("failed to create task plan", zap.String("task_id", req.TaskID), zap.Error(err)) - return nil, err - } - - s.logger.Info("created task plan", zap.String("task_id", req.TaskID), zap.String("plan_id", plan.ID)) - s.publishEvent(ctx, events.TaskPlanCreated, plan) - - return plan, nil + return s.upsertPlan(ctx, req) } -// GetPlan retrieves a task plan by task ID. -// Returns nil, nil if no plan exists. -func (s *PlanService) GetPlan(ctx context.Context, taskID string) (*models.TaskPlan, error) { - if taskID == "" { - return nil, ErrTaskIDRequired - } - - plan, err := s.repo.GetTaskPlan(ctx, taskID) - if err != nil { - s.logger.Error("failed to get task plan", zap.String("task_id", taskID), zap.Error(err)) - return nil, err - } - - return plan, nil -} - -// UpdatePlanRequest contains parameters for updating a task plan. +// UpdatePlanRequest mirrors CreatePlanRequest; kept as a distinct type for API clarity. type UpdatePlanRequest struct { - TaskID string - Title string // Optional: if empty, preserves existing title - Content string - CreatedBy string // Optional: if empty, preserves existing or defaults to "user" + TaskID string + Title string + Content string + CreatedBy string + AuthorKind string + AuthorName string } -// UpdatePlan updates an existing task plan. +// UpdatePlan updates an existing plan (errors if missing). func (s *PlanService) UpdatePlan(ctx context.Context, req UpdatePlanRequest) (*models.TaskPlan, error) { if req.TaskID == "" { return nil, ErrTaskIDRequired } - existing, err := s.repo.GetTaskPlan(ctx, req.TaskID) if err != nil { - s.logger.Error("failed to get existing task plan", zap.String("task_id", req.TaskID), zap.Error(err)) return nil, err } if existing == nil { return nil, ErrTaskPlanNotFound } - - // Preserve existing values if not provided title := req.Title if title == "" { title = existing.Title @@ -143,60 +122,297 @@ func (s *PlanService) UpdatePlan(ctx context.Context, req UpdatePlanRequest) (*m if createdBy == "" { createdBy = existing.CreatedBy } + return s.upsertPlan(ctx, CreatePlanRequest{ + TaskID: req.TaskID, + Title: title, + Content: req.Content, + CreatedBy: createdBy, + AuthorKind: req.AuthorKind, + AuthorName: req.AuthorName, + }) +} + +// upsertPlan is the shared write path. It upserts the task_plans HEAD row and either +// coalesces into the latest revision (same author within window) or appends a new revision +// — both steps run in one write transaction via WritePlanRevision so HEAD and history +// cannot diverge under concurrent writers or partial failures. +func (s *PlanService) upsertPlan(ctx context.Context, req CreatePlanRequest) (*models.TaskPlan, error) { + if req.TaskID == "" { + return nil, ErrTaskIDRequired + } + + title := req.Title + if title == "" { + title = "Plan" + } + // Resolve a missing AuthorName for agent writes from the active session's + // profile snapshot before falling back to the literal "Agent". The MCP path + // (handleCreateTaskPlan / handleUpdateTaskPlan) doesn't carry the agent's + // display name in the request, so without this lookup every agent revision + // would render as "Agent" in the history UI. + if req.AuthorName == "" { + kindHint := req.AuthorKind + if kindHint == "" { + kindHint = req.CreatedBy + } + if kindHint == createdByAgent { + req.AuthorName = s.resolveAgentDisplayName(ctx, req.TaskID) + } + } + authorKind, authorName, createdBy := resolveAuthor(req) + + existing, err := s.repo.GetTaskPlan(ctx, req.TaskID) + if err != nil { + s.logger.Error("get existing plan", zap.String("task_id", req.TaskID), zap.Error(err)) + return nil, err + } + eventType := events.TaskPlanCreated + if existing != nil { + eventType = events.TaskPlanUpdated + } plan := &models.TaskPlan{ - ID: existing.ID, TaskID: req.TaskID, Title: title, Content: req.Content, CreatedBy: createdBy, - CreatedAt: existing.CreatedAt, + } + if existing != nil { + plan.ID = existing.ID + plan.CreatedAt = existing.CreatedAt } - if err := s.repo.UpdateTaskPlan(ctx, plan); err != nil { - s.logger.Error("failed to update task plan", zap.String("task_id", req.TaskID), zap.Error(err)) + latest, err := s.repo.GetLatestTaskPlanRevision(ctx, req.TaskID) + if err != nil { + return nil, err + } + now := time.Now().UTC() + coalesce := s.canCoalesce(latest, authorKind, authorName, now) + + rev := &models.TaskPlanRevision{ + TaskID: req.TaskID, + Title: title, + Content: req.Content, + AuthorKind: authorKind, + AuthorName: authorName, + } + var coalesceID *string + if coalesce { + coalesceID = &latest.ID + // Preserve the original revision's author + number on merge. + rev.RevisionNumber = latest.RevisionNumber + rev.AuthorKind = latest.AuthorKind + rev.AuthorName = latest.AuthorName + rev.CreatedAt = latest.CreatedAt + } + + if err := s.repo.WritePlanRevision(ctx, plan, rev, coalesceID); err != nil { + s.logger.Error("write plan revision", zap.String("task_id", req.TaskID), zap.Error(err)) return nil, err } - s.logger.Info("updated task plan", zap.String("task_id", req.TaskID)) - s.publishEvent(ctx, events.TaskPlanUpdated, plan) - + s.publishPlanEvent(ctx, eventType, plan) + s.publishRevisionEvent(ctx, rev, coalesce) return plan, nil } -// DeletePlan deletes a task plan by task ID. +// resolveAgentDisplayName returns the agent profile's display name for the +// task's most recent session, or "" if no usable session/snapshot exists. +// Tries the active session first (running/starting/waiting) and falls back to +// the most recent session by started_at so plans written between turns still +// get the right author name. +func (s *PlanService) resolveAgentDisplayName(ctx context.Context, taskID string) string { + session, err := s.repo.GetActiveTaskSessionByTaskID(ctx, taskID) + if err != nil || session == nil { + session, err = s.repo.GetTaskSessionByTaskID(ctx, taskID) + if err != nil || session == nil { + return "" + } + } + return agentDisplayNameFromSnapshot(session.AgentProfileSnapshot) +} + +// agentDisplayNameFromSnapshot picks the best available display name from a +// session's agent_profile_snapshot. The orchestrator's canonical key is +// "name" (the profile's display name, e.g. "Claude Sonnet 4.5"); we try it +// first so a snapshot that carries both "name" and a stale older "label" +// doesn't render the stale value. Falls back through "label" (older paths) +// and "agent_display_name" (some DTO mappings) before giving up. +func agentDisplayNameFromSnapshot(snapshot map[string]interface{}) string { + if snapshot == nil { + return "" + } + for _, key := range []string{"name", "label", "agent_display_name"} { + if v, ok := snapshot[key]; ok { + if s, ok := v.(string); ok && s != "" { + return s + } + } + } + return "" +} + +func (s *PlanService) canCoalesce(latest *models.TaskPlanRevision, authorKind, authorName string, now time.Time) bool { + if latest == nil { + return false + } + if latest.RevertOfRevisionID != nil { + return false // revert markers are permanent + } + if latest.AuthorKind != authorKind || latest.AuthorName != authorName { + return false + } + if s.coalesceWindow <= 0 { + return false + } + return now.Sub(latest.UpdatedAt) < s.coalesceWindow +} + +// resolveAuthor derives the authoritative (kind, name, legacyCreatedBy) tuple +// from a write request. Callers may pass explicit AuthorKind/AuthorName; when +// absent we fall back to CreatedBy and a literal display name. +func resolveAuthor(req CreatePlanRequest) (kind, name, createdBy string) { + createdBy = req.CreatedBy + kind = req.AuthorKind + if kind == "" { + kind = createdBy + } + if kind != createdByAgent && kind != createdByUser { + kind = createdByAgent + } + if createdBy == "" { + createdBy = kind + } + name = req.AuthorName + if name == "" { + if kind == createdByAgent { + name = defaultAgentAuthorFallback + } else { + name = defaultUserAuthorFallback + } + } + return kind, name, createdBy +} + +// GetPlan retrieves a task plan by task ID. Returns nil, nil if missing. +func (s *PlanService) GetPlan(ctx context.Context, taskID string) (*models.TaskPlan, error) { + if taskID == "" { + return nil, ErrTaskIDRequired + } + return s.repo.GetTaskPlan(ctx, taskID) +} + +// 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 { if taskID == "" { return ErrTaskIDRequired } - - // Get plan before deleting for event payload existing, err := s.repo.GetTaskPlan(ctx, taskID) if err != nil { - s.logger.Error("failed to get task plan for delete", zap.String("task_id", taskID), zap.Error(err)) return err } if existing == nil { return ErrTaskPlanNotFound } - if err := s.repo.DeleteTaskPlan(ctx, taskID); err != nil { - s.logger.Error("failed to delete task plan", zap.String("task_id", taskID), zap.Error(err)) return err } - - s.logger.Info("deleted task plan", zap.String("task_id", taskID)) - s.publishEvent(ctx, events.TaskPlanDeleted, existing) - + s.publishPlanEvent(ctx, events.TaskPlanDeleted, existing) return nil } -// publishEvent publishes a task plan event to the event bus. -func (s *PlanService) publishEvent(ctx context.Context, eventType string, plan *models.TaskPlan) { +// ListRevisions returns plan revisions newest-first without content (metadata only). +func (s *PlanService) ListRevisions(ctx context.Context, taskID string) ([]*models.TaskPlanRevision, error) { + if taskID == "" { + return nil, ErrTaskIDRequired + } + return s.repo.ListTaskPlanRevisions(ctx, taskID, 0) +} + +// GetRevision returns a single revision with content (for diff/preview). +func (s *PlanService) GetRevision(ctx context.Context, id string) (*models.TaskPlanRevision, error) { + rev, err := s.repo.GetTaskPlanRevision(ctx, id) + if err != nil { + return nil, err + } + if rev == nil { + return nil, ErrRevisionNotFound + } + return rev, nil +} + +// RevertPlanRequest parameters for a revert-to-revision operation. +type RevertPlanRequest struct { + TaskID string + TargetRevisionID string + AuthorName string // user display name; "User" fallback when empty +} + +// RevertPlan creates a new revision whose content mirrors the target and updates HEAD, +// atomically via WritePlanRevision. Revert revisions are never coalesced (the "restored +// from vK" marker is preserved). +func (s *PlanService) RevertPlan(ctx context.Context, req RevertPlanRequest) (*models.TaskPlanRevision, error) { + if req.TaskID == "" { + return nil, ErrTaskIDRequired + } + if req.TargetRevisionID == "" { + return nil, ErrRevisionIDRequired + } + target, err := s.repo.GetTaskPlanRevision(ctx, req.TargetRevisionID) + if err != nil { + return nil, err + } + if target == nil { + return nil, ErrRevisionNotFound + } + if target.TaskID != req.TaskID { + return nil, ErrRevisionTaskMismatch + } + + authorName := req.AuthorName + if authorName == "" { + authorName = defaultUserAuthorFallback + } + + head, err := s.repo.GetTaskPlan(ctx, req.TaskID) + if err != nil { + return nil, err + } + plan := &models.TaskPlan{ + TaskID: req.TaskID, + Title: target.Title, + Content: target.Content, + CreatedBy: createdByUser, + } + if head != nil { + plan.ID = head.ID + plan.CreatedAt = head.CreatedAt + } + + targetID := target.ID + rev := &models.TaskPlanRevision{ + TaskID: req.TaskID, + Title: target.Title, + Content: target.Content, + AuthorKind: createdByUser, + AuthorName: authorName, + RevertOfRevisionID: &targetID, + } + if err := s.repo.WritePlanRevision(ctx, plan, rev, nil); err != nil { + return nil, err + } + + s.publishPlanEvent(ctx, events.TaskPlanUpdated, plan) + s.publishRevisionEvent(ctx, rev, false) + s.publishReverted(ctx, rev) + return rev, nil +} + +func (s *PlanService) publishPlanEvent(ctx context.Context, eventType string, plan *models.TaskPlan) { if s.eventBus == nil { return } - payload := map[string]interface{}{ "id": plan.ID, "task_id": plan.TaskID, @@ -206,8 +422,45 @@ func (s *PlanService) publishEvent(ctx context.Context, eventType string, plan * "created_at": plan.CreatedAt, "updated_at": plan.UpdatedAt, } - if err := s.eventBus.Publish(ctx, eventType, bus.NewEvent(eventType, "plan-service", payload)); err != nil { - s.logger.Error("failed to publish task plan event", zap.String("event_type", eventType), zap.Error(err)) + s.logger.Error("publish plan event", zap.String("event_type", eventType), zap.Error(err)) } } + +func (s *PlanService) publishRevisionEvent(ctx context.Context, rev *models.TaskPlanRevision, coalesced bool) { + if s.eventBus == nil { + return + } + payload := revisionPayload(rev) + payload["coalesced"] = coalesced + if err := s.eventBus.Publish(ctx, events.TaskPlanRevisionCreated, bus.NewEvent(events.TaskPlanRevisionCreated, "plan-service", payload)); err != nil { + s.logger.Error("publish revision event", zap.Error(err)) + } +} + +func (s *PlanService) publishReverted(ctx context.Context, rev *models.TaskPlanRevision) { + if s.eventBus == nil { + return + } + payload := revisionPayload(rev) + if err := s.eventBus.Publish(ctx, events.TaskPlanReverted, bus.NewEvent(events.TaskPlanReverted, "plan-service", payload)); err != nil { + s.logger.Error("publish reverted event", zap.Error(err)) + } +} + +func revisionPayload(rev *models.TaskPlanRevision) map[string]interface{} { + p := map[string]interface{}{ + "id": rev.ID, + "task_id": rev.TaskID, + "revision_number": rev.RevisionNumber, + "title": rev.Title, + "author_kind": rev.AuthorKind, + "author_name": rev.AuthorName, + "created_at": rev.CreatedAt, + "updated_at": rev.UpdatedAt, + } + if rev.RevertOfRevisionID != nil { + p["revert_of_revision_id"] = *rev.RevertOfRevisionID + } + return p +} diff --git a/apps/backend/internal/task/service/plan_service_test.go b/apps/backend/internal/task/service/plan_service_test.go index 9f4838de4f..ba81e3db57 100644 --- a/apps/backend/internal/task/service/plan_service_test.go +++ b/apps/backend/internal/task/service/plan_service_test.go @@ -192,3 +192,259 @@ func TestPlanService_DeletePlanNotFound(t *testing.T) { t.Errorf("expected ErrTaskPlanNotFound, got %v", err) } } + +func TestPlanService_CreatesInitialRevision(t *testing.T) { + svc, _, repo := createTestPlanService(t) + ctx := context.Background() + seedTask(t, ctx, repo, "task-rev") + + _, err := svc.CreatePlan(ctx, CreatePlanRequest{ + TaskID: "task-rev", Content: "v1", + AuthorKind: "agent", AuthorName: "Claude", + }) + if err != nil { + t.Fatalf("create: %v", err) + } + + list, _ := svc.ListRevisions(ctx, "task-rev") + if len(list) != 1 { + t.Fatalf("expected 1 revision, got %d", len(list)) + } + if list[0].AuthorName != "Claude" || list[0].AuthorKind != "agent" { + t.Errorf("unexpected author: %+v", list[0]) + } + if list[0].RevisionNumber != 1 { + t.Errorf("expected rev #1, got %d", list[0].RevisionNumber) + } +} + +func TestPlanService_CoalescesWithinWindow(t *testing.T) { + svc, _, repo := createTestPlanService(t) + ctx := context.Background() + seedTask(t, ctx, repo, "task-co") + svc.coalesceWindow = 10 * time.Minute // force generous window + + _, _ = svc.CreatePlan(ctx, CreatePlanRequest{ + TaskID: "task-co", Content: "v1", + AuthorKind: "agent", AuthorName: "Claude", + }) + _, _ = svc.CreatePlan(ctx, CreatePlanRequest{ + TaskID: "task-co", Content: "v2", + AuthorKind: "agent", AuthorName: "Claude", + }) + + list, _ := svc.ListRevisions(ctx, "task-co") + if len(list) != 1 { + t.Fatalf("expected coalesced to 1, got %d", len(list)) + } + if list[0].Content != "v2" { + t.Errorf("expected merged content v2, got %q", list[0].Content) + } +} + +func TestPlanService_AppendsWhenWindowExpired(t *testing.T) { + svc, _, repo := createTestPlanService(t) + ctx := context.Background() + seedTask(t, ctx, repo, "task-win") + svc.coalesceWindow = 0 // disable coalescing + + _, _ = svc.CreatePlan(ctx, CreatePlanRequest{ + TaskID: "task-win", Content: "v1", + AuthorKind: "agent", AuthorName: "Claude", + }) + _, _ = svc.CreatePlan(ctx, CreatePlanRequest{ + TaskID: "task-win", Content: "v2", + AuthorKind: "agent", AuthorName: "Claude", + }) + + list, _ := svc.ListRevisions(ctx, "task-win") + if len(list) != 2 { + t.Fatalf("expected 2 separate revisions, got %d", len(list)) + } +} + +func TestPlanService_AuthorSwitchBreaksCoalesce(t *testing.T) { + svc, _, repo := createTestPlanService(t) + ctx := context.Background() + seedTask(t, ctx, repo, "task-sw") + svc.coalesceWindow = 10 * time.Minute + + _, _ = svc.CreatePlan(ctx, CreatePlanRequest{ + TaskID: "task-sw", Content: "agent-wrote", + AuthorKind: "agent", AuthorName: "Claude", + }) + _, _ = svc.CreatePlan(ctx, CreatePlanRequest{ + TaskID: "task-sw", Content: "user-edited", + AuthorKind: "user", AuthorName: "Alice", + }) + + list, _ := svc.ListRevisions(ctx, "task-sw") + if len(list) != 2 { + t.Fatalf("expected 2 revisions (author switch breaks coalesce), got %d", len(list)) + } + if list[0].AuthorKind != "user" || list[1].AuthorKind != "agent" { + t.Errorf("unexpected order: [%s, %s]", list[0].AuthorKind, list[1].AuthorKind) + } +} + +func TestPlanService_RevertToEarlierRevision(t *testing.T) { + svc, _, repo := createTestPlanService(t) + ctx := context.Background() + seedTask(t, ctx, repo, "task-rv") + svc.coalesceWindow = 0 + + _, _ = svc.CreatePlan(ctx, CreatePlanRequest{TaskID: "task-rv", Content: "v1", AuthorKind: "agent", AuthorName: "Claude"}) + _, _ = svc.CreatePlan(ctx, CreatePlanRequest{TaskID: "task-rv", Content: "v2", AuthorKind: "agent", AuthorName: "Claude"}) + _, _ = svc.CreatePlan(ctx, CreatePlanRequest{TaskID: "task-rv", Content: "v3", AuthorKind: "agent", AuthorName: "Claude"}) + + list, _ := svc.ListRevisions(ctx, "task-rv") + if len(list) != 3 { + t.Fatalf("expected 3 before revert, got %d", len(list)) + } + v1 := list[2] + + revert, err := svc.RevertPlan(ctx, RevertPlanRequest{ + TaskID: "task-rv", TargetRevisionID: v1.ID, AuthorName: "Alice", + }) + if err != nil { + t.Fatalf("revert: %v", err) + } + if revert.RevertOfRevisionID == nil || *revert.RevertOfRevisionID != v1.ID { + t.Errorf("revert_of_revision_id mismatch: %v", revert.RevertOfRevisionID) + } + if revert.AuthorKind != "user" || revert.AuthorName != "Alice" { + t.Errorf("expected user/Alice, got %s/%s", revert.AuthorKind, revert.AuthorName) + } + if revert.RevisionNumber != 4 { + t.Errorf("expected rev #4, got %d", revert.RevisionNumber) + } + + head, _ := svc.GetPlan(ctx, "task-rv") + if head.Content != "v1" { + t.Errorf("expected HEAD content v1, got %q", head.Content) + } +} + +func TestPlanService_RevertNeverCoalesces(t *testing.T) { + svc, _, repo := createTestPlanService(t) + ctx := context.Background() + seedTask(t, ctx, repo, "task-rvc") + svc.coalesceWindow = 10 * time.Minute + + _, _ = svc.CreatePlan(ctx, CreatePlanRequest{TaskID: "task-rvc", Content: "v1", AuthorKind: "agent", AuthorName: "Claude"}) + list, _ := svc.ListRevisions(ctx, "task-rvc") + v1 := list[0] + + // Two reverts by the same user in quick succession must remain separate rows. + _, _ = svc.RevertPlan(ctx, RevertPlanRequest{TaskID: "task-rvc", TargetRevisionID: v1.ID, AuthorName: "Alice"}) + _, _ = svc.RevertPlan(ctx, RevertPlanRequest{TaskID: "task-rvc", TargetRevisionID: v1.ID, AuthorName: "Alice"}) + + list, _ = svc.ListRevisions(ctx, "task-rvc") + if len(list) != 3 { + t.Fatalf("expected 3 revisions (1 original + 2 reverts), got %d", len(list)) + } +} + +func TestPlanService_RevertRejectsWrongTask(t *testing.T) { + svc, _, repo := createTestPlanService(t) + ctx := context.Background() + seedTask(t, ctx, repo, "task-x") + _ = repo.CreateTask(ctx, &models.Task{ + ID: "task-y", WorkspaceID: "ws-plan", WorkflowID: "wf-plan", Title: "Y", + State: v1.TaskStateCreated, CreatedAt: time.Now().UTC(), UpdatedAt: time.Now().UTC(), + }) + + _, _ = svc.CreatePlan(ctx, CreatePlanRequest{TaskID: "task-x", Content: "x", AuthorKind: "agent"}) + xList, _ := svc.ListRevisions(ctx, "task-x") + xRev := xList[0] + + _, err := svc.RevertPlan(ctx, RevertPlanRequest{ + TaskID: "task-y", TargetRevisionID: xRev.ID, AuthorName: "Alice", + }) + if err != ErrRevisionTaskMismatch { + t.Errorf("expected ErrRevisionTaskMismatch, got %v", err) + } +} + +func TestPlanService_AgentAuthorNameFromSession(t *testing.T) { + svc, _, repo := createTestPlanService(t) + ctx := context.Background() + seedTask(t, ctx, repo, "task-an") + + // Seed an active session with an agent profile snapshot. The MCP path + // resolves the agent's display name from this snapshot when the request + // doesn't carry an explicit author_name. + now := time.Now().UTC() + if err := repo.CreateTaskSession(ctx, &models.TaskSession{ + ID: "sess-an", + TaskID: "task-an", + AgentExecutionID: "exec-an", + AgentProfileID: "ap-claude", + AgentProfileSnapshot: map[string]interface{}{ + "id": "ap-claude", + "name": "Claude Sonnet 4.5", + "agent_id": "claude", + "agent_name": "claude", + }, + State: models.TaskSessionState("RUNNING"), + StartedAt: now, + UpdatedAt: now, + }); err != nil { + t.Fatalf("CreateTaskSession failed: %v", err) + } + + // MCP path: created_by=agent, no author_name provided. + _, err := svc.CreatePlan(ctx, CreatePlanRequest{ + TaskID: "task-an", + Content: "first draft", + CreatedBy: "agent", + }) + if err != nil { + t.Fatalf("CreatePlan failed: %v", err) + } + + list, _ := svc.ListRevisions(ctx, "task-an") + if len(list) != 1 { + t.Fatalf("expected 1 revision, got %d", len(list)) + } + if list[0].AuthorKind != "agent" { + t.Errorf("expected author_kind=agent, got %q", list[0].AuthorKind) + } + if list[0].AuthorName != "Claude Sonnet 4.5" { + t.Errorf("expected author_name resolved from session snapshot, got %q", list[0].AuthorName) + } +} + +func TestPlanService_AgentAuthorNameFallsBackWhenNoSession(t *testing.T) { + svc, _, repo := createTestPlanService(t) + ctx := context.Background() + seedTask(t, ctx, repo, "task-an2") + + // No session seeded → resolution returns "" → resolveAuthor falls back to "Agent". + _, err := svc.CreatePlan(ctx, CreatePlanRequest{ + TaskID: "task-an2", + Content: "first draft", + CreatedBy: "agent", + }) + if err != nil { + t.Fatalf("CreatePlan failed: %v", err) + } + + list, _ := svc.ListRevisions(ctx, "task-an2") + if list[0].AuthorName != defaultAgentAuthorFallback { + t.Errorf("expected fallback %q, got %q", defaultAgentAuthorFallback, list[0].AuthorName) + } +} + +func TestPlanService_RevertMissingRevision(t *testing.T) { + svc, _, repo := createTestPlanService(t) + ctx := context.Background() + seedTask(t, ctx, repo, "task-mr") + + _, err := svc.RevertPlan(ctx, RevertPlanRequest{ + TaskID: "task-mr", TargetRevisionID: "does-not-exist", AuthorName: "Alice", + }) + if err != ErrRevisionNotFound { + t.Errorf("expected ErrRevisionNotFound, got %v", err) + } +} diff --git a/apps/backend/pkg/websocket/actions.go b/apps/backend/pkg/websocket/actions.go index 09651c48db..d17acc0c9e 100644 --- a/apps/backend/pkg/websocket/actions.go +++ b/apps/backend/pkg/websocket/actions.go @@ -57,18 +57,21 @@ const ( ActionEnvironmentDelete = "environment.delete" // Task actions - ActionTaskList = "task.list" - ActionTaskCreate = "task.create" - ActionTaskGet = "task.get" - ActionTaskUpdate = "task.update" - ActionTaskDelete = "task.delete" - ActionTaskMove = "task.move" - ActionTaskState = "task.state" - ActionTaskArchive = "task.archive" - ActionTaskPlanCreate = "task.plan.create" - ActionTaskPlanGet = "task.plan.get" - ActionTaskPlanUpdate = "task.plan.update" - ActionTaskPlanDelete = "task.plan.delete" + ActionTaskList = "task.list" + ActionTaskCreate = "task.create" + ActionTaskGet = "task.get" + ActionTaskUpdate = "task.update" + ActionTaskDelete = "task.delete" + ActionTaskMove = "task.move" + ActionTaskState = "task.state" + ActionTaskArchive = "task.archive" + ActionTaskPlanCreate = "task.plan.create" + ActionTaskPlanGet = "task.plan.get" + ActionTaskPlanUpdate = "task.plan.update" + ActionTaskPlanDelete = "task.plan.delete" + ActionTaskPlanRevisionsList = "task.plan.revisions.list" + ActionTaskPlanRevisionGet = "task.plan.revision.get" + ActionTaskPlanRevert = "task.plan.revert" ActionTaskSessionList = "task.session.list" ActionTaskSessionResume = "task.session.resume" @@ -159,6 +162,8 @@ const ( ActionTaskPlanCreated = "task.plan.created" ActionTaskPlanUpdated = "task.plan.updated" ActionTaskPlanDeleted = "task.plan.deleted" + ActionTaskPlanRevisionCreated = "task.plan.revision.created" + ActionTaskPlanReverted = "task.plan.reverted" ActionAgentUpdated = "agent.updated" ActionAgentAvailableUpdated = "agent.available.updated" ActionWorkspaceCreated = "workspace.created" diff --git a/apps/web/components/editors/tiptap/tiptap-plan-readonly.tsx b/apps/web/components/editors/tiptap/tiptap-plan-readonly.tsx new file mode 100644 index 0000000000..e53f8b37cc --- /dev/null +++ b/apps/web/components/editors/tiptap/tiptap-plan-readonly.tsx @@ -0,0 +1,87 @@ +"use client"; + +import { useEffect } from "react"; +import { useTheme } from "next-themes"; +import { useEditor, EditorContent } from "@tiptap/react"; +import StarterKit from "@tiptap/starter-kit"; +import Highlight from "@tiptap/extension-highlight"; +import Underline from "@tiptap/extension-underline"; +import Link from "@tiptap/extension-link"; +import TaskList from "@tiptap/extension-task-list"; +import TaskItem from "@tiptap/extension-task-item"; +import { Table } from "@tiptap/extension-table"; +import { TableRow } from "@tiptap/extension-table-row"; +import { TableCell } from "@tiptap/extension-table-cell"; +import { TableHeader } from "@tiptap/extension-table-header"; +import { Markdown } from "tiptap-markdown"; +import { common, createLowlight } from "lowlight"; +import { createCodeBlockWithMermaid } from "./tiptap-mermaid-extension"; +import { cn } from "@/lib/utils"; + +type Props = { + /** Markdown source. The editor's tiptap-markdown extension parses it via + * `setContent`, so headings, lists, tables, and code blocks render properly. */ + content: string; + className?: string; + testId?: string; +}; + +const lowlight = createLowlight(common); + +/** Read-only Tiptap renderer for plan revision content. + * + * Reuses the same StarterKit + Markdown + tables/tasks/code-block extensions as + * the live plan editor so previewed markdown matches what the user sees while + * editing — minus interactive bits (slash menu, drag handles, comment marks). + * + * Initial markdown content is set via `editor.commands.setContent(content, ...)` + * after creation rather than the `content` option, because tiptap-markdown's + * setContent override is what triggers the markdown -> doc parse. + */ +export function PlanReadOnlyMarkdown({ content, className, testId }: Props) { + const { resolvedTheme } = useTheme(); + const editor = useEditor({ + immediatelyRender: false, + editable: false, + extensions: [ + StarterKit.configure({ codeBlock: false }), + createCodeBlockWithMermaid(lowlight), + Markdown.configure({ html: true, transformPastedText: false, transformCopiedText: false }), + Link.configure({ openOnClick: false }), + Highlight, + Underline, + TaskList, + TaskItem.configure({ nested: true }), + Table.configure({ resizable: false }), + TableRow, + TableCell, + TableHeader, + ], + }); + + useEffect(() => { + if (!editor || editor.isDestroyed) return; + // Tiptap-markdown intercepts setContent for markdown strings; setting + // content this way (instead of via the `content` option) ensures the + // input gets parsed as markdown rather than treated as raw text/HTML. + editor.commands.setContent(content, { emitUpdate: false }); + }, [editor, content]); + + // Reuse the same class names as the live plan editor so the heading sizes, + // list spacing, code-block styling, etc. defined in `globals.css` under + // `.markdown-body` and `.tiptap-plan-wrapper` apply identically here. + return ( + + ); +} diff --git a/apps/web/components/task/task-plan-diff-dialog.tsx b/apps/web/components/task/task-plan-diff-dialog.tsx new file mode 100644 index 0000000000..58eef52ccb --- /dev/null +++ b/apps/web/components/task/task-plan-diff-dialog.tsx @@ -0,0 +1,447 @@ +"use client"; + +import { useEffect, useMemo, useRef, useState, type ReactNode } from "react"; +import { IconLoader2 } from "@tabler/icons-react"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@kandev/ui/dialog"; +import { Button } from "@kandev/ui/button"; +import { ToggleGroup, ToggleGroupItem } from "@kandev/ui/toggle-group"; +import { cn } from "@/lib/utils"; +import type { TaskPlanRevision } from "@/lib/types/http"; +import { lineDiff, diffSummary, type DiffLine, type DiffLineKind } from "./task-plan-diff"; + +type Props = { + /** Revision pair in arbitrary user-pick order; the dialog re-orders them by + * revision_number so the older one is always the "before" side. */ + pair: [TaskPlanRevision | null, TaskPlanRevision | null]; + loadContent: (revisionId: string) => Promise; + onClose: () => void; + /** Restore the older revision (the "before" side) — null if the pair isn't + * fully populated. */ + onRestoreOlder: (revisionId: string) => void; +}; + +type DiffMode = "unified" | "split"; + +const KIND_CLASS: Record = { + add: "bg-emerald-500/10 border-l-2 border-emerald-500/60", + remove: "bg-rose-500/10 border-l-2 border-rose-500/60", + context: "border-l-2 border-transparent", +}; + +const KIND_PREFIX: Record = { + add: "+", + remove: "-", + context: " ", +}; + +export function PlanRevisionDiffDialog({ + pair, + loadContent, + onClose, + onRestoreOlder, +}: Props): ReactNode { + const [before, after] = useMemo(() => orderPair(pair), [pair]); + const [mode, setMode] = useState("unified"); + const open = before !== null && after !== null; + const sameRevision = before !== null && after !== null && before.id === after.id; + const title = + before && after ? `Compare v${before.revision_number} → v${after.revision_number}` : "Compare"; + + return ( + { + if (!v) onClose(); + }} + > + + + {title} + + Line-level diff between the two selected versions. + + + + + + {before && !sameRevision && ( + + )} + + + + ); +} + +function DiffBody({ + before, + after, + loadContent, + mode, + setMode, +}: { + before: TaskPlanRevision | null; + after: TaskPlanRevision | null; + loadContent: (revisionId: string) => Promise; + mode: DiffMode; + setMode: (m: DiffMode) => void; +}) { + const { beforeContent, afterContent, lines, error } = useDiffContent(before, after, loadContent); + const summary = lines ? diffSummary(lines) : null; + const sameRevision = before !== null && after !== null && before.id === after.id; + return ( +
+
+
+ {summary ? `${summary.added} added · ${summary.removed} removed` : "Loading…"} +
+ { + if (v === "unified" || v === "split") setMode(v); + }} + data-testid="plan-revision-diff-mode-toggle" + > + + Unified + + + Split + + +
+
+ +
+
+ ); +} + +function DiffBodyInner({ + mode, + lines, + beforeContent, + afterContent, + error, + sameRevision, + summary, +}: { + mode: DiffMode; + lines: DiffLine[] | null; + beforeContent: string | null; + afterContent: string | null; + error: string | null; + sameRevision: boolean; + summary: { added: number; removed: number } | null; +}) { + if (error) return
{error}
; + if (lines === null) return ; + if (sameRevision) + return These are the same version; nothing to compare.; + if (lines.length === 0) return (both versions are empty); + if (summary && summary.added === 0 && summary.removed === 0) { + return No textual changes between these versions.; + } + if (mode === "split") { + return ; + } + return ; +} + +function UnifiedDiff({ lines }: { lines: DiffLine[] }) { + return ( + // inline-block + min-w-full lets the row backgrounds extend past the + // viewport so highlights stay solid when the user scrolls horizontally. +
    + {lines.map((line, i) => ( + + ))} +
+ ); +} + +function DiffLineRow({ line }: { line: DiffLine }) { + return ( +
  • + + {KIND_PREFIX[line.kind]} + + {line.text || " "} +
    • + ); +} + +/** Side-by-side diff with two equal-width panes that scroll horizontally + * together. The dialog stays at its fixed width; each pane shows up to half + * of it and gets its own horizontal scrollbar for long lines. A scroll + * listener keeps both panes' `scrollLeft` in lockstep so corresponding rows + * stay aligned visually. */ +function SplitDiff({ + beforeContent, + afterContent, +}: { + beforeContent: string; + afterContent: string; +}) { + const rows = useMemo( + () => alignSplit(beforeContent, afterContent), + [beforeContent, afterContent], + ); + const beforeRef = useRef(null); + const afterRef = useRef(null); + // Re-entrancy guard: when we set the partner pane's scrollLeft, that fires + // the partner's onScroll, which would bounce back here without this flag. + const syncingRef = useRef(false); + + useEffect(() => { + const before = beforeRef.current; + const after = afterRef.current; + if (!before || !after) return; + const sync = (source: HTMLDivElement, target: HTMLDivElement) => () => { + if (syncingRef.current) return; + syncingRef.current = true; + target.scrollLeft = source.scrollLeft; + // Release on the next frame so the bounced onScroll has fired and + // returned early, but we don't permanently disable syncing. + requestAnimationFrame(() => { + syncingRef.current = false; + }); + }; + const onBefore = sync(before, after); + const onAfter = sync(after, before); + before.addEventListener("scroll", onBefore, { passive: true }); + after.addEventListener("scroll", onAfter, { passive: true }); + return () => { + before.removeEventListener("scroll", onBefore); + after.removeEventListener("scroll", onAfter); + }; + }, [rows]); + + return ( +
    + + +
    + ); +} + +const SplitPane = ({ + ref, + rows, + side, + testIdSuffix, +}: { + ref: React.RefObject; + rows: SplitRow[]; + side: "before" | "after"; + testIdSuffix: string; +}) => ( +
    + {/* w-max lets each pane's intrinsic width be the longest line on that + * side; combined with the parent grid-cols-2, the visible portion is + * still capped at 50% of the dialog while horizontal scroll reveals + * the rest. */} +
    + {rows.map((row, i) => ( + + ))} +
    +
    +); + +function SplitCell({ side, line }: { side: "before" | "after"; line: DiffLine | null }) { + // Empty cells (a removal on the after side, or an addition on the before + // side) get a subtle striped background so the row alignment is visible. + if (!line) { + return ( +
    + ); + } + return ( +
    + + {KIND_PREFIX[line.kind]} + + {line.text || " "} +
    + ); +} + +function DiffLoading() { + return ( +
    + + Loading… +
    + ); +} + +function DiffMessage({ children }: { children: ReactNode }) { + return
    {children}
    ; +} + +/** Sort a 2-slot pair by revision_number ascending so the older revision is + * always the "before" side, regardless of pick order. */ +function orderPair( + pair: [TaskPlanRevision | null, TaskPlanRevision | null], +): [TaskPlanRevision | null, TaskPlanRevision | null] { + const [a, b] = pair; + if (!a || !b) return pair; + return a.revision_number <= b.revision_number ? [a, b] : [b, a]; +} + +type SplitRow = { before: DiffLine | null; after: DiffLine | null }; + +/** Align the line-diff into side-by-side rows: each row contains at most one + * before-line and one after-line. Removals render on the left only; adds on + * the right only; context lines align across both sides. Adjacent + * remove/add chunks are zipped together so a single-line edit shows up on the + * same row. */ +function alignSplit(beforeContent: string, afterContent: string): SplitRow[] { + const lines = lineDiff(beforeContent, afterContent); + const rows: SplitRow[] = []; + let i = 0; + while (i < lines.length) { + const line = lines[i]; + if (line.kind === "context") { + rows.push({ before: line, after: line }); + i++; + continue; + } + // Collect a contiguous remove block, then any adjacent add block. + const removes: DiffLine[] = []; + while (i < lines.length && lines[i].kind === "remove") { + removes.push(lines[i]); + i++; + } + const adds: DiffLine[] = []; + while (i < lines.length && lines[i].kind === "add") { + adds.push(lines[i]); + i++; + } + const max = Math.max(removes.length, adds.length); + for (let k = 0; k < max; k++) { + rows.push({ before: removes[k] ?? null, after: adds[k] ?? null }); + } + } + return rows; +} + +function useDiffContent( + before: TaskPlanRevision | null, + after: TaskPlanRevision | null, + loadContent: (revisionId: string) => Promise, +): { + beforeContent: string | null; + afterContent: string | null; + lines: DiffLine[] | null; + error: string | null; +} { + const [beforeContent, setBeforeContent] = useState(null); + const [afterContent, setAfterContent] = useState(null); + const [error, setError] = useState(null); + + useEffect(() => { + if (!before || !after) return; + let cancelled = false; + Promise.all([loadContent(before.id), loadContent(after.id)]) + .then(([b, a]) => { + if (cancelled) return; + setBeforeContent(b); + setAfterContent(a); + }) + .catch((err) => { + if (cancelled) return; + setError(err instanceof Error ? err.message : "Failed to load revision content"); + }); + return () => { + cancelled = true; + }; + }, [before, after, loadContent]); + + const lines = useMemo(() => { + if (beforeContent === null || afterContent === null) return null; + return lineDiff(beforeContent, afterContent); + }, [beforeContent, afterContent]); + return { beforeContent, afterContent, lines, error }; +} diff --git a/apps/web/components/task/task-plan-diff.test.ts b/apps/web/components/task/task-plan-diff.test.ts new file mode 100644 index 0000000000..9b7afb8a9e --- /dev/null +++ b/apps/web/components/task/task-plan-diff.test.ts @@ -0,0 +1,69 @@ +import { describe, it, expect } from "vitest"; +import { lineDiff, diffSummary } from "./task-plan-diff"; + +describe("lineDiff", () => { + it("returns all-context for identical strings", () => { + const result = lineDiff("a\nb\nc", "a\nb\nc"); + expect(result.map((l) => l.kind)).toEqual(["context", "context", "context"]); + expect(result.map((l) => l.text)).toEqual(["a", "b", "c"]); + }); + + it("marks added lines", () => { + const result = lineDiff("a\n", "a\nb\n"); + expect(result).toEqual([ + { kind: "context", text: "a" }, + { kind: "add", text: "b" }, + ]); + }); + + it("marks removed lines", () => { + const result = lineDiff("a\nb\n", "a\n"); + expect(result).toEqual([ + { kind: "context", text: "a" }, + { kind: "remove", text: "b" }, + ]); + }); + + it("handles a mixed diff", () => { + const result = lineDiff("a\nb\nc\n", "a\nB\nc\n"); + expect(result).toEqual([ + { kind: "context", text: "a" }, + { kind: "remove", text: "b" }, + { kind: "add", text: "B" }, + { kind: "context", text: "c" }, + ]); + }); + + it("empty before", () => { + const result = lineDiff("", "x\ny\n"); + expect(result.map((l) => l.kind)).toEqual(["add", "add"]); + expect(result.map((l) => l.text)).toEqual(["x", "y"]); + }); + + it("empty after", () => { + const result = lineDiff("x\ny\n", ""); + expect(result.map((l) => l.kind)).toEqual(["remove", "remove"]); + }); + + it("does not emit a trailing empty line", () => { + const result = lineDiff("a\n", "a\n"); + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ kind: "context", text: "a" }); + }); +}); + +describe("diffSummary", () => { + it("counts added and removed", () => { + const result = diffSummary([ + { kind: "context", text: "a" }, + { kind: "add", text: "b" }, + { kind: "add", text: "c" }, + { kind: "remove", text: "d" }, + ]); + expect(result).toEqual({ added: 2, removed: 1 }); + }); + + it("zero on no changes", () => { + expect(diffSummary([{ kind: "context", text: "a" }])).toEqual({ added: 0, removed: 0 }); + }); +}); diff --git a/apps/web/components/task/task-plan-diff.ts b/apps/web/components/task/task-plan-diff.ts new file mode 100644 index 0000000000..dc9c101602 --- /dev/null +++ b/apps/web/components/task/task-plan-diff.ts @@ -0,0 +1,51 @@ +import { diffLines } from "diff"; + +export type DiffLineKind = "add" | "remove" | "context"; + +export type DiffLine = { + kind: DiffLineKind; + /** Single line of text without the trailing newline. */ + text: string; +}; + +/** + * Line-level diff of two strings in unified order. Each input line maps to one + * `DiffLine`; identical regions are emitted as `context`, lines present only in + * `before` as `remove`, and lines present only in `after` as `add`. + * + * Trailing newlines are normalized away so a final blank line doesn't show up + * as a phantom diff entry. + */ +export function lineDiff(before: string, after: string): DiffLine[] { + const out: DiffLine[] = []; + for (const part of diffLines(before, after)) { + const kind = partKind(part); + // Each part.value can contain multiple lines; split and drop a trailing + // empty string from the inevitable final "\n". + const lines = part.value.split("\n"); + if (lines.length > 0 && lines[lines.length - 1] === "") { + lines.pop(); + } + for (const text of lines) { + out.push({ kind, text }); + } + } + return out; +} + +function partKind(part: { added?: boolean; removed?: boolean }): DiffLineKind { + if (part.added) return "add"; + if (part.removed) return "remove"; + return "context"; +} + +/** Counts of added and removed lines in a diff sequence. */ +export function diffSummary(lines: DiffLine[]): { added: number; removed: number } { + let added = 0; + let removed = 0; + for (const l of lines) { + if (l.kind === "add") added++; + else if (l.kind === "remove") removed++; + } + return { added, removed }; +} diff --git a/apps/web/components/task/task-plan-panel.tsx b/apps/web/components/task/task-plan-panel.tsx index 414cc6fa57..8fe3c55726 100644 --- a/apps/web/components/task/task-plan-panel.tsx +++ b/apps/web/components/task/task-plan-panel.tsx @@ -1,7 +1,8 @@ "use client"; import { memo, useState, useCallback, useEffect, useRef, useMemo } from "react"; -import { PanelRoot, PanelBody } from "./panel-primitives"; +import { PanelRoot, PanelBody, PanelHeaderBarSplit } from "./panel-primitives"; +import { TaskPlanRevisions } from "./task-plan-revisions"; import dynamic from "next/dynamic"; import { IconLoader2, IconFileText, IconRobot, IconMessage, IconClick } from "@tabler/icons-react"; import { cn } from "@/lib/utils"; @@ -15,10 +16,10 @@ 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 { usePanelSearch } from "@/hooks/use-panel-search"; -import { planSearchPluginKey } from "@/components/editors/tiptap/search-highlight-extension"; +import { usePlanFindShortcut } from "./use-plan-find-shortcut"; // Dynamic import to avoid SSR issues with TipTap const PlanEditor = dynamic( @@ -43,7 +44,22 @@ type TaskPlanPanelProps = { }; function useTaskPlanPanelState(taskId: string | null, visible: boolean) { - const { plan, isLoading, isSaving, savePlan } = useTaskPlan(taskId, { visible }); + const { + plan, + isLoading, + isSaving, + savePlan, + revisions, + isLoadingRevisions, + loadRevisions, + loadRevisionContent, + revertTo, + previewRevisionId, + setPreviewRevision, + comparePair, + toggleCompareSelection, + clearComparePair, + } = useTaskPlan(taskId, { visible }); const activeSessionId = useAppStore((state) => state.tasks.activeSessionId); const activeSession = useAppStore((state) => activeSessionId ? (state.taskSessions.items[activeSessionId] ?? null) : null, @@ -114,6 +130,16 @@ function useTaskPlanPanelState(taskId: string | null, visible: boolean) { editorWrapperRef, editorInstanceRef, editorInstance, + revisions, + isLoadingRevisions, + loadRevisions, + loadRevisionContent, + revertTo, + previewRevisionId, + setPreviewRevision, + comparePair, + toggleCompareSelection, + clearComparePair, }; } @@ -122,37 +148,16 @@ export const TaskPlanPanel = memo(function TaskPlanPanel({ visible = true, }: TaskPlanPanelProps) { const state = useTaskPlanPanelState(taskId, visible); - const { - plan, - isLoading, - isSaving, - savePlan, - activeSessionId, - draftContent, - setDraftContent, - editorKey, - isEditorFocused, - handleEmptyStateClick, - hasUnsavedChanges, - commentState, - selectionState, - handleEditorReady, - handleCommentDeleted, - commentHighlights, - isAgentBusy, - isAgentCreatingPlan, - editorWrapperRef, - editorInstanceRef, - editorInstance, - } = state; - // Ctrl+S to save immediately - useSaveShortcut(hasUnsavedChanges, isSaving, savePlan, draftContent, plan?.title); + useSaveShortcut( + state.hasUnsavedChanges, + state.isSaving, + state.savePlan, + state.draftContent, + state.plan?.title, + ); - // Ctrl+F in-document find - const planSearch = usePlanFindShortcut(editorWrapperRef, editorInstance); - - if (isLoading) { + if (state.isLoading) { return (
    @@ -169,35 +174,64 @@ export const TaskPlanPanel = memo(function TaskPlanPanel({ ); } - const { textSelection, setTextSelection } = selectionState; + return ; +}); +function PlanPanelContent({ + taskId, + state, +}: { + taskId: string; + state: ReturnType; +}) { + const { editorWrapperRef, editorInstanceRef, editorInstance, selectionState } = state; + const { textSelection, setTextSelection } = selectionState; + // Ctrl+F in-document find (registers a keydown listener on the editor wrapper) + const planSearch = usePlanFindShortcut(editorWrapperRef, editorInstance); return ( + {planSearch.isOpen && ( ); -}); +} function removeCommentMark(editor: Editor | null, commentId: string) { if (!editor) return; @@ -441,66 +475,6 @@ function usePlanSelection( return { textSelection, setTextSelection, handleCommentHighlightClick, handleSelectionClose }; } -/** Ctrl+F find-in-plan shortcut + editor wiring */ -function usePlanFindShortcut( - wrapperRef: React.RefObject, - editor: Editor | null, -) { - const [isOpen, setIsOpen] = useState(false); - const [query, setQuery] = useState(""); - const [matchInfo, setMatchInfo] = useState({ current: 0, total: 0 }); - - const readMatchInfo = useCallback(() => { - if (!editor) return; - const s = planSearchPluginKey.getState(editor.state); - if (!s) return; - const next = { - current: s.matches.length ? s.current + 1 : 0, - total: s.matches.length, - }; - // Bail out when nothing changed — TipTap fires `transaction` on every cursor - // move, and allocating a new object identity would re-render this hook on - // every keystroke / click in the editor (and can cascade into update loops). - setMatchInfo((prev) => - prev.current === next.current && prev.total === next.total ? prev : next, - ); - }, [editor]); - - useEffect(() => { - if (!editor) return; - const handler = () => readMatchInfo(); - editor.on("transaction", handler); - return () => { - editor.off("transaction", handler); - }; - }, [editor, readMatchInfo]); - - useEffect(() => { - if (!editor) return; - if (!isOpen) return; - editor.commands.setPlanSearchQuery(query); - }, [query, isOpen, editor]); - - useEffect(() => { - if (isOpen) return; - if (!editor) return; - editor.commands.clearPlanSearch(); - }, [isOpen, editor]); - - const open = useCallback(() => setIsOpen(true), []); - const close = useCallback(() => setIsOpen(false), []); - const findNext = useCallback(() => { - editor?.commands.planSearchNext(); - }, [editor]); - const findPrev = useCallback(() => { - editor?.commands.planSearchPrev(); - }, [editor]); - - usePanelSearch({ containerRef: wrapperRef, isOpen, onOpen: open, onClose: close }); - - return { isOpen, open, close, query, setQuery, findNext, findPrev, matchInfo }; -} - /** Ctrl+S save shortcut */ function useSaveShortcut( hasUnsavedChanges: boolean, @@ -578,3 +552,58 @@ function PlanEmptyState({
    ); } + +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/components/task/task-plan-preview-dialog.tsx b/apps/web/components/task/task-plan-preview-dialog.tsx new file mode 100644 index 0000000000..3fb5a75aed --- /dev/null +++ b/apps/web/components/task/task-plan-preview-dialog.tsx @@ -0,0 +1,188 @@ +"use client"; + +import dynamic from "next/dynamic"; +import { useEffect, useState, type ReactNode } from "react"; +import { IconLoader2, IconRestore } from "@tabler/icons-react"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@kandev/ui/dialog"; +import { Button } from "@kandev/ui/button"; +import { Badge } from "@kandev/ui/badge"; +import type { TaskPlanRevision } from "@/lib/types/http"; +import { formatPreciseTime } from "@/lib/utils"; + +const PlanReadOnlyMarkdown = dynamic( + () => + import("@/components/editors/tiptap/tiptap-plan-readonly").then( + (mod) => mod.PlanReadOnlyMarkdown, + ), + { ssr: false }, +); + +type Props = { + revision: TaskPlanRevision | null; + authorLabel: string; + loadContent: (revisionId: string) => Promise; + onClose: () => void; + onRestore: () => void; + /** Open the diff dialog comparing the previewed revision with the previous + * revision (vN-1). Disabled if the previewed one is the oldest. */ + onCompareWithPrevious: (() => void) | null; + /** Open the diff dialog comparing the previewed revision with current HEAD. */ + onCompareWithCurrent: () => void; + /** True when the revision is the current HEAD — Restore + Compare-with-current + * CTAs hide. */ + isCurrent: boolean; +}; + +/** Read-only modal preview of a single plan revision. The parent should give + * this component a `key` derived from the revision id so it remounts (and + * resets its loader state) when the user opens a different revision. */ +export function PlanRevisionPreviewDialog({ + revision, + authorLabel, + loadContent, + onClose, + onRestore, + onCompareWithPrevious, + onCompareWithCurrent, + isCurrent, +}: Props): ReactNode { + const open = revision !== null; + return ( + { + if (!v) onClose(); + }} + > + + + + Version {revision?.revision_number} + {isCurrent && ( + + current + + )} + + + {authorLabel} + · + {revision ? formatPreciseTime(revision.updated_at) : ""} + {revision?.revert_of_revision_id && ( + + restored from earlier version + + )} + + + + + + + + {!isCurrent && revision && ( + <> + {onCompareWithPrevious && ( + + )} + + + + )} + + + + ); +} + +function PreviewBody({ + revision, + loadContent, +}: { + revision: TaskPlanRevision | null; + loadContent: (revisionId: string) => Promise; +}) { + const [content, setContent] = useState(null); + const [error, setError] = useState(null); + + useEffect(() => { + if (!revision) return; + let cancelled = false; + loadContent(revision.id) + .then((c) => { + if (!cancelled) setContent(c); + }) + .catch((err) => { + if (!cancelled) { + setError(err instanceof Error ? err.message : "Failed to load revision"); + } + }); + return () => { + cancelled = true; + }; + }, [revision, loadContent]); + + return ( +
    + +
    + ); +} + +function PreviewBodyInner({ content, error }: { content: string | null; error: string | null }) { + if (error) { + return
    {error}
    ; + } + if (content === null) { + return ( +
    + + Loading… +
    + ); + } + if (content.trim() === "") { + return
    (empty plan)
    ; + } + return ; +} diff --git a/apps/web/components/task/task-plan-revisions.tsx b/apps/web/components/task/task-plan-revisions.tsx new file mode 100644 index 0000000000..782d7f45c2 --- /dev/null +++ b/apps/web/components/task/task-plan-revisions.tsx @@ -0,0 +1,522 @@ +"use client"; + +import { useState, useCallback, useEffect, useMemo, useRef, type ReactNode } from "react"; +import { IconHistory, IconUser, IconRestore, IconLoader2 } from "@tabler/icons-react"; +import { Popover, PopoverContent, PopoverTrigger } from "@kandev/ui/popover"; +import { Button } from "@kandev/ui/button"; +import { Badge } from "@kandev/ui/badge"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@kandev/ui/dialog"; +import { toast } from "sonner"; +import type { TaskPlanRevision } from "@/lib/types/http"; +import { formatPreciseTime } from "@/lib/utils"; +import { AgentLogo } from "@/components/agent-logo"; +import { useAppStore } from "@/components/state-provider"; +import { PlanRevisionPreviewDialog } from "./task-plan-preview-dialog"; +import { PlanRevisionDiffDialog } from "./task-plan-diff-dialog"; + +type ComparePair = [string | null, string | null]; + +type TaskPlanRevisionsProps = { + taskId: string; + revisions: TaskPlanRevision[]; + isLoading: boolean; + isSaving: boolean; + onOpen: () => void; + onRevert: (revisionId: string) => Promise; + loadRevisionContent: (revisionId: string) => Promise; + previewRevisionId: string | null; + setPreviewRevision: (revisionId: string | null) => void; + /** comparePair / toggleCompareSelection / clearComparePair are still threaded + * through so the diff dialog can carry an arbitrary pair, but the popover + * UI no longer surfaces a selection mode — pairs are seeded from inside the + * preview dialog ("Compare with previous" / "Compare with current"). */ + comparePair: ComparePair; + toggleCompareSelection: (revisionId: string) => void; + clearComparePair: () => void; + disabled?: boolean; +}; + +/** Label rendered in the UI for any user-authored revision. Display-only — + * the backend still stamps its own author_name on the row. */ +const USER_DISPLAY_NAME = "You"; + +export function TaskPlanRevisions(props: TaskPlanRevisionsProps) { + const [open, setOpen] = useState(false); + const [confirmTarget, setConfirmTarget] = useState(null); + const [diffOpen, setDiffOpen] = useState(false); + const agentName = useActiveAgentBackendName(); + const handleOpenChange = useTriggerOnFirstOpen(setOpen, props.onOpen); + + const previewRevision = useMemo( + () => props.revisions.find((r) => r.id === props.previewRevisionId) ?? null, + [props.revisions, props.previewRevisionId], + ); + const compareRevisions = useMemo<[TaskPlanRevision | null, TaskPlanRevision | null]>( + () => [ + props.revisions.find((r) => r.id === props.comparePair[0]) ?? null, + props.revisions.find((r) => r.id === props.comparePair[1]) ?? null, + ], + [props.revisions, props.comparePair], + ); + const headRevision = props.revisions[0] ?? null; + + return ( + <> + + setOpen(false)} + /> + + ); +} + +/** Lazy-load revisions on the first popover open; subsequent opens reuse the + * cached list. Wraps the open setter so callers don't need a ref of their own. */ +function useTriggerOnFirstOpen(setOpen: (v: boolean) => void, onOpen: () => void) { + const loadedRef = useRef(false); + return useCallback( + (next: boolean) => { + setOpen(next); + if (next && !loadedRef.current) { + loadedRef.current = true; + onOpen(); + } + }, + [setOpen, onOpen], + ); +} + +function RevisionsPopover({ + open, + onOpenChange, + agentName, + confirmTargetSetter, + revisions, + isLoading, + setPreviewRevision, + disabled = false, +}: TaskPlanRevisionsProps & { + open: boolean; + onOpenChange: (next: boolean) => void; + agentName: string | null; + confirmTargetSetter: (rev: TaskPlanRevision | null) => void; +}) { + const hasRevisions = revisions.length > 0; + return ( + + + + + +
    + Plan history + {isLoading && } +
    + {/* Plain overflow-y-auto is more reliable than ScrollArea inside a Popover + * — the popover doesn't constrain its child by default, so a fixed + * max-height with native scroll keeps the list bounded. */} +
    + setPreviewRevision(rev.id)} + /> +
    +     +     + ); +} + +type DialogStackProps = { + revisions: TaskPlanRevision[]; + previewRevision: TaskPlanRevision | null; + compareRevisions: [TaskPlanRevision | null, TaskPlanRevision | null]; + diffOpen: boolean; + confirmTarget: TaskPlanRevision | null; + isSaving: boolean; + loadRevisionContent: (revisionId: string) => Promise; + headRevision: TaskPlanRevision | null; + onRevert: (revisionId: string) => Promise; + setConfirmTarget: (rev: TaskPlanRevision | null) => void; + setDiffOpen: (v: boolean) => void; + setPreviewRevision: (id: string | null) => void; + clearComparePair: () => void; + toggleCompareSelection: (id: string) => void; + closePopover: () => void; +}; + +function RevisionsDialogStack({ + revisions, + previewRevision, + compareRevisions, + diffOpen, + confirmTarget, + isSaving, + loadRevisionContent, + headRevision, + onRevert, + setConfirmTarget, + setDiffOpen, + setPreviewRevision, + clearComparePair, + toggleCompareSelection, + closePopover, +}: DialogStackProps) { + const isPreviewCurrent = previewRevision !== null && previewRevision.id === headRevision?.id; + const previousRevision = useMemo(() => { + if (!previewRevision) return null; + // revisions are sorted newest-first by revision_number; previous = the + // first entry whose revision_number is strictly less than the previewed one. + return revisions.find((r) => r.revision_number < previewRevision.revision_number) ?? null; + }, [revisions, previewRevision]); + + const handleRevert = useCallback( + async (revision: TaskPlanRevision) => { + try { + const result = await onRevert(revision.id); + if (result) { + toast.success(`Plan restored to v${revision.revision_number}`); + closePopover(); + setPreviewRevision(null); + setDiffOpen(false); + } else { + // `revertTo` (the hook impl) swallows errors and returns null, so + // we surface the failure here too — without this branch the dialog + // closes silently on failure and the user has no feedback. + toast.error("Failed to restore plan"); + } + } catch (err) { + toast.error(err instanceof Error ? err.message : "Failed to restore plan"); + } + }, + [onRevert, setPreviewRevision, setDiffOpen, closePopover], + ); + + const seedComparePair = useCallback( + (a: TaskPlanRevision, b: TaskPlanRevision) => { + // Replace any existing pair with [a, b] so the diff opens immediately. + clearComparePair(); + toggleCompareSelection(a.id); + if (a.id !== b.id) toggleCompareSelection(b.id); + setPreviewRevision(null); + setDiffOpen(true); + }, + [clearComparePair, toggleCompareSelection, setPreviewRevision, setDiffOpen], + ); + + const onCompareWithCurrent = useCallback(() => { + if (!previewRevision || !headRevision) return; + seedComparePair(previewRevision, headRevision); + }, [previewRevision, headRevision, seedComparePair]); + + const onCompareWithPrevious = useMemo(() => { + if (!previewRevision || !previousRevision) return null; + return () => seedComparePair(previousRevision, previewRevision); + }, [previewRevision, previousRevision, seedComparePair]); + + return ( + <> + setPreviewRevision(null)} + onRestore={() => { + if (previewRevision) setConfirmTarget(previewRevision); + }} + onCompareWithPrevious={onCompareWithPrevious} + onCompareWithCurrent={onCompareWithCurrent} + isCurrent={isPreviewCurrent} + /> + setDiffOpen(false)} + onRestoreOlder={(revisionId) => { + const rev = revisions.find((r) => r.id === revisionId); + if (rev) setConfirmTarget(rev); + }} + /> + setConfirmTarget(null)} + onConfirm={async () => { + if (!confirmTarget) return; + try { + await handleRevert(confirmTarget); + } finally { + setConfirmTarget(null); + } + }} + /> + + ); +} + +/** Display label for an author. User revisions always render as "You"; + * agent revisions use the stored author_name (the agent profile display name). */ +function authorLabel(revision: TaskPlanRevision): string { + if (revision.author_kind === "user") return USER_DISPLAY_NAME; + return revision.author_name || "Agent"; +} + +/** Resolve the active session's agent backend name (e.g. "claude", "codex") so + * agent revisions can render the same logo we use elsewhere. Returns null when + * there is no active session or its snapshot doesn't carry an agent_name. */ +function useActiveAgentBackendName(): string | null { + const activeSessionId = useAppStore((state) => state.tasks.activeSessionId); + const snapshot = useAppStore((state) => + activeSessionId ? state.taskSessions.items[activeSessionId]?.agent_profile_snapshot : null, + ); + return useMemo(() => { + const name = (snapshot as { agent_name?: unknown } | null | undefined)?.agent_name; + return typeof name === "string" && name.length > 0 ? name : null; + }, [snapshot]); +} + +function RevisionList({ + revisions, + isLoading, + agentName, + onRevertClick, + onRowClick, +}: { + revisions: TaskPlanRevision[]; + isLoading: boolean; + agentName: string | null; + onRevertClick: (rev: TaskPlanRevision) => void; + onRowClick: (rev: TaskPlanRevision) => void; +}) { + if (revisions.length === 0 && !isLoading) { + return ( +
    + No revisions yet. Edits will appear here. +
    + ); + } + return ( +
      + {revisions.map((rev, i) => ( + + ))} +
    + ); +} + +function RevisionAuthor({ + revision, + agentName, +}: { + revision: TaskPlanRevision; + agentName: string | null; +}) { + if (revision.author_kind === "agent") { + return ( + <> + {agentName ? : null} + + {revision.author_name} + + + ); + } + return ( + <> + + + {USER_DISPLAY_NAME} + + + ); +} + +function RevisionRow({ + revision, + isCurrent, + agentName, + onRevertClick, + onRowClick, +}: { + revision: TaskPlanRevision; + isCurrent: boolean; + agentName: string | null; + onRevertClick: (rev: TaskPlanRevision) => void; + onRowClick: (rev: TaskPlanRevision) => void; +}) { + // Force re-render every 30s so the precise timestamp ("5m ago", "Today, + // 14:32", …) refreshes as the revision ages — `formatPreciseTime` derives + // from `revision.updated_at` on every render. + const [, setTick] = useState(0); + useEffect(() => { + const id = setInterval(() => setTick((t) => t + 1), 30_000); + return () => clearInterval(id); + }, []); + const timestamp = formatPreciseTime(revision.updated_at); + + return ( +
  • + + {!isCurrent && ( + + )} +
    • + ); +} + +function RevertConfirmDialog({ + target, + isSaving, + onCancel, + onConfirm, +}: { + target: TaskPlanRevision | null; + isSaving: boolean; + onCancel: () => void; + onConfirm: () => void | Promise; +}): ReactNode { + const open = target !== null; + return ( + { + if (!v) onCancel(); + }} + > + + + Restore to version {target?.revision_number}? + + This creates a new version with v{target?.revision_number}'s content. Nothing is + lost; the current version stays in history. + + + + + + + + + ); +} diff --git a/apps/web/components/task/use-plan-find-shortcut.ts b/apps/web/components/task/use-plan-find-shortcut.ts new file mode 100644 index 0000000000..c693ec3b6c --- /dev/null +++ b/apps/web/components/task/use-plan-find-shortcut.ts @@ -0,0 +1,67 @@ +"use client"; + +import { useCallback, useEffect, useState } from "react"; +import type { Editor } from "@tiptap/core"; +import { usePanelSearch } from "@/hooks/use-panel-search"; +import { planSearchPluginKey } from "@/components/editors/tiptap/search-highlight-extension"; + +/** Ctrl+F find-in-plan shortcut + editor wiring. Returns the search-bar state + * the panel renders into ``. */ +export function usePlanFindShortcut( + wrapperRef: React.RefObject, + editor: Editor | null, +) { + const [isOpen, setIsOpen] = useState(false); + const [query, setQuery] = useState(""); + const [matchInfo, setMatchInfo] = useState({ current: 0, total: 0 }); + + const readMatchInfo = useCallback(() => { + if (!editor) return; + const s = planSearchPluginKey.getState(editor.state); + if (!s) return; + const next = { + current: s.matches.length ? s.current + 1 : 0, + total: s.matches.length, + }; + // Bail out when nothing changed — TipTap fires `transaction` on every cursor + // move, and allocating a new object identity would re-render this hook on + // every keystroke / click in the editor (and can cascade into update loops). + setMatchInfo((prev) => + prev.current === next.current && prev.total === next.total ? prev : next, + ); + }, [editor]); + + useEffect(() => { + if (!editor) return; + const handler = () => readMatchInfo(); + editor.on("transaction", handler); + return () => { + editor.off("transaction", handler); + }; + }, [editor, readMatchInfo]); + + useEffect(() => { + if (!editor) return; + if (!isOpen) return; + editor.commands.setPlanSearchQuery(query); + }, [query, isOpen, editor]); + + useEffect(() => { + if (isOpen) return; + if (!editor) return; + editor.commands.clearPlanSearch(); + }, [isOpen, editor]); + + const open = useCallback(() => setIsOpen(true), []); + const close = useCallback(() => setIsOpen(false), []); + const findNext = useCallback(() => { + editor?.commands.planSearchNext(); + }, [editor]); + const findPrev = useCallback(() => { + editor?.commands.planSearchPrev(); + }, [editor]); + + usePanelSearch({ containerRef: wrapperRef, isOpen, onOpen: open, onClose: close }); + + return { isOpen, open, close, query, setQuery, findNext, findPrev, matchInfo }; +} diff --git a/apps/web/e2e/fixtures/backend.ts b/apps/web/e2e/fixtures/backend.ts index bd9f7af981..2cc2c7b56f 100644 --- a/apps/web/e2e/fixtures/backend.ts +++ b/apps/web/e2e/fixtures/backend.ts @@ -243,6 +243,8 @@ exec git "$@" KANDEV_LOG_LEVEL: process.env.KANDEV_LOG_LEVEL ?? "warn", AGENTCTL_INSTANCE_PORT_BASE: String(agentctlPortBase), AGENTCTL_INSTANCE_PORT_MAX: String(agentctlPortMax), + // Short window makes coalesce-boundary tests practical without wall-clock waits. + KANDEV_PLAN_COALESCE_WINDOW_MS: process.env.KANDEV_PLAN_COALESCE_WINDOW_MS ?? "2000", GIT_AUTHOR_NAME: "E2E Test", GIT_AUTHOR_EMAIL: "e2e@test.local", GIT_COMMITTER_NAME: "E2E Test", diff --git a/apps/web/e2e/pages/session-page.ts b/apps/web/e2e/pages/session-page.ts index 7bda63ed53..1f65f4594c 100644 --- a/apps/web/e2e/pages/session-page.ts +++ b/apps/web/e2e/pages/session-page.ts @@ -696,6 +696,136 @@ export class SessionPage { return this.changes.getByTestId(`bulk-discard-${variant}`); } + // --- Plan revisions / rewind --- + + /** Rewind button in the plan panel header (opens revision history popover). */ + rewindButton(): Locator { + return this.planPanel.getByTestId("plan-rewind-button"); + } + + /** Plan revisions popover (opens after clicking rewind). */ + revisionsPopover(): Locator { + return this.page.getByTestId("plan-revisions-popover"); + } + + /** All revision rows inside the popover, newest-first. */ + revisionRows(): Locator { + return this.revisionsPopover().getByTestId("plan-revision-row"); + } + + /** Specific revision row by number. */ + revisionRow(n: number): Locator { + return this.revisionsPopover().locator(`[data-revision-number="${n}"]`); + } + + /** Revert button scoped to a given revision row. */ + revertButton(row: Locator): Locator { + return row.getByTestId("plan-revision-revert-button"); + } + + /** Revert-confirm dialog. */ + revertConfirmDialog(): Locator { + return this.page.getByTestId("plan-revert-confirm-dialog"); + } + + revertConfirmOk(): Locator { + return this.page.getByTestId("plan-revert-confirm-ok"); + } + + revertConfirmCancel(): Locator { + return this.page.getByTestId("plan-revert-confirm-cancel"); + } + + /** TipTap editor inside the plan panel (for typing user edits). */ + planEditor(): Locator { + return this.planPanel.locator(".ProseMirror"); + } + + /** Open the rewind popover and wait for it to render. No-op when already open. */ + async openRewind(): Promise { + if (await this.revisionsPopover().isVisible()) return; + await this.rewindButton().click(); + await expect(this.revisionsPopover()).toBeVisible({ timeout: 5_000 }); + } + + /** Open rewind, click revert on the row with the given revision number, and confirm. */ + async revertToRevision(n: number): Promise { + await this.openRewind(); + await this.revertButton(this.revisionRow(n)).click(); + await expect(this.revertConfirmDialog()).toBeVisible({ timeout: 5_000 }); + await this.revertConfirmOk().click(); + } + + // --- Plan revision preview & compare (Phase 6) --- + + /** Click the row body (not the Revert/Compare buttons) to open the preview dialog. */ + revisionRowBody(row: Locator): Locator { + return row.getByTestId("plan-revision-row-body"); + } + + previewDialog(): Locator { + return this.page.getByTestId("plan-revision-preview-dialog"); + } + + previewBody(): Locator { + return this.page.getByTestId("plan-revision-preview-body"); + } + + previewRestoreButton(): Locator { + return this.page.getByTestId("plan-revision-preview-restore"); + } + + previewCompareWithCurrentButton(): Locator { + return this.page.getByTestId("plan-revision-preview-compare-with-current"); + } + + previewCompareWithPreviousButton(): Locator { + return this.page.getByTestId("plan-revision-preview-compare-with-previous"); + } + + previewCloseButton(): Locator { + return this.page.getByTestId("plan-revision-preview-close"); + } + + diffDialog(): Locator { + return this.page.getByTestId("plan-revision-diff-dialog"); + } + + diffSummary(): Locator { + return this.page.getByTestId("plan-revision-diff-summary"); + } + + diffLines(kind?: "add" | "remove" | "context"): Locator { + const root = this.diffDialog(); + if (!kind) return root.getByTestId("plan-revision-diff-line"); + return root.locator(`[data-testid="plan-revision-diff-line"][data-line-kind="${kind}"]`); + } + + diffSplitCells(kind?: "add" | "remove" | "context" | "empty"): Locator { + const root = this.diffDialog(); + if (!kind) return root.getByTestId("plan-revision-diff-split-cell"); + return root.locator(`[data-testid="plan-revision-diff-split-cell"][data-line-kind="${kind}"]`); + } + + diffModeToggle(mode: "unified" | "split"): Locator { + return this.page.getByTestId(`plan-revision-diff-mode-${mode}`); + } + + diffRestoreButton(): Locator { + return this.page.getByTestId("plan-revision-diff-restore"); + } + + diffCloseButton(): Locator { + return this.page.getByTestId("plan-revision-diff-close"); + } + + /** Open rewind and click into the row body to bring up the preview dialog. */ + async openRevisionPreview(n: number): Promise { + await this.openRewind(); + await this.revisionRowBody(this.revisionRow(n)).click(); + await expect(this.previewDialog()).toBeVisible({ timeout: 5_000 }); + } + // --- Panel search helpers (Ctrl+F feature) --- /** Any currently-mounted panel search bar. */ diff --git a/apps/web/e2e/tests/task/plan-checkpointing.spec.ts b/apps/web/e2e/tests/task/plan-checkpointing.spec.ts new file mode 100644 index 0000000000..f1c5759f62 --- /dev/null +++ b/apps/web/e2e/tests/task/plan-checkpointing.spec.ts @@ -0,0 +1,455 @@ +import { type Page } from "@playwright/test"; +import { test, expect } from "../../fixtures/test-base"; +import type { SeedData } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; +import { SessionPage } from "../../pages/session-page"; + +// --------------------------------------------------------------------------- +// Overview +// --------------------------------------------------------------------------- +// +// Covers the plan checkpointing feature (task plan revisions + rewind/revert). +// +// Coalesce window is set to 2000ms by the backend fixture +// (KANDEV_PLAN_COALESCE_WINDOW_MS=2000). Tests that cross the boundary use +// e2e:delay(2500) inside the mock-agent script; tests that stay within use +// e2e:delay(200). +// +// Agent-authored revisions are seeded through the MCP tool +// (create_task_plan_kandev). User-authored revisions are produced by typing +// into the plan editor and waiting for the 1500ms autosave debounce. + +// --------------------------------------------------------------------------- +// Shared helpers +// --------------------------------------------------------------------------- + +/** Create a task with agent, navigate to session, wait for agent idle. */ +async function seedTaskAndWaitForIdle( + testPage: Page, + apiClient: ApiClient, + seedData: SeedData, + title: string, + description = "/e2e:simple-message", +) { + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + title, + seedData.agentProfileId, + { + description, + 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(); + // Agent writes may auto-switch the active panel to Plan, hiding the chat + // input. Accept either the chat idle placeholder or the Plan panel as a + // signal that the agent has produced output. + await expect + .poll( + async () => (await session.idleInput().isVisible()) || (await session.planPanel.isVisible()), + { timeout: 30_000, message: "agent output not visible" }, + ) + .toBe(true); + + return { session, taskId: task.id, sessionId: task.session_id! }; +} + +/** Ensure the plan panel is visible. When an agent write auto-opens it, + * the toggle is unnecessary (and would close it). After a page reload the + * panel may not auto-open, in which case togglePlanMode adds it; we poll + * a few times because hydration can race the click. */ +async function openPlanPanel(session: SessionPage) { + if (await session.planPanel.isVisible()) return; + for (let attempt = 0; attempt < 3; attempt++) { + try { + await session.togglePlanMode(); + } catch { + // toggle button not yet ready — retry after a short wait + } + try { + await session.planPanel.waitFor({ state: "visible", timeout: 5_000 }); + return; + } catch { + // Not visible yet; loop and try toggling again. + } + } + await expect(session.planPanel).toBeVisible({ timeout: 10_000 }); +} + +/** Poll for the number of revision rows in the popover. */ +async function expectRevisionCount(session: SessionPage, count: number, timeout = 10_000) { + await expect(session.revisionRows()).toHaveCount(count, { timeout }); +} + +// MCP script building blocks — each emits one agent-authored plan revision. +function mcpWrite(content: string): string { + // JSON-string-escape content: handles backslashes, quotes, and control chars. + const escaped = JSON.stringify(content).slice(1, -1); + return `e2e:mcp:kandev:create_task_plan_kandev({"task_id":"{task_id}","content":"${escaped}"})`; +} + +/** Build a mock-agent script that chains plan writes with inter-write delays. */ +function planWriteScript(writes: Array<{ content: string; delayMsAfter?: number }>): string { + const lines: string[] = ['e2e:thinking("Seeding plan revisions...")']; + for (const w of writes) { + lines.push(mcpWrite(w.content)); + if (w.delayMsAfter) lines.push(`e2e:delay(${w.delayMsAfter})`); + } + lines.push('e2e:message("Revisions seeded.")'); + return lines.join("\n"); +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +test.describe("Plan checkpointing — rewind UI", () => { + test.describe.configure({ retries: 1 }); + + test("empty plan: rewind button is disabled when no revisions exist", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(60_000); + + const { session } = await seedTaskAndWaitForIdle( + testPage, + apiClient, + seedData, + "Checkpoint empty", + ); + await openPlanPanel(session); + + await expect(session.rewindButton()).toBeVisible({ timeout: 5_000 }); + // No plan yet → no revisions → button disabled. + await expect(session.rewindButton()).toBeDisabled(); + }); + + test("single agent write: rewind exposes one revision with current badge", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(90_000); + + const script = planWriteScript([{ content: "Initial plan" }]); + const { session } = await seedTaskAndWaitForIdle( + testPage, + apiClient, + seedData, + "Checkpoint single", + script, + ); + await openPlanPanel(session); + await expect(session.planPanel.getByText("Initial plan", { exact: false })).toBeVisible({ + timeout: 10_000, + }); + + await session.openRewind(); + await expectRevisionCount(session, 1); + await expect(session.revisionRow(1).getByTestId("plan-revision-current-badge")).toBeVisible(); + }); + + test("two agent writes within coalesce window: remain one revision", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(90_000); + + const script = planWriteScript([ + { content: "Draft v1", delayMsAfter: 200 }, + { content: "Draft v2 (merged)" }, + ]); + const { session } = await seedTaskAndWaitForIdle( + testPage, + apiClient, + seedData, + "Checkpoint coalesce", + script, + ); + await openPlanPanel(session); + await expect(session.planPanel.getByText("Draft v2", { exact: false })).toBeVisible({ + timeout: 10_000, + }); + + await session.openRewind(); + await expectRevisionCount(session, 1); + }); + + test("two agent writes across coalesce window: produce two revisions", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(90_000); + + const script = planWriteScript([ + { content: "First pass", delayMsAfter: 2500 }, + { content: "Second pass" }, + ]); + const { session } = await seedTaskAndWaitForIdle( + testPage, + apiClient, + seedData, + "Checkpoint cross-window", + script, + ); + await openPlanPanel(session); + await expect(session.planPanel.getByText("Second pass", { exact: false })).toBeVisible({ + timeout: 10_000, + }); + + await session.openRewind(); + await expectRevisionCount(session, 2); + // Newest first: v2 has current badge; v1 has revert button instead. + await expect(session.revisionRow(2).getByTestId("plan-revision-current-badge")).toBeVisible(); + await expect(session.revisionRow(1).getByTestId("plan-revision-current-badge")).toHaveCount(0); + await expect(session.revisionRow(1).getByTestId("plan-revision-revert-button")).toBeVisible(); + }); + + test("revert to earlier revision: HEAD updates and new revision appears", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + + const script = planWriteScript([ + { content: "Agent draft A", delayMsAfter: 2500 }, + { content: "Agent draft B" }, + ]); + const { session } = await seedTaskAndWaitForIdle( + testPage, + apiClient, + seedData, + "Checkpoint revert", + script, + ); + await openPlanPanel(session); + await expect(session.planPanel.getByText("Agent draft B", { exact: false })).toBeVisible({ + timeout: 10_000, + }); + + await session.openRewind(); + await expectRevisionCount(session, 2); + + // Revert to v1. + await session.revertToRevision(1); + + // HEAD content must now be the old v1 content. + await expect(session.planPanel.getByText("Agent draft A", { exact: false })).toBeVisible({ + timeout: 10_000, + }); + + // Reopen popover; expect 3 revisions with restore marker on v3. + await session.openRewind(); + await expectRevisionCount(session, 3); + await expect(session.revisionRow(3).getByTestId("plan-revision-current-badge")).toBeVisible(); + await expect(session.revisionRow(3).getByTestId("plan-revision-revert-marker")).toBeVisible(); + }); + + test("revert confirm dialog: cancel leaves revisions untouched", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(90_000); + + const script = planWriteScript([ + { content: "First", delayMsAfter: 2500 }, + { content: "Second" }, + ]); + const { session } = await seedTaskAndWaitForIdle( + testPage, + apiClient, + seedData, + "Checkpoint cancel", + script, + ); + await openPlanPanel(session); + + await session.openRewind(); + await expectRevisionCount(session, 2); + + // Click revert on v1 then cancel the dialog. + await session.revertButton(session.revisionRow(1)).click(); + await expect(session.revertConfirmDialog()).toBeVisible({ timeout: 5_000 }); + await session.revertConfirmCancel().click(); + await expect(session.revertConfirmDialog()).toBeHidden({ timeout: 5_000 }); + + // HEAD is unchanged and the two revisions are still there. + await expect(session.planPanel.getByText("Second", { exact: false })).toBeVisible(); + await session.openRewind(); + await expectRevisionCount(session, 2); + }); + + test("author switch breaks coalesce: user edit creates a second revision", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + + const script = planWriteScript([{ content: "Agent wrote this" }]); + const { session } = await seedTaskAndWaitForIdle( + testPage, + apiClient, + seedData, + "Checkpoint author switch", + script, + ); + await openPlanPanel(session); + await expect(session.planPanel.getByText("Agent wrote this", { exact: false })).toBeVisible({ + timeout: 10_000, + }); + + // User types into the editor; autosave fires after 1500ms. + const editor = session.planEditor(); + await editor.click(); + const modifier = process.platform === "darwin" ? "Meta" : "Control"; + await testPage.keyboard.press(`${modifier}+a`); + await testPage.keyboard.type("User overwrote it"); + // Wait past autosave debounce + short settle time. + await testPage.waitForTimeout(2_000); + + await session.openRewind(); + await expectRevisionCount(session, 2); + // v2 (newest) is the user write — UI displays "You" for any user-authored + // revision regardless of stored author_name. v1 stays as the agent label, + // resolved from the active session's profile snapshot — exact value varies + // per E2E setup ("Claude", "mock-default", …) so we just ensure it's a + // non-empty agent name (i.e., not the user sentinel). + const v2Author = session.revisionRow(2).getByTestId("plan-revision-author"); + const v1Author = session.revisionRow(1).getByTestId("plan-revision-author"); + await expect(v1Author).not.toHaveText("You"); + await expect(v1Author).not.toBeEmpty(); + await expect(v2Author).toHaveText("You"); + }); + + test("preview dialog: row click opens content, restore goes through confirm", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + + const script = planWriteScript([ + { content: "Older draft", delayMsAfter: 2500 }, + { content: "Newer draft" }, + ]); + const { session } = await seedTaskAndWaitForIdle( + testPage, + apiClient, + seedData, + "Checkpoint preview", + script, + ); + await openPlanPanel(session); + await session.openRewind(); + await expectRevisionCount(session, 2); + + // Open the older revision via row click. + await session.openRevisionPreview(1); + await expect(session.previewBody()).toContainText("Older draft", { timeout: 5_000 }); + await expect(session.previewRestoreButton()).toBeVisible(); + + // Restore from preview routes through the confirm dialog. + await session.previewRestoreButton().click(); + await expect(session.revertConfirmDialog()).toBeVisible({ timeout: 5_000 }); + await session.revertConfirmOk().click(); + await expect(session.revertConfirmDialog()).toBeHidden({ timeout: 5_000 }); + // HEAD is now the older content again. + await expect(session.planPanel.getByText("Older draft", { exact: false })).toBeVisible({ + timeout: 10_000, + }); + + await session.openRewind(); + await expectRevisionCount(session, 3); + }); + + test("compare diff: open from preview, view unified + split, restore older", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + + const script = planWriteScript([ + { content: "alpha\nbeta\ngamma", delayMsAfter: 2500 }, + { content: "alpha\nBETA\ngamma\ndelta" }, + ]); + const { session } = await seedTaskAndWaitForIdle( + testPage, + apiClient, + seedData, + "Checkpoint compare", + script, + ); + await openPlanPanel(session); + await session.openRewind(); + await expectRevisionCount(session, 2); + + // Open the older revision in preview, then "Compare with current" — this + // is the only entry point now; there is no per-row compare toggle. + await session.openRevisionPreview(1); + await session.previewCompareWithCurrentButton().click(); + await expect(session.diffDialog()).toBeVisible({ timeout: 5_000 }); + await expect(session.diffDialog()).toContainText("Compare v1 → v2"); + + // Summary + unified diff lines. + await expect(session.diffSummary()).toContainText("added", { timeout: 5_000 }); + await expect(session.diffSummary()).toContainText("removed"); + await expect(session.diffLines("add").first()).toBeVisible(); + await expect(session.diffLines("remove").first()).toBeVisible(); + + // Switch to split mode: side-by-side cells render with kinds. + await session.diffModeToggle("split").click(); + await expect(session.diffSplitCells("remove").first()).toBeVisible({ timeout: 5_000 }); + await expect(session.diffSplitCells("add").first()).toBeVisible(); + + // Back to unified. + await session.diffModeToggle("unified").click(); + await expect(session.diffLines("add").first()).toBeVisible(); + + // Restore from diff targets the older revision (v1). + await session.diffRestoreButton().click(); + await expect(session.revertConfirmDialog()).toBeVisible({ timeout: 5_000 }); + await expect(session.revertConfirmDialog()).toContainText("version 1"); + await session.revertConfirmCancel().click(); + }); + + test("persistence across reload: revisions survive page refresh", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + + const script = planWriteScript([{ content: "One", delayMsAfter: 2500 }, { content: "Two" }]); + const { session, taskId } = await seedTaskAndWaitForIdle( + testPage, + apiClient, + seedData, + "Checkpoint reload", + script, + ); + await openPlanPanel(session); + await session.openRewind(); + await expectRevisionCount(session, 2); + + // Reload and reopen the panel. + await testPage.goto(`/t/${taskId}`); + const freshSession = new SessionPage(testPage); + await freshSession.waitForLoad(); + await openPlanPanel(freshSession); + await freshSession.openRewind(); + await expectRevisionCount(freshSession, 2); + }); +}); diff --git a/apps/web/hooks/domains/session/use-task-plan.ts b/apps/web/hooks/domains/session/use-task-plan.ts index b11cf7b0fc..ca0f33e78b 100644 --- a/apps/web/hooks/domains/session/use-task-plan.ts +++ b/apps/web/hooks/domains/session/use-task-plan.ts @@ -1,12 +1,17 @@ import { useEffect, useCallback, useState, useRef } from "react"; -import { useAppStore } from "@/components/state-provider"; +import { useAppStore, useAppStoreApi } from "@/components/state-provider"; import { getTaskPlan, createTaskPlan, updateTaskPlan, deleteTaskPlan, + listPlanRevisions, + getPlanRevision, + revertPlanRevision, } from "@/lib/api/domains/plan-api"; -import type { TaskPlan } from "@/lib/types/http"; +import type { TaskPlan, TaskPlanRevision } from "@/lib/types/http"; + +const EMPTY_REVISIONS: readonly TaskPlanRevision[] = Object.freeze([]); /** * Hook to fetch and manage the plan for a task. @@ -119,6 +124,8 @@ export function useTaskPlan(taskId: string | null, options?: { visible?: boolean } }, [taskId, setTaskPlan, setTaskPlanSaving]); + const revisionsBundle = useTaskPlanRevisions(taskId, setTaskPlanSaving, setError); + return { plan: plan ?? null, isLoading, @@ -127,5 +134,138 @@ export function useTaskPlan(taskId: string | null, options?: { visible?: boolean savePlan, deletePlan: removePlan, refetch: fetchPlan, + ...revisionsBundle, + }; +} + +const EMPTY_PAIR: readonly [string | null, string | null] = Object.freeze([ + null, + null, +]) as readonly [string | null, string | null]; + +function useTaskPlanRevisions( + taskId: string | null, + setTaskPlanSaving: (taskId: string, saving: boolean) => void, + setError: (err: string | null) => void, +) { + const revisions = useAppStore((state) => + taskId ? (state.taskPlans.revisionsByTaskId[taskId] ?? EMPTY_REVISIONS) : EMPTY_REVISIONS, + ) as TaskPlanRevision[]; + const isLoadingRevisions = useAppStore((state) => + taskId ? (state.taskPlans.revisionsLoadingByTaskId[taskId] ?? false) : false, + ); + const isRevisionsLoaded = useAppStore((state) => + taskId ? (state.taskPlans.revisionsLoadedByTaskId[taskId] ?? false) : false, + ); + const connectionStatus = useAppStore((state) => state.connection.status); + const storeApi = useAppStoreApi(); + const setPlanRevisions = useAppStore((state) => state.setPlanRevisions); + const setPlanRevisionsLoading = useAppStore((state) => state.setPlanRevisionsLoading); + const cachePlanRevisionContent = useAppStore((state) => state.cachePlanRevisionContent); + + const loadRevisions = useCallback(async () => { + if (!taskId) return; + setPlanRevisionsLoading(taskId, true); + try { + const list = await listPlanRevisions(taskId); + setPlanRevisions(taskId, list); + } catch (err) { + console.error("Failed to load plan revisions:", err); + setError(err instanceof Error ? err.message : "Failed to load revisions"); + } finally { + setPlanRevisionsLoading(taskId, false); + } + }, [taskId, setPlanRevisions, setPlanRevisionsLoading, setError]); + + // Load revisions once on mount — events may have fired before the WS connected. + useEffect(() => { + if (connectionStatus !== "connected") return; + if (!taskId || isRevisionsLoaded || isLoadingRevisions) return; + loadRevisions(); + }, [taskId, connectionStatus, isRevisionsLoaded, isLoadingRevisions, loadRevisions]); + + const loadRevisionContent = useCallback( + async (revisionId: string): Promise => { + // Read the cache lazily via the store API inside the callback so this + // function's identity stays stable across cache updates. Selecting the + // cache object as a hook input would re-create the callback whenever + // any task's content was cached, which retriggers the dialogs' + // content-fetch effects (cache short-circuits, but the work is wasted). + const cached = storeApi.getState().taskPlans.revisionContentCache[revisionId]; + if (cached !== undefined) return cached; + // Pass taskId so the backend can enforce revision-belongs-to-task. + const rev = await getPlanRevision(revisionId, taskId ?? undefined); + const content = rev.content ?? ""; + cachePlanRevisionContent(revisionId, content); + return content; + }, + [taskId, storeApi, cachePlanRevisionContent], + ); + + const revertTo = useCallback( + async (revisionId: string, authorName?: string): Promise => { + if (!taskId) return null; + setTaskPlanSaving(taskId, true); + setError(null); + try { + return await revertPlanRevision(taskId, revisionId, authorName); + } catch (err) { + console.error("Failed to revert plan:", err); + setError(err instanceof Error ? err.message : "Failed to revert plan"); + return null; + } finally { + setTaskPlanSaving(taskId, false); + } + }, + [taskId, setTaskPlanSaving, setError], + ); + + return { + revisions, + isLoadingRevisions, + loadRevisions, + loadRevisionContent, + revertTo, + ...usePreviewCompareState(taskId), + }; +} + +/** Phase 6: preview + compare selectors and actions, scoped to the active task. */ +function usePreviewCompareState(taskId: string | null) { + const previewRevisionId = useAppStore((state) => + taskId ? (state.taskPlans.previewRevisionIdByTaskId[taskId] ?? null) : null, + ); + const comparePair = useAppStore((state) => + taskId ? (state.taskPlans.comparePairByTaskId[taskId] ?? EMPTY_PAIR) : EMPTY_PAIR, + ) as [string | null, string | null]; + const setPreviewRevisionStore = useAppStore((state) => state.setPreviewRevision); + const toggleComparePairStore = useAppStore((state) => state.toggleComparePair); + const clearComparePairStore = useAppStore((state) => state.clearComparePair); + + const setPreviewRevision = useCallback( + (revisionId: string | null) => { + if (!taskId) return; + setPreviewRevisionStore(taskId, revisionId); + }, + [taskId, setPreviewRevisionStore], + ); + const toggleCompareSelection = useCallback( + (revisionId: string) => { + if (!taskId) return; + toggleComparePairStore(taskId, revisionId); + }, + [taskId, toggleComparePairStore], + ); + const clearComparePair = useCallback(() => { + if (!taskId) return; + clearComparePairStore(taskId); + }, [taskId, clearComparePairStore]); + + return { + previewRevisionId, + setPreviewRevision, + comparePair, + toggleCompareSelection, + clearComparePair, }; } diff --git a/apps/web/lib/api/domains/plan-api.ts b/apps/web/lib/api/domains/plan-api.ts index c9e5bfce53..6da9cc43f0 100644 --- a/apps/web/lib/api/domains/plan-api.ts +++ b/apps/web/lib/api/domains/plan-api.ts @@ -1,5 +1,5 @@ import { getWebSocketClient } from "@/lib/ws/connection"; -import type { TaskPlan } from "@/lib/types/http"; +import type { TaskPlan, TaskPlanRevision } from "@/lib/types/http"; const WS_CLIENT_UNAVAILABLE = "WebSocket client not available"; @@ -80,3 +80,57 @@ export async function deleteTaskPlan(taskId: string): Promise { } await client.request("task.plan.delete", { task_id: taskId }); } + +/** + * List all revisions for a plan (metadata only, newest first). + */ +export async function listPlanRevisions(taskId: string): Promise { + const client = getWebSocketClient(); + if (!client) { + throw new Error(WS_CLIENT_UNAVAILABLE); + } + const response = await client.request("task.plan.revisions.list", { task_id: taskId }); + const list = (response as { revisions?: TaskPlanRevision[] })?.revisions; + return list ?? []; +} + +/** + * Fetch a single revision including its content (for diff/preview). + * `taskId` is optional but recommended — the backend uses it to enforce + * ownership (a connected client can only read revisions belonging to a + * task it already holds a reference to). + */ +export async function getPlanRevision( + revisionId: string, + taskId?: string, +): Promise { + const client = getWebSocketClient(); + if (!client) { + throw new Error(WS_CLIENT_UNAVAILABLE); + } + const payload: Record = { revision_id: revisionId }; + if (taskId) payload.task_id = taskId; + const response = await client.request("task.plan.revision.get", payload); + return response as TaskPlanRevision; +} + +/** + * Revert the plan to a previous revision. Server appends a new revision with the target's content. + */ +export async function revertPlanRevision( + taskId: string, + revisionId: string, + authorName?: string, +): Promise { + const client = getWebSocketClient(); + if (!client) { + throw new Error(WS_CLIENT_UNAVAILABLE); + } + const payload: Record = { + task_id: taskId, + revision_id: revisionId, + }; + if (authorName) payload.author_name = authorName; + const response = await client.request("task.plan.revert", payload); + return response as TaskPlanRevision; +} diff --git a/apps/web/lib/state/slices/session/session-slice.ts b/apps/web/lib/state/slices/session/session-slice.ts index 1a2a0e10e6..1ade07a095 100644 --- a/apps/web/lib/state/slices/session/session-slice.ts +++ b/apps/web/lib/state/slices/session/session-slice.ts @@ -79,6 +79,12 @@ export const defaultSessionState: SessionSliceState = { loadingByTaskId: {}, loadedByTaskId: {}, savingByTaskId: {}, + revisionsByTaskId: {}, + revisionsLoadingByTaskId: {}, + revisionsLoadedByTaskId: {}, + revisionContentCache: {}, + previewRevisionIdByTaskId: {}, + comparePairByTaskId: {}, lastSeenUpdatedAtByTaskId: {}, }, queue: { bySessionId: {}, isLoading: {} }, @@ -178,10 +184,23 @@ function buildTaskPlanActions(set: ImmerSet) { }), clearTaskPlan: (taskId: string) => set((draft) => { + // revisionContentCache is keyed by revisionId, so pick the IDs for this + // task before deleting the revisions list and drop their cache entries. + const revs = draft.taskPlans.revisionsByTaskId[taskId]; + if (revs) { + for (const r of revs) { + delete draft.taskPlans.revisionContentCache[r.id]; + } + } delete draft.taskPlans.byTaskId[taskId]; delete draft.taskPlans.loadingByTaskId[taskId]; delete draft.taskPlans.loadedByTaskId[taskId]; delete draft.taskPlans.savingByTaskId[taskId]; + delete draft.taskPlans.revisionsByTaskId[taskId]; + delete draft.taskPlans.revisionsLoadingByTaskId[taskId]; + delete draft.taskPlans.revisionsLoadedByTaskId[taskId]; + delete draft.taskPlans.previewRevisionIdByTaskId[taskId]; + delete draft.taskPlans.comparePairByTaskId[taskId]; delete draft.taskPlans.lastSeenUpdatedAtByTaskId[taskId]; }), markTaskPlanSeen: (taskId: string) => @@ -189,9 +208,86 @@ function buildTaskPlanActions(set: ImmerSet) { const plan = draft.taskPlans.byTaskId[taskId]; draft.taskPlans.lastSeenUpdatedAtByTaskId[taskId] = plan?.updated_at ?? ""; }), + setPlanRevisions: ( + taskId: string, + revisions: Parameters[1], + ) => + set((draft) => { + draft.taskPlans.revisionsByTaskId[taskId] = [...revisions].sort( + (a, b) => b.revision_number - a.revision_number, + ); + draft.taskPlans.revisionsLoadedByTaskId[taskId] = true; + draft.taskPlans.revisionsLoadingByTaskId[taskId] = false; + }), + upsertPlanRevision: ( + taskId: string, + revision: Parameters[1], + ) => + set((draft) => { + const list = draft.taskPlans.revisionsByTaskId[taskId] ?? []; + const idx = list.findIndex((r) => r.id === revision.id); + if (idx === -1) { + list.unshift(revision); + } else { + list[idx] = { ...list[idx], ...revision }; + // Coalesced writes update an existing revision's content on the + // backend, but the WS payload carries metadata only — drop any + // cached content so the next preview refetches. + delete draft.taskPlans.revisionContentCache[revision.id]; + } + list.sort((a, b) => b.revision_number - a.revision_number); + draft.taskPlans.revisionsByTaskId[taskId] = list; + }), + setPlanRevisionsLoading: (taskId: string, loading: boolean) => + set((draft) => { + draft.taskPlans.revisionsLoadingByTaskId[taskId] = loading; + }), + cachePlanRevisionContent: (revisionId: string, content: string) => + set((draft) => { + draft.taskPlans.revisionContentCache[revisionId] = content; + }), + ...buildPreviewCompareActions(set), }; } +function buildPreviewCompareActions(set: ImmerSet) { + return { + setPreviewRevision: (taskId: string, revisionId: string | null) => + set((draft) => { + if (revisionId === null) { + delete draft.taskPlans.previewRevisionIdByTaskId[taskId]; + } else { + draft.taskPlans.previewRevisionIdByTaskId[taskId] = revisionId; + } + }), + toggleComparePair: (taskId: string, revisionId: string) => + set((draft) => { + draft.taskPlans.comparePairByTaskId[taskId] = nextPair( + draft.taskPlans.comparePairByTaskId[taskId] ?? [null, null], + revisionId, + ); + }), + clearComparePair: (taskId: string) => + set((draft) => { + delete draft.taskPlans.comparePairByTaskId[taskId]; + }), + }; +} + +/** Compute the next compare-pair after a toggle. Already-selected ids unselect; + * empty slots fill in order (slot 0 first); a full pair drops slot 0 and shifts + * slot 1 → 0, putting the new pick in slot 1 (FIFO of length 2). */ +function nextPair( + current: readonly [string | null, string | null], + revisionId: string, +): [string | null, string | null] { + if (current[0] === revisionId) return [current[1], null]; + if (current[1] === revisionId) return [current[0], null]; + if (current[0] === null) return [revisionId, current[1]]; + if (current[1] === null) return [current[0], revisionId]; + return [current[1], revisionId]; +} + export const createSessionSlice: StateCreator< SessionSlice, [["zustand/immer", never]], diff --git a/apps/web/lib/state/slices/session/types.ts b/apps/web/lib/state/slices/session/types.ts index 10718ea1b5..cde68ad071 100644 --- a/apps/web/lib/state/slices/session/types.ts +++ b/apps/web/lib/state/slices/session/types.ts @@ -1,4 +1,4 @@ -import type { Message, TaskSession, Turn, TaskPlan } from "@/lib/types/http"; +import type { Message, TaskSession, Turn, TaskPlan, TaskPlanRevision } from "@/lib/types/http"; export type MessagesState = { bySession: Record; @@ -62,11 +62,24 @@ export type ActiveModelState = { bySessionId: Record; }; +/** Ordered slot pair for the compare-revisions feature. Either slot may be + * null. Reducers enforce a 2-slot cap and reject duplicates. */ +export type ComparePair = [string | null, string | null]; + export type TaskPlansState = { byTaskId: Record; loadingByTaskId: Record; loadedByTaskId: Record; savingByTaskId: Record; + revisionsByTaskId: Record; + revisionsLoadingByTaskId: Record; + revisionsLoadedByTaskId: Record; + revisionContentCache: Record; // revisionId -> content + // Phase 6: preview + compare state + previewRevisionIdByTaskId: Record; + comparePairByTaskId: Record; + // From main: tracks the last `updated_at` the user has seen, so the panel + // can flag unseen-changes after agent writes between visits. lastSeenUpdatedAtByTaskId: Record; }; @@ -143,6 +156,15 @@ export type SessionSliceActions = { setTaskPlanSaving: (taskId: string, saving: boolean) => void; clearTaskPlan: (taskId: string) => void; markTaskPlanSeen: (taskId: string) => void; + // Revision actions + setPlanRevisions: (taskId: string, revisions: TaskPlanRevision[]) => void; + upsertPlanRevision: (taskId: string, revision: TaskPlanRevision) => void; + setPlanRevisionsLoading: (taskId: string, loading: boolean) => void; + cachePlanRevisionContent: (revisionId: string, content: string) => void; + // Phase 6: preview + compare actions + setPreviewRevision: (taskId: string, revisionId: string | null) => void; + toggleComparePair: (taskId: string, revisionId: string) => void; + clearComparePair: (taskId: string) => void; // Queue actions setQueueStatus: (sessionId: string, status: QueueStatus) => void; setQueueLoading: (sessionId: string, loading: boolean) => void; diff --git a/apps/web/lib/state/store.ts b/apps/web/lib/state/store.ts index cc7a75eb22..4347aae3ff 100644 --- a/apps/web/lib/state/store.ts +++ b/apps/web/lib/state/store.ts @@ -396,6 +396,21 @@ export type AppState = { setTaskPlanSaving: (taskId: string, saving: boolean) => void; clearTaskPlan: (taskId: string) => void; markTaskPlanSeen: (taskId: string) => void; + // Plan revision actions + setPlanRevisions: ( + taskId: string, + revisions: import("@/lib/types/http").TaskPlanRevision[], + ) => void; + upsertPlanRevision: ( + taskId: string, + revision: import("@/lib/types/http").TaskPlanRevision, + ) => void; + setPlanRevisionsLoading: (taskId: string, loading: boolean) => void; + cachePlanRevisionContent: (revisionId: string, content: string) => void; + // Plan revision preview + compare actions + setPreviewRevision: (taskId: string, revisionId: string | null) => void; + toggleComparePair: (taskId: string, revisionId: string) => void; + clearComparePair: (taskId: string) => void; // Queue actions setQueueStatus: (sessionId: string, status: import("./slices/session/types").QueueStatus) => void; setQueueLoading: (sessionId: string, loading: boolean) => void; diff --git a/apps/web/lib/types/backend.ts b/apps/web/lib/types/backend.ts index e3f507f1dc..70f93f7cfa 100644 --- a/apps/web/lib/types/backend.ts +++ b/apps/web/lib/types/backend.ts @@ -7,6 +7,8 @@ export type BackendMessageType = | "task.plan.created" | "task.plan.updated" | "task.plan.deleted" + | "task.plan.revision.created" + | "task.plan.reverted" | "agent.updated" | "agent.available.updated" | "terminal.output" @@ -477,6 +479,19 @@ export type TaskPlanEventPayload = { 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 QueuedMessagePayload = { content: string; model?: string; @@ -501,6 +516,11 @@ export type BackendMessageMap = { "task.plan.created": BackendMessage<"task.plan.created", TaskPlanEventPayload>; "task.plan.updated": BackendMessage<"task.plan.updated", TaskPlanEventPayload>; "task.plan.deleted": BackendMessage<"task.plan.deleted", TaskPlanEventPayload>; + "task.plan.revision.created": BackendMessage< + "task.plan.revision.created", + TaskPlanRevisionEventPayload + >; + "task.plan.reverted": BackendMessage<"task.plan.reverted", TaskPlanRevisionEventPayload>; "agent.updated": BackendMessage<"agent.updated", AgentUpdatePayload>; "agent.available.updated": BackendMessage< "agent.available.updated", diff --git a/apps/web/lib/types/http-agents.ts b/apps/web/lib/types/http-agents.ts index 80c63e764f..8decf7efab 100644 --- a/apps/web/lib/types/http-agents.ts +++ b/apps/web/lib/types/http-agents.ts @@ -269,6 +269,21 @@ export type TaskPlanResponse = { plan: TaskPlan | null; }; +// A single revision in the task plan history. `content` is optional because +// list responses/WS broadcasts omit it for size; fetch detail separately. +export type TaskPlanRevision = { + id: string; + task_id: string; + revision_number: number; + title: string; + content?: string; + author_kind: "agent" | "user"; + author_name: string; + revert_of_revision_id?: string | null; + created_at: string; + updated_at: string; +}; + // Stats types export type TaskStatsDTO = { task_id: string; diff --git a/apps/web/lib/utils.ts b/apps/web/lib/utils.ts index 16b4d0b8f9..3865ffd7ec 100644 --- a/apps/web/lib/utils.ts +++ b/apps/web/lib/utils.ts @@ -170,6 +170,46 @@ export function formatRelativeTime(dateString: string): string { return date.toLocaleDateString(); } +/** + * Format a date string with minute-level precision and short context. + * Examples: "just now", "5m ago", "Today, 14:32", "Yesterday, 14:32", + * "Apr 25, 14:32", "Apr 25 2025, 14:32". + */ +export function formatPreciseTime(dateString: string): string { + const date = new Date(dateString); + const now = new Date(); + const diffMs = now.getTime() - date.getTime(); + const diffSec = Math.floor(diffMs / 1000); + const diffMin = Math.floor(diffSec / 60); + + if (diffSec < 10) return "just now"; + if (diffSec < 60) return `${diffSec}s ago`; + if (diffMin < 60) return `${diffMin}m ago`; + + const time = date.toLocaleTimeString(undefined, { hour: "2-digit", minute: "2-digit" }); + const sameDay = + date.getFullYear() === now.getFullYear() && + date.getMonth() === now.getMonth() && + date.getDate() === now.getDate(); + if (sameDay) return `Today, ${time}`; + + const yesterday = new Date(now); + yesterday.setDate(now.getDate() - 1); + const isYesterday = + date.getFullYear() === yesterday.getFullYear() && + date.getMonth() === yesterday.getMonth() && + date.getDate() === yesterday.getDate(); + if (isYesterday) return `Yesterday, ${time}`; + + const sameYear = date.getFullYear() === now.getFullYear(); + const dateLabel = date.toLocaleDateString(undefined, { + month: "short", + day: "numeric", + year: sameYear ? undefined : "numeric", + }); + return `${dateLabel}, ${time}`; +} + /** * Convert an absolute file path to a relative path based on a workspace root. * If the path is within the workspace, returns the relative portion. diff --git a/apps/web/lib/ws/handlers/task-plans.ts b/apps/web/lib/ws/handlers/task-plans.ts index 9973ad0c26..81b4c10945 100644 --- a/apps/web/lib/ws/handlers/task-plans.ts +++ b/apps/web/lib/ws/handlers/task-plans.ts @@ -2,8 +2,12 @@ import type { StoreApi } from "zustand"; import type { AppState } from "@/lib/state/store"; import type { BackendMessageMap } from "@/lib/types/backend"; import type { WsHandlers } from "@/lib/ws/handlers/types"; +import type { TaskPlanRevision } from "@/lib/types/http"; type PlanMessage = BackendMessageMap["task.plan.created"] | BackendMessageMap["task.plan.updated"]; +type RevisionMessage = + | BackendMessageMap["task.plan.revision.created"] + | BackendMessageMap["task.plan.reverted"]; function handlePlanUpsert(store: StoreApi, message: PlanMessage) { const { task_id, id, title, content, created_by, created_at, updated_at } = message.payload; @@ -28,6 +32,22 @@ function handlePlanUpsert(store: StoreApi, message: PlanMessage) { } } +function handleRevisionPush(store: StoreApi, message: RevisionMessage) { + const p = message.payload; + const rev: TaskPlanRevision = { + id: p.id, + task_id: p.task_id, + revision_number: p.revision_number, + title: p.title, + author_kind: p.author_kind, + author_name: p.author_name, + revert_of_revision_id: p.revert_of_revision_id ?? null, + created_at: p.created_at, + updated_at: p.updated_at, + }; + store.getState().upsertPlanRevision(p.task_id, rev); +} + export function registerTaskPlansHandlers(store: StoreApi): WsHandlers { return { "task.plan.created": (message) => handlePlanUpsert(store, message), @@ -41,5 +61,13 @@ export function registerTaskPlansHandlers(store: StoreApi): WsHandlers store.getState().setTaskPlan(task_id, null); store.getState().markTaskPlanSeen(task_id); }, + "task.plan.revision.created": (message) => handleRevisionPush(store, message), + // `task.plan.reverted` is published alongside `task.plan.revision.created` + // for the same row by the backend RevertPlan path. Re-running the upsert + // on this event would be a no-op against the list (same id, same data) + // but would needlessly evict the revisionContentCache entry and trigger + // an extra Zustand update. Treat this event as a notification-only signal + // — register no-op so the dispatcher doesn't warn about an unhandled type. + "task.plan.reverted": () => {}, }; }