From 34227949328cc6c26607663250afee7fefebc533 Mon Sep 17 00:00:00 2001 From: Luan Martins Date: Fri, 11 Sep 2026 12:51:31 +0100 Subject: [PATCH] feat(workflow): one-shot step-entry move overrides (#3389) Co-authored-by: Carlos Florencio --- apps/backend/internal/backendapp/services.go | 1 + .../mcp/handlers/config_handlers_test.go | 123 +----- .../mcp/handlers/config_task_handlers.go | 189 ++++---- .../config_task_handlers_envelope_test.go | 101 +++++ .../internal/mcp/server/config_handlers.go | 10 +- .../mcp/server/config_handlers_test.go | 20 + apps/backend/internal/mcp/server/handlers.go | 26 +- apps/backend/internal/mcp/server/server.go | 3 +- .../internal/office/service/prompt_builder.go | 24 + .../office/service/prompt_builder_test.go | 31 ++ .../office/service/scheduler_integration.go | 1 + .../orchestrator/event_handlers_workflow.go | 191 +++++++- ...rs_workflow_move_override_findings_test.go | 214 +++++++++ .../event_handlers_workflow_triggers_test.go | 124 ++++++ .../messagequeue/repository_sqlite.go | 69 ++- .../messagequeue/repository_sqlite_test.go | 37 ++ .../orchestrator/messagequeue/types.go | 6 + .../prompt_launch_fallback_test.go | 5 +- .../internal/orchestrator/task_operations.go | 113 +++-- .../internal/orchestrator/watcher/watcher.go | 1 + .../orchestrator/workflow_prompt_test.go | 32 ++ apps/backend/internal/task/dto/dto.go | 14 +- apps/backend/internal/task/handlers/errors.go | 28 ++ .../task/handlers/task_http_handlers.go | 18 +- .../task/handlers/task_ws_handlers.go | 23 +- apps/backend/internal/task/models/models.go | 7 + .../internal/task/models/public_metadata.go | 6 +- .../task/models/public_metadata_test.go | 37 ++ .../internal/task/repository/sqlite/task.go | 23 + .../sqlite/task_deferred_move_test.go | 90 ++++ .../internal/task/service/service_events.go | 3 +- .../internal/task/service/service_workflow.go | 128 +++++- .../service_workflow_stranded_marker_test.go | 130 ++++++ .../task/service/service_workflow_test.go | 52 +++ apps/backend/internal/workflow/move/errors.go | 49 ++ .../internal/workflow/move/normalization.go | 41 ++ .../backend/internal/workflow/move/overlay.go | 118 +++++ .../internal/workflow/move/overlay_test.go | 190 ++++++++ apps/backend/internal/workflow/move/types.go | 144 ++++++ .../internal/workflow/move/types_test.go | 135 ++++++ .../model-config-selector-content.tsx | 1 + ...task-preview-panel-step-indicator.test.tsx | 4 +- apps/web/components/task-preview-panel.tsx | 10 +- .../components/task/chat/chat-status-bar.tsx | 35 +- .../task/chat/messages/user-message-body.tsx | 115 +++-- .../task/mobile/mobile-task-move-options.tsx | 137 ++++++ .../session-task-switcher-sheet-dialogs.tsx | 121 +++++ .../session-task-switcher-sheet-props.ts | 2 + .../session-task-switcher-sheet.test.tsx | 48 ++ .../mobile/session-task-switcher-sheet.tsx | 252 ++++------- .../task/passthrough-toolbar.test.tsx | 3 + .../components/task/passthrough-toolbar.tsx | 30 +- .../task/task-move-context-menu.test.tsx | 174 ++++++++ .../task/task-move-context-menu.tsx | 296 ++++++++++++- .../task/task-switcher-context-menu.test.tsx | 183 +++++++- .../task/task-switcher-context-menu.tsx | 345 +++++++++------ .../web/components/task/task-switcher-row.tsx | 4 + .../task/task-switcher-touch-drag-cancel.ts | 87 ++++ .../components/task/task-switcher-types.ts | 3 + apps/web/components/task/task-switcher.tsx | 4 + .../task/workflow-move-options-form.test.tsx | 100 +++++ .../task/workflow-move-options.test.ts | 28 ++ .../components/task/workflow-move-options.tsx | 333 ++++++++++++++ .../workflow-move-proceed-button.test.tsx | 383 ++++++++++++++++ .../task/workflow-move-proceed-button.tsx | 417 ++++++++++++++++++ .../components/task/workflow-move-surface.ts | 12 + .../task/workflow-step-disclosure-controls.ts | 151 +++++++ .../task/workflow-step-disclosure.tsx | 341 ++++++-------- .../components/task/workflow-stepper.test.tsx | 38 ++ apps/web/components/task/workflow-stepper.tsx | 91 +++- apps/web/e2e/helpers/api-client.ts | 102 +++-- ...obile-workflow-step-move-overrides.spec.ts | 179 ++++++++ .../workflow-step-move-overrides-helpers.ts | 121 +++++ .../workflow-step-move-overrides.spec.ts | 266 +++++++++++ .../hooks/domains/kanban/use-plan-actions.ts | 87 ++-- .../kanban/use-preview-workflow-step-move.ts | 7 +- .../domains/kanban/use-workflow-move.test.ts | 65 +++ .../hooks/domains/kanban/use-workflow-move.ts | 35 ++ .../domains/kanban/use-workflow-step-move.ts | 6 +- apps/web/hooks/use-task-actions.ts | 9 +- apps/web/lib/api/domains/kanban-api.test.ts | 70 +++ apps/web/lib/api/domains/kanban-api.ts | 52 ++- apps/web/lib/types/http.ts | 6 + .../lib/utils/workflow-instructions.test.ts | 90 +++- apps/web/lib/utils/workflow-instructions.ts | 92 +++- apps/web/src/locales/en/task.json | 11 + apps/web/src/locales/en/workflows.json | 1 + apps/web/src/locales/pseudo/task.json | 11 + apps/web/src/locales/pseudo/workflows.json | 1 + apps/web/src/locales/pt-pt/task.json | 11 + apps/web/src/locales/pt-pt/workflows.json | 1 + apps/web/src/locales/zh-cn/task.json | 11 + apps/web/src/locales/zh-cn/workflows.json | 1 + apps/web/src/locales/zh-hk/task.json | 11 + apps/web/src/locales/zh-hk/workflows.json | 1 + apps/web/src/locales/zh-tw/task.json | 11 + apps/web/src/locales/zh-tw/workflows.json | 1 + .../2026-08-13-workflow-move-overrides.md | 74 ++++ .../plan.md | 45 ++ .../task-01-preserve-prepared-profile.md | 28 ++ .../workflow-step-move-overrides/plan.md | 122 +++++ ...task-01-backend-move-contract-and-entry.md | 48 ++ .../task-02-frontend-move-options.md | 48 ++ .../task-03-e2e-docs-and-delivery.md | 50 +++ docs/public/agent-communication.md | 16 +- docs/public/tasks-and-workflows.md | 12 +- docs/public/websocket-api.md | 21 + docs/public/workflow-import-export.md | 2 +- docs/public/workflow-tips.md | 6 + docs/specs/office/glossary.md | 1 - .../workflow-step-move-overrides/spec.md | 124 ++++++ 111 files changed, 7145 insertions(+), 1016 deletions(-) create mode 100644 apps/backend/internal/mcp/handlers/config_task_handlers_envelope_test.go create mode 100644 apps/backend/internal/orchestrator/event_handlers_workflow_move_override_findings_test.go create mode 100644 apps/backend/internal/task/service/service_workflow_stranded_marker_test.go create mode 100644 apps/backend/internal/workflow/move/errors.go create mode 100644 apps/backend/internal/workflow/move/normalization.go create mode 100644 apps/backend/internal/workflow/move/overlay.go create mode 100644 apps/backend/internal/workflow/move/overlay_test.go create mode 100644 apps/backend/internal/workflow/move/types.go create mode 100644 apps/backend/internal/workflow/move/types_test.go create mode 100644 apps/web/components/task/mobile/mobile-task-move-options.tsx create mode 100644 apps/web/components/task/mobile/session-task-switcher-sheet-dialogs.tsx create mode 100644 apps/web/components/task/task-move-context-menu.test.tsx create mode 100644 apps/web/components/task/task-switcher-touch-drag-cancel.ts create mode 100644 apps/web/components/task/workflow-move-options-form.test.tsx create mode 100644 apps/web/components/task/workflow-move-options.test.ts create mode 100644 apps/web/components/task/workflow-move-options.tsx create mode 100644 apps/web/components/task/workflow-move-proceed-button.test.tsx create mode 100644 apps/web/components/task/workflow-move-proceed-button.tsx create mode 100644 apps/web/components/task/workflow-move-surface.ts create mode 100644 apps/web/components/task/workflow-step-disclosure-controls.ts create mode 100644 apps/web/e2e/tests/workflow/mobile-workflow-step-move-overrides.spec.ts create mode 100644 apps/web/e2e/tests/workflow/workflow-step-move-overrides-helpers.ts create mode 100644 apps/web/e2e/tests/workflow/workflow-step-move-overrides.spec.ts create mode 100644 apps/web/hooks/domains/kanban/use-workflow-move.test.ts create mode 100644 apps/web/hooks/domains/kanban/use-workflow-move.ts create mode 100644 docs/decisions/2026-08-13-workflow-move-overrides.md create mode 100644 docs/plans/workflow-move-profile-override-repair/plan.md create mode 100644 docs/plans/workflow-move-profile-override-repair/task-01-preserve-prepared-profile.md create mode 100644 docs/plans/workflow-step-move-overrides/plan.md create mode 100644 docs/plans/workflow-step-move-overrides/task-01-backend-move-contract-and-entry.md create mode 100644 docs/plans/workflow-step-move-overrides/task-02-frontend-move-options.md create mode 100644 docs/plans/workflow-step-move-overrides/task-03-e2e-docs-and-delivery.md create mode 100644 docs/specs/workflow-step-move-overrides/spec.md diff --git a/apps/backend/internal/backendapp/services.go b/apps/backend/internal/backendapp/services.go index 8b745cb78a..a048706c50 100644 --- a/apps/backend/internal/backendapp/services.go +++ b/apps/backend/internal/backendapp/services.go @@ -222,6 +222,7 @@ func provideServices(cfg *config.Config, log *logger.Logger, repos *Repositories // Wire start step resolver to task service for CreateTask taskSvc.SetStartStepResolver(&startStepResolverAdapter{svc: workflowSvc}) + // Session history is owned by workflow service, but access is owned by the // task service. Keep the authorization check at the service boundary. workflowSvc.SetSessionAccessChecker(taskSvc.AuthorizeSessionAccess) diff --git a/apps/backend/internal/mcp/handlers/config_handlers_test.go b/apps/backend/internal/mcp/handlers/config_handlers_test.go index aca38b793d..6ac8f9d628 100644 --- a/apps/backend/internal/mcp/handlers/config_handlers_test.go +++ b/apps/backend/internal/mcp/handlers/config_handlers_test.go @@ -465,14 +465,7 @@ func TestApplyMoveTaskImmediate_RollsBackExactHandoffAfterMoveFailure(t *testing }) h := &Handlers{taskSvc: svc, messageQueue: queue, logger: testLogger(t).WithFields()} msg := makeWSMessage(t, ws.ActionMCPMoveTask, map[string]interface{}{}) - response, err := h.applyMoveTaskImmediate(ctx, msg, struct { - TaskID string `json:"task_id"` - WorkflowID string `json:"workflow_id"` - WorkflowStepID string `json:"workflow_step_id"` - Position int `json:"position"` - Prompt string `json:"prompt"` - SenderSessionID string `json:"sender_session_id"` - }{ + response, err := h.applyMoveTaskImmediate(ctx, msg, moveTaskRequest{ TaskID: "task-rollback", WorkflowID: "missing-workflow", WorkflowStepID: "missing-step", Prompt: "handoff", }, session) @@ -512,7 +505,13 @@ func (r *pendingMoveFailingQueuer) RemoveEntryForSession( return nil, messagequeue.ErrEntryNotFound } -func TestDeferMoveTask_RollsBackHandoffWhenPendingMovePersistenceFails(t *testing.T) { +// TestDeferMoveTask_PendingMovePersistenceFailureLeavesQueueUntouched verifies +// the deferred move path surfaces an internal error when SetPendingMove fails +// and does not mutate the session queue. One-shot instructions now ride the +// PendingMove's EntryOptions rather than a pre-queued hand-off, so there is no +// hand-off message to roll back: the pre-existing queue entry must survive and +// nothing may be removed. +func TestDeferMoveTask_PendingMovePersistenceFailureLeavesQueueUntouched(t *testing.T) { svc, repo := newTestTaskService(t) seedRunningTask( t, repo, @@ -535,72 +534,11 @@ func TestDeferMoveTask_RollsBackHandoffWhenPendingMovePersistenceFails(t *testin response, err := h.handleMoveTask(context.Background(), msg) require.NoError(t, err) assertWSError(t, response, ws.ErrorCodeInternalError) - require.Equal(t, []string{"queued-2"}, queue.removedIDs) + require.Empty(t, queue.removedIDs) require.Len(t, queue.calls, 1) assert.Equal(t, "preexisting", queue.calls[0].ID) } -type rollbackContextMessageQueuer struct { - recordingMessageQueuer - queued *messagequeue.QueuedMessage - rollbackCtx context.Context - removedEntry string - takeCalled bool -} - -func (r *rollbackContextMessageQueuer) QueueMessageWithMetadata( - ctx context.Context, - sessionID, taskID, content, model, userID string, - planMode bool, - attachments []messagequeue.MessageAttachment, - metadata map[string]interface{}, -) (*messagequeue.QueuedMessage, error) { - msg, err := r.recordingMessageQueuer.QueueMessageWithMetadata( - ctx, sessionID, taskID, content, model, userID, planMode, attachments, metadata, - ) - if err == nil { - msg.ID = "queued-handoff" - r.queued = msg - } - return msg, err -} - -func (r *rollbackContextMessageQueuer) TakeQueued(ctx context.Context, _ string) (*messagequeue.QueuedMessage, bool) { - r.rollbackCtx = ctx - r.takeCalled = true - if r.queued == nil { - return nil, false - } - msg := r.queued - r.queued = nil - return msg, true -} - -func (r *rollbackContextMessageQueuer) RemoveEntry(ctx context.Context, _ string, entryID string) error { - r.rollbackCtx = ctx - r.removedEntry = entryID - if r.queued != nil && r.queued.ID == entryID { - r.queued = nil - } - return nil -} - -func (r *rollbackContextMessageQueuer) RemoveEntryForSession( - ctx context.Context, - identity messagequeue.QueueSessionIdentity, - entryID string, -) (*messagequeue.QueueRemovalResult, error) { - r.rollbackCtx = ctx - r.removedEntry = entryID - if r.queued == nil || r.queued.ID != entryID || - r.queued.SessionID != identity.SessionID || r.queued.TaskID != identity.TaskID { - return nil, messagequeue.ErrEntryNotFound - } - removed := *r.queued - r.queued = nil - return &messagequeue.QueueRemovalResult{Removed: []messagequeue.QueuedMessage{removed}}, nil -} - // TestQueueMoveTaskPrompt_NilQueueReturnsError ensures the call is safe (no panic) // and surfaces a descriptive error so callers can fail fast instead of silently // dropping the user-supplied prompt. @@ -657,41 +595,6 @@ func TestQueueMoveTaskPrompt_QueuesWithExpectedFields(t *testing.T) { assert.Equal(t, "", got.Model) } -func TestApplyMoveTaskImmediateRollsBackQueueWithDetachedContext(t *testing.T) { - taskSvc, _ := newTestTaskService(t) - queue := &rollbackContextMessageQueuer{} - h := &Handlers{ - taskSvc: taskSvc, - messageQueue: queue, - logger: testLogger(t).WithFields(), - } - ctx, cancel := context.WithCancel(context.Background()) - cancel() - msg := makeWSMessage(t, ws.ActionMCPMoveTask, nil) - req := struct { - TaskID string `json:"task_id"` - WorkflowID string `json:"workflow_id"` - WorkflowStepID string `json:"workflow_step_id"` - Position int `json:"position"` - Prompt string `json:"prompt"` - SenderSessionID string `json:"sender_session_id"` - }{ - TaskID: "missing-task", - WorkflowID: "missing-workflow", - WorkflowStepID: "missing-step", - Prompt: "handoff", - } - session := &models.TaskSession{ID: "session-rollback", TaskID: req.TaskID} - - resp, err := h.applyMoveTaskImmediate(ctx, msg, req, session) - require.NoError(t, err) - assertWSError(t, resp, ws.ErrorCodeInternalError) - require.NotNil(t, queue.rollbackCtx) - assert.NoError(t, queue.rollbackCtx.Err()) - assert.Equal(t, "queued-handoff", queue.removedEntry) - assert.False(t, queue.takeCalled) - assert.Nil(t, queue.queued) -} func TestHandleDeleteTask_MissingTaskID(t *testing.T) { h := &Handlers{} msg := makeWSMessage(t, ws.ActionMCPDeleteTask, map[string]string{}) @@ -1325,8 +1228,12 @@ func TestDeferMoveTask_AcceptsValidStep(t *testing.T) { assert.Equal(t, "dst-step3", queue.pendingMoves[0].WorkflowStepID) assert.Equal(t, "sess-caller3", queue.pendingMoves[0].SenderSessionID) assert.NotEmpty(t, queue.pendingMoves[0].MoveID) - require.Len(t, queue.calls, 1) - assert.Equal(t, queue.pendingMoves[0].MoveID, queue.calls[0].Metadata[messagequeue.MetadataDeferredMoveID]) + // The legacy prompt is folded into one-shot entry instructions carried on + // the PendingMove; no hand-off message is pre-queued at defer time — + // instructions ride the target-step entry overlay applied at turn-end. + require.NotNil(t, queue.pendingMoves[0].EntryOptions) + assert.Equal(t, "continue the work", queue.pendingMoves[0].EntryOptions.Instructions) + assert.Empty(t, queue.calls) } func TestMoveTaskErrorMessage_SanitizesClassifiedErrors(t *testing.T) { diff --git a/apps/backend/internal/mcp/handlers/config_task_handlers.go b/apps/backend/internal/mcp/handlers/config_task_handlers.go index 3818c88956..d5735bd7cc 100644 --- a/apps/backend/internal/mcp/handlers/config_task_handlers.go +++ b/apps/backend/internal/mcp/handlers/config_task_handlers.go @@ -17,20 +17,35 @@ import ( taskrepo "github.com/kandev/kandev/internal/task/repository/sqlite" "github.com/kandev/kandev/internal/task/service" wfmodels "github.com/kandev/kandev/internal/workflow/models" + workflowmove "github.com/kandev/kandev/internal/workflow/move" v1 "github.com/kandev/kandev/pkg/api/v1" ws "github.com/kandev/kandev/pkg/websocket" "go.uber.org/zap" ) +// moveTaskRequest is the parsed move_task_kandev payload. Prompt is a legacy +// alias for entry_options.instructions: it is folded into EntryOptions during +// normalization and never used directly by the move paths afterward. +type moveTaskRequest struct { + TaskID string `json:"task_id"` + WorkflowID string `json:"workflow_id"` + WorkflowStepID string `json:"workflow_step_id"` + Position int `json:"position"` + Prompt string `json:"prompt"` + SenderSessionID string `json:"sender_session_id"` + EntryOptions *workflowmove.EntryOptions `json:"entry_options,omitempty"` +} + +const ( + // moveDispositionApplied marks an MCP move that committed immediately. + moveDispositionApplied = "applied" + // moveDispositionDeferred marks an MCP move recorded to run at the source + // session's turn-end. + moveDispositionDeferred = "deferred" +) + func (h *Handlers) handleMoveTask(ctx context.Context, msg *ws.Message) (*ws.Message, error) { - var req struct { - TaskID string `json:"task_id"` - WorkflowID string `json:"workflow_id"` - WorkflowStepID string `json:"workflow_step_id"` - Position int `json:"position"` - Prompt string `json:"prompt"` - SenderSessionID string `json:"sender_session_id"` - } + var req moveTaskRequest if err := json.Unmarshal(msg.Payload, &req); err != nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil) } @@ -44,10 +59,21 @@ func (h *Handlers) handleMoveTask(ctx context.Context, msg *ws.Message) (*ws.Mes return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "workflow_step_id is required", nil) } - // Prompt is OPTIONAL — config-mode/admin moves don't always have an agent - // to hand off to. When supplied, it activates the deferred-move path that - // hands the receiving agent a directive on its first turn at the new step. - // When omitted, we just move the task and return. + // Fold the legacy top-level prompt into entry_options.instructions and trim + // all fields. Supplying both a legacy prompt and a nested instructions value + // is rejected. The normalized options are the single request shape used by + // both the deferred and immediate paths below. + entryOptions, err := workflowmove.NormalizeEntryOptions(req.EntryOptions, req.Prompt) + if err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil) + } + req.EntryOptions = entryOptions + req.Prompt = "" + + // entry_options are OPTIONAL — config-mode/admin moves don't always have an + // agent to hand off to. When supplied, one-shot instructions/reset/profile + // are applied when the task enters the target step. When omitted, we just + // move the task and return. session, lookupErr := h.lookupSession(ctx, req.TaskID) if lookupErr != nil { // Backend lookup failure is an internal error, not validation — don't @@ -81,14 +107,7 @@ func (h *Handlers) handleMoveTask(ctx context.Context, msg *ws.Message) (*ws.Mes func (h *Handlers) deferMoveTask( ctx context.Context, msg *ws.Message, - req struct { - TaskID string `json:"task_id"` - WorkflowID string `json:"workflow_id"` - WorkflowStepID string `json:"workflow_step_id"` - Position int `json:"position"` - Prompt string `json:"prompt"` - SenderSessionID string `json:"sender_session_id"` - }, + req moveTaskRequest, session *models.TaskSession, ) (*ws.Message, error) { if h.messageQueue == nil { @@ -154,21 +173,18 @@ func (h *Handlers) deferMoveTask( return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "target workflow is in a different workspace", nil) } - } - - moveID := uuid.NewString() - var handoff *queuedMoveTaskPrompt - if req.Prompt != "" { - wrapped := "You were moved to this step with the following message: " + req.Prompt - var err error - handoff, err = h.queueMoveTaskPromptWithMoveID(ctx, queueIdentityForSession(session), wrapped, moveID) - if err != nil { - h.logger.Error("move_task: failed to queue hand-off prompt", - zap.String("task_id", req.TaskID), zap.String("session_id", session.ID), zap.Error(err)) - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, - "failed to queue move_task hand-off prompt", nil) + // One-shot entry options require an actual workflow step change; the + // deferred path bypasses the service that otherwise enforces this. + if err := workflowmove.ValidateEntryOptions(req.EntryOptions, moveChange(task.WorkflowStepID, req.WorkflowStepID)); err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil) } } + + // The one-shot options travel on the PendingMove and are applied when the + // target step is entered at turn-end (appended to an auto-start prompt or + // queued once for an existing session). No hand-off message is pre-queued + // here; instructions ride the entry overlay. + moveID := uuid.NewString() if err := h.messageQueue.SetPendingMove(ctx, session.ID, &messagequeue.PendingMove{ MoveID: moveID, SessionIncarnationID: session.QueueIncarnationID, @@ -178,15 +194,28 @@ func (h *Handlers) deferMoveTask( Position: req.Position, Actor: string(wfmodels.StepTransitionActorAgent), SenderSessionID: req.SenderSessionID, + EntryOptions: req.EntryOptions, }); err != nil { - h.rollbackMoveTaskPrompt(ctx, handoff) h.logger.Error("move_task: failed to persist deferred move", zap.String("task_id", req.TaskID), zap.String("session_id", session.ID), zap.Error(err)) return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "failed to persist deferred move", nil) } - return ws.NewResponse(msg.ID, msg.Action, - h.synthesizeMovedTaskDTO(ctx, req.TaskID, req.WorkflowID, req.WorkflowStepID, req.Position)) + return ws.NewResponse(msg.ID, msg.Action, dto.MoveTaskResponse{ + Task: h.synthesizeMovedTaskDTO(ctx, req.TaskID, req.WorkflowID, req.WorkflowStepID, req.Position), + MoveID: moveID, + EntryOptions: req.EntryOptions, + Disposition: moveDispositionDeferred, + }) +} + +// moveChange classifies a move as a workflow step change or position-only based +// on the task's current and target step IDs. +func moveChange(fromStepID, toStepID string) workflowmove.MoveChange { + if fromStepID != toStepID { + return workflowmove.MoveChangeStep + } + return workflowmove.MoveChangePositionOnly } // applyMoveTaskImmediate runs the move now, optionally queueing a hand-off @@ -196,29 +225,9 @@ func (h *Handlers) deferMoveTask( func (h *Handlers) applyMoveTaskImmediate( ctx context.Context, msg *ws.Message, - req struct { - TaskID string `json:"task_id"` - WorkflowID string `json:"workflow_id"` - WorkflowStepID string `json:"workflow_step_id"` - Position int `json:"position"` - Prompt string `json:"prompt"` - SenderSessionID string `json:"sender_session_id"` - }, + req moveTaskRequest, session *models.TaskSession, ) (*ws.Message, error) { - var handoff *queuedMoveTaskPrompt - if req.Prompt != "" && session != nil { - wrapped := "You were moved to this step with the following message: " + req.Prompt - var err error - handoff, err = h.queueMoveTaskPrompt(ctx, queueIdentityForSession(session), wrapped) - if err != nil { - h.logger.Error("move_task: failed to queue hand-off prompt for idle session", - zap.String("task_id", req.TaskID), zap.String("session_id", session.ID), zap.Error(err)) - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, - "failed to queue move_task hand-off prompt", nil) - } - } - // Attribution uses the CALLING session (req.SenderSessionID, injected // server-side by the MCP server from its own bound session — see // moveTaskHandler), never the target task's session captured above: @@ -233,21 +242,41 @@ func (h *Handlers) applyMoveTaskImmediate( attribution.SessionID = req.SenderSessionID } moveCtx := steptelemetry.WithAttribution(ctx, attribution) + // One-shot options are validated (normalization, step-change requirement, + // profile preflight, recipient availability) inside MoveTaskWithOptions and + // applied when the task enters the target step. No hand-off message is + // pre-queued here; instructions ride the entry overlay. result, err := h.taskSvc.MoveTaskWithOptions(moveCtx, req.TaskID, req.WorkflowID, req.WorkflowStepID, req.Position, - service.MoveTaskOptions{StepHistoryActor: wfmodels.StepTransitionActorAgent}) + service.MoveTaskOptions{StepHistoryActor: wfmodels.StepTransitionActorAgent, EntryOptions: req.EntryOptions}) if err != nil { - // Without the transition, its handoff must not reach a later turn. - h.rollbackMoveTaskPrompt(ctx, handoff) h.logger.Error("failed to move task", zap.Error(err)) return ws.NewError(msg.ID, msg.Action, classifyMoveTaskError(err), moveTaskErrorMessage(err), nil) } - return ws.NewResponse(msg.ID, msg.Action, dto.FromTask(result.Task)) + response := dto.MoveTaskResponse{ + Task: dto.FromTask(result.Task), + MoveID: result.MoveID, + EntryOptions: result.EntryOptions, + Disposition: moveDispositionApplied, + } + if result.WorkflowStep != nil { + response.WorkflowStep = dto.FromWorkflowStep(result.WorkflowStep) + } + return ws.NewResponse(msg.ID, msg.Action, response) } func classifyMoveTaskError(err error) string { if err == nil { return ws.ErrorCodeInternalError } + if errors.Is(err, workflowmove.ErrMoveConflict) { + return ws.ErrorCodeConflict + } + if errors.Is(err, workflowmove.ErrConflictingInstructions) || + errors.Is(err, workflowmove.ErrEntryOptionsRequireStepChange) || + errors.Is(err, workflowmove.ErrEntryOptionsUnsupported) || + errors.Is(err, workflowmove.ErrEntryTargetUnavailable) { + return ws.ErrorCodeValidation + } msg := strings.ToLower(err.Error()) switch { case strings.Contains(msg, "wip limit exceeded"), @@ -280,17 +309,17 @@ func moveTaskErrorMessage(err error) string { // sees a "successful move" response shape, freeing it to end the turn (which // is what triggers applyPendingMove). If we can't load the task, fall back to // a minimal map so the call still resolves. -func (h *Handlers) synthesizeMovedTaskDTO(ctx context.Context, taskID, workflowID, workflowStepID string, position int) any { +func (h *Handlers) synthesizeMovedTaskDTO(ctx context.Context, taskID, workflowID, workflowStepID string, position int) dto.TaskDTO { task, err := h.taskSvc.GetTask(ctx, taskID) if err != nil || task == nil { h.logger.Warn("failed to load task for synthetic move response", zap.String("task_id", taskID), zap.Error(err)) - return map[string]any{ - "id": taskID, - "workflow_id": workflowID, - "workflow_step_id": workflowStepID, - "position": position, + return dto.TaskDTO{ + ID: taskID, + WorkflowID: workflowID, + WorkflowStepID: workflowStepID, + Position: position, } } clone := *task @@ -328,14 +357,6 @@ type queuedMoveTaskPrompt struct { entryID string } -func queueIdentityForSession(session *models.TaskSession) messagequeue.QueueSessionIdentity { - return messagequeue.QueueSessionIdentity{ - TaskID: session.TaskID, - SessionID: session.ID, - SessionIncarnationID: session.QueueIncarnationID, - } -} - // queueMoveTaskPrompt enqueues a handoff for one exact session incarnation and // returns the entry identity needed to undo only that handoff. func (h *Handlers) queueMoveTaskPrompt( @@ -391,24 +412,6 @@ func (h *Handlers) queueMoveTaskPromptWithMoveID( return &queuedMoveTaskPrompt{identity: identity, entryID: entry.ID}, nil } -func (h *Handlers) rollbackMoveTaskPrompt(ctx context.Context, handoff *queuedMoveTaskPrompt) { - if handoff == nil || h.messageQueue == nil { - return - } - rollbackCtx := context.WithoutCancel(ctx) - if _, err := h.messageQueue.RemoveEntryForSession(rollbackCtx, handoff.identity, handoff.entryID); err != nil { - h.logger.Error("move_task: failed to roll back queued hand-off prompt", - zap.String("task_id", handoff.identity.TaskID), - zap.String("session_id", handoff.identity.SessionID), - zap.String("entry_id", handoff.entryID), - zap.Error(err)) - return - } - if queue, ok := h.messageQueue.(*messagequeue.Service); ok { - h.publishQueueStatusEvent(rollbackCtx, handoff.identity, queue) - } -} - func (h *Handlers) handleDeleteTask(ctx context.Context, msg *ws.Message) (*ws.Message, error) { taskID, err := unmarshalStringField(msg.Payload, "task_id") if err != nil { diff --git a/apps/backend/internal/mcp/handlers/config_task_handlers_envelope_test.go b/apps/backend/internal/mcp/handlers/config_task_handlers_envelope_test.go new file mode 100644 index 0000000000..baf79d258b --- /dev/null +++ b/apps/backend/internal/mcp/handlers/config_task_handlers_envelope_test.go @@ -0,0 +1,101 @@ +package handlers + +// Move-result envelope coverage for the MCP move_task_kandev handler +// (codex thread 3936395919): both the immediate and deferred paths must return +// a dto.MoveTaskResponse carrying move_id, normalized entry_options, and a +// disposition so an agent can distinguish deferred acceptance from an immediate +// move and correlate the retained one-shot options with the eventual entry. + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/kandev/kandev/internal/task/dto" + "github.com/kandev/kandev/internal/task/models" + v1 "github.com/kandev/kandev/pkg/api/v1" + ws "github.com/kandev/kandev/pkg/websocket" +) + +// TestHandleMoveTask_ImmediateReturnsMoveResultEnvelope asserts the idle path +// returns an "applied" envelope with the moved task, rather than a bare task DTO. +func TestHandleMoveTask_ImmediateReturnsMoveResultEnvelope(t *testing.T) { + svc, repo := newTestTaskService(t) + ctx := context.Background() + now := time.Now().UTC() + + require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-env", Name: "Env", CreatedAt: now, UpdatedAt: now})) + require.NoError(t, repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-env", WorkspaceID: "ws-env", Name: "Board", CreatedAt: now, UpdatedAt: now})) + require.NoError(t, repo.CreateTask(ctx, &models.Task{ + ID: "task-env", WorkspaceID: "ws-env", WorkflowID: "wf-env", + WorkflowStepID: "step-work", Title: "Move me", State: v1.TaskStateInProgress, + CreatedAt: now, UpdatedAt: now, + })) + + h := &Handlers{taskSvc: svc, logger: testLogger(t).WithFields()} + msg := makeWSMessage(t, ws.ActionMCPMoveTask, map[string]interface{}{ + "task_id": "task-env", "workflow_id": "wf-env", "workflow_step_id": "step-done", "position": 0, + }) + + resp, err := h.handleMoveTask(ctx, msg) + require.NoError(t, err) + require.NotEqual(t, ws.MessageTypeError, resp.Type, "payload: %s", string(resp.Payload)) + + var env dto.MoveTaskResponse + require.NoError(t, json.Unmarshal(resp.Payload, &env)) + assert.Equal(t, moveDispositionApplied, env.Disposition) + assert.Equal(t, "task-env", env.Task.ID) + assert.Equal(t, "step-done", env.Task.WorkflowStepID) + assert.Empty(t, env.MoveID, "an option-less move mints no move_id") + assert.Nil(t, env.EntryOptions) +} + +// TestHandleMoveTask_DeferredReturnsMoveResultEnvelope asserts the deferred +// (active-session) path returns a "deferred" envelope whose move_id matches the +// recorded PendingMove and whose entry_options echo the accepted one-shot +// override, so the agent can correlate the retained options with the eventual +// step entry. +func TestHandleMoveTask_DeferredReturnsMoveResultEnvelope(t *testing.T) { + svc, repo := newTestTaskService(t) + ctx := context.Background() + now := time.Now().UTC() + + require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-envd", Name: "Env", CreatedAt: now, UpdatedAt: now})) + require.NoError(t, repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-envd", WorkspaceID: "ws-envd", Name: "Board", CreatedAt: now, UpdatedAt: now})) + require.NoError(t, repo.CreateTask(ctx, &models.Task{ + ID: "task-envd", WorkspaceID: "ws-envd", WorkflowID: "wf-envd", + WorkflowStepID: "step-work", Title: "Move me", State: v1.TaskStateInProgress, + CreatedAt: now, UpdatedAt: now, + })) + require.NoError(t, repo.CreateTaskSession(ctx, &models.TaskSession{ + ID: "sess-envd", TaskID: "task-envd", State: models.TaskSessionStateRunning, + IsPrimary: true, StartedAt: now, UpdatedAt: now, + })) + + queue := &pendingMoveRecordingQueuer{} + h := &Handlers{taskSvc: svc, messageQueue: queue, logger: testLogger(t).WithFields()} + msg := makeWSMessage(t, ws.ActionMCPMoveTask, map[string]interface{}{ + "task_id": "task-envd", "workflow_id": "wf-envd", "workflow_step_id": "step-done", "position": 0, + "entry_options": map[string]interface{}{"reset_context": true}, + }) + + resp, err := h.handleMoveTask(ctx, msg) + require.NoError(t, err) + require.NotEqual(t, ws.MessageTypeError, resp.Type, "payload: %s", string(resp.Payload)) + require.Len(t, queue.pendingMoves, 1) + + var env dto.MoveTaskResponse + require.NoError(t, json.Unmarshal(resp.Payload, &env)) + assert.Equal(t, moveDispositionDeferred, env.Disposition) + assert.Equal(t, "task-envd", env.Task.ID) + assert.Equal(t, "step-done", env.Task.WorkflowStepID) + assert.NotEmpty(t, env.MoveID, "a deferred optioned move must expose its move_id") + assert.Equal(t, queue.pendingMoves[0].MoveID, env.MoveID, + "envelope move_id must correlate with the recorded PendingMove") + require.NotNil(t, env.EntryOptions) + assert.True(t, env.EntryOptions.ResetContext, "accepted one-shot options must be echoed back") +} diff --git a/apps/backend/internal/mcp/server/config_handlers.go b/apps/backend/internal/mcp/server/config_handlers.go index 103459eec8..bbbd2614e8 100644 --- a/apps/backend/internal/mcp/server/config_handlers.go +++ b/apps/backend/internal/mcp/server/config_handlers.go @@ -362,12 +362,13 @@ func (s *Server) registerConfigTaskTools() { ) s.mcpServer.AddTool( mcp.NewTool("move_task_kandev", - mcp.WithDescription("Move a task to a different workflow step. When the source session is mid-turn (RUNNING), the move is deferred to turn-end automatically — prompt is optional (use it for cross-agent hand-offs). Idle-session and admin moves apply immediately."), + mcp.WithDescription(`Move a task to a different workflow step. When the source session is mid-turn (RUNNING), the move is deferred to turn-end automatically — prompt is optional (use it for cross-agent hand-offs). Idle-session and admin moves apply immediately. Returns a move-result envelope: "disposition" is "applied" when the move committed immediately or "deferred" when it was recorded for turn-end, "task" is the moved (or target-step) task, and an optioned move also returns "move_id" and the accepted "entry_options" so you can correlate the one-shot override with the eventual entry.`), mcp.WithString("task_id", mcp.Required(), mcp.Description("The task ID")), mcp.WithString("workflow_id", mcp.Required(), mcp.Description("Target workflow ID")), mcp.WithString("workflow_step_id", mcp.Required(), mcp.Description("Target workflow step ID")), mcp.WithNumber("position", mcp.Description("Position within the step (0-based)")), - mcp.WithString("prompt", mcp.Description("Optional hand-off message for the receiving agent at the new step. Mid-turn moves are always deferred; include a prompt when the next agent needs context (e.g. QA → review). Omit for self-moves like Work → Done.")), + mcp.WithString("prompt", mcp.Description("Legacy alias for entry_options.instructions. Optional hand-off message applied once when the task enters the new step. Supplying both this and entry_options.instructions is rejected. Omit for self-moves like Work → Done.")), + moveTaskEntryOptionsToolOption(), ), s.wrapHandler("move_task_kandev", s.moveTaskHandler()), ) @@ -700,6 +701,11 @@ func (s *Server) moveTaskHandler() server.ToolHandlerFunc { if args := req.GetArguments(); args["position"] != nil { payload["position"] = args["position"] } + // entry_options is a nested object; forward it verbatim for the backend + // to normalize (fold the legacy prompt alias) and validate. + if args := req.GetArguments(); args["entry_options"] != nil { + payload["entry_options"] = args["entry_options"] + } return s.forwardToBackend(ctx, ws.ActionMCPMoveTask, payload) } } diff --git a/apps/backend/internal/mcp/server/config_handlers_test.go b/apps/backend/internal/mcp/server/config_handlers_test.go index e64b9fcade..408d83f769 100644 --- a/apps/backend/internal/mcp/server/config_handlers_test.go +++ b/apps/backend/internal/mcp/server/config_handlers_test.go @@ -77,6 +77,26 @@ func toolInputProperties(t *testing.T, s *Server, toolName string) map[string]in return props } +func TestMoveTaskToolSchemasExposeEntryOptions(t *testing.T) { + for name, server := range map[string]*Server{ + "task": newTaskModeServer(t, &testBackend{}, "task-current"), + "config": newTestServer(t, &testBackend{}), + } { + props := toolInputProperties(t, server, "move_task_kandev") + entryOptions, ok := props["entry_options"].(map[string]interface{}) + if !ok { + t.Fatalf("%s move_task schema must expose entry_options as an object", name) + } + nested, ok := entryOptions["properties"].(map[string]interface{}) + if !ok { + t.Fatalf("%s entry_options schema must expose nested properties", name) + } + for _, field := range []string{"reset_context", "instructions", "skip_step_prompt"} { + assert.Contains(t, nested, field, "%s entry_options schema must expose %s", name, field) + } + } +} + // --- Action constant tests --- func TestActionConstants_MatchWebSocketActions(t *testing.T) { diff --git a/apps/backend/internal/mcp/server/handlers.go b/apps/backend/internal/mcp/server/handlers.go index 6798d80b71..a8a744e3c6 100644 --- a/apps/backend/internal/mcp/server/handlers.go +++ b/apps/backend/internal/mcp/server/handlers.go @@ -26,6 +26,7 @@ const ( promptArg = "prompt" questionsArg = "questions" optionsArg = "options" + instructionsArg = "instructions" idArg = "id" titleArg = "title" labelArg = "label" @@ -43,8 +44,29 @@ const ( reqKey = "required" typeKey = "type" stringType = "string" + agentProfileIDArg = "agent_profile_id" ) +func moveTaskEntryOptionsToolOption() mcp.ToolOption { + return mcp.WithObject("entry_options", + mcp.Description("One-shot overrides applied only when the task enters the target step; they never change durable step configuration and require an actual workflow step change."), + mcp.Properties(map[string]any{ + "reset_context": map[string]any{ + typeKey: "boolean", + descriptionArg: "Reset the target session's agent context before the step's on_enter actions run.", + }, + instructionsArg: map[string]any{ + typeKey: stringType, + descriptionArg: "One-time instructions appended to the target step's prompt (never replacing it) for this entry only.", + }, + "skip_step_prompt": map[string]any{ + typeKey: "boolean", + descriptionArg: "Suppress the destination step's configured prompt (and its task-description fallback) for this one entry. With instructions the agent auto-starts a turn carrying only those instructions; without instructions no turn starts and the task lands idle.", + }, + }), + ) +} + func (s *Server) listWorkspacesHandler() server.ToolHandlerFunc { return func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { // Backend returns {workspaces: [...], total: N} @@ -180,7 +202,7 @@ func (s *Server) createTaskHandler() server.ToolHandlerFunc { "title": title, "description": req.GetString("prompt", ""), autopilotArg: req.GetBool(autopilotArg, false), - "agent_profile_id": req.GetString("agent_profile_id", ""), + agentProfileIDArg: req.GetString(agentProfileIDArg, ""), "executor_profile_id": req.GetString("executor_profile_id", ""), "source_task_id": s.taskID, "start_agent": startAgent, @@ -528,7 +550,7 @@ func (s *Server) spawnSessionHandler() server.ToolHandlerFunc { "sender_task_id": s.taskID, "sender_session_id": s.sessionID, } - copyOptionalStringArg(payload, req, "agent_profile_id") + copyOptionalStringArg(payload, req, agentProfileIDArg) copyOptionalStringArg(payload, req, "name") var result map[string]interface{} if err := s.backend.RequestPayload(ctx, ws.ActionMCPSpawnSession, payload, &result); err != nil { diff --git a/apps/backend/internal/mcp/server/server.go b/apps/backend/internal/mcp/server/server.go index 2eb0b957c2..7e99c688e2 100644 --- a/apps/backend/internal/mcp/server/server.go +++ b/apps/backend/internal/mcp/server/server.go @@ -1261,12 +1261,13 @@ func (s *Server) registerKanbanTools() { ) s.mcpServer.AddTool( mcp.NewTool("move_task_kandev", - mcp.WithDescription("Move a task to a different workflow step. When the source session is mid-turn (RUNNING), the move is deferred to turn-end automatically — prompt is optional (use it for cross-agent hand-offs). Idle-session and admin moves apply immediately."), + mcp.WithDescription(`Move a task to a different workflow step. When the source session is mid-turn (RUNNING), the move is deferred to turn-end automatically — prompt is optional (use it for cross-agent hand-offs). Idle-session and admin moves apply immediately. Returns a move-result envelope: "disposition" is "applied" when the move committed immediately or "deferred" when it was recorded for turn-end, "task" is the moved (or target-step) task, and an optioned move also returns "move_id" and the accepted "entry_options" so you can correlate the one-shot override with the eventual entry.`), mcp.WithString("task_id", mcp.Required(), mcp.Description("The task ID")), mcp.WithString("workflow_id", mcp.Required(), mcp.Description("Target workflow ID")), mcp.WithString("workflow_step_id", mcp.Required(), mcp.Description("Target workflow step ID")), mcp.WithNumber("position", mcp.Description("Position within the step (0-based)")), mcp.WithString("prompt", mcp.Description("Optional hand-off message for the receiving agent at the new step. Mid-turn moves are always deferred; include a prompt when the next agent needs context (e.g. QA → review). Omit for self-moves like Work → Done.")), + moveTaskEntryOptionsToolOption(), ), s.wrapHandler("move_task_kandev", s.moveTaskHandler()), ) diff --git a/apps/backend/internal/office/service/prompt_builder.go b/apps/backend/internal/office/service/prompt_builder.go index f7df0a79bb..543ad0a018 100644 --- a/apps/backend/internal/office/service/prompt_builder.go +++ b/apps/backend/internal/office/service/prompt_builder.go @@ -12,10 +12,23 @@ import ( v1 "github.com/kandev/kandev/pkg/api/v1" ) +// RunPayloadOneTimeInstructionsKey names the run-payload field carrying a +// workflow move's one-shot instructions. The orchestrator's office auto-start +// path writes it (officeRunPayloadOneTimeInstructionsKey); the two packages +// intentionally avoid importing each other, so the key is duplicated as a +// literal on both sides. +const RunPayloadOneTimeInstructionsKey = "one_time_instructions" + // PromptContext holds the data needed to build a run prompt. type PromptContext struct { Reason string + // OneTimeInstructions carries a workflow move's one-shot instructions for + // this run only. When set, BuildPrompt appends it after the per-reason + // prompt body. Office runs build their prompt from a template rather than + // the durable step, so move instructions ride on the run payload. + OneTimeInstructions string + // Task fields TaskID string TaskIdentifier string @@ -113,10 +126,21 @@ func BuildPrompt(pc *PromptContext) string { default: prompt = fmt.Sprintf("You have been woken for reason: %s.", pc.Reason) } + prompt = appendOneTimeInstructions(prompt, pc.OneTimeInstructions) prompt = appendHandoffSection(prompt, pc.HandoffContext) return appendRuntimeContext(prompt, pc) } +// appendOneTimeInstructions appends a workflow move's one-shot instructions to +// the run prompt. Empty or whitespace-only input leaves the prompt unchanged. +func appendOneTimeInstructions(prompt, instructions string) string { + instructions = strings.TrimSpace(instructions) + if instructions == "" { + return prompt + } + return prompt + "\n\n## One-time workflow move instructions\n\n" + instructions +} + // appendHandoffSection renders the office task-handoffs context block // (Related tasks / Documents available / Workspace) onto every run // prompt that carries a HandoffContext. Document bodies are diff --git a/apps/backend/internal/office/service/prompt_builder_test.go b/apps/backend/internal/office/service/prompt_builder_test.go index 6536812d61..8a3e3d1e87 100644 --- a/apps/backend/internal/office/service/prompt_builder_test.go +++ b/apps/backend/internal/office/service/prompt_builder_test.go @@ -600,3 +600,34 @@ func TestBuildPrompt_WorkNoFeedback(t *testing.T) { t.Errorf("no-feedback work prompt should not mention reviewers:\n%s", prompt) } } + +func TestBuildPrompt_AppendsOneTimeMoveInstructions(t *testing.T) { + pc := &service.PromptContext{ + Reason: service.RunReasonTaskAssigned, + TaskIdentifier: "KAN-9", + TaskTitle: "Ship it", + OneTimeInstructions: "only touch the migration file", + } + prompt := service.BuildPrompt(pc) + + if !strings.Contains(prompt, "One-time workflow move instructions") { + t.Errorf("prompt missing one-time instructions heading:\n%s", prompt) + } + if !strings.Contains(prompt, "only touch the migration file") { + t.Errorf("prompt missing one-time instructions body:\n%s", prompt) + } +} + +func TestBuildPrompt_OmitsEmptyOneTimeInstructions(t *testing.T) { + pc := &service.PromptContext{ + Reason: service.RunReasonTaskAssigned, + TaskIdentifier: "KAN-9", + TaskTitle: "Ship it", + OneTimeInstructions: " ", + } + prompt := service.BuildPrompt(pc) + + if strings.Contains(prompt, "One-time workflow move instructions") { + t.Errorf("whitespace-only instructions must not add a section:\n%s", prompt) + } +} diff --git a/apps/backend/internal/office/service/scheduler_integration.go b/apps/backend/internal/office/service/scheduler_integration.go index ff148143cb..efd72d638b 100644 --- a/apps/backend/internal/office/service/scheduler_integration.go +++ b/apps/backend/internal/office/service/scheduler_integration.go @@ -918,6 +918,7 @@ func (si *SchedulerIntegration) buildPromptContext( ) *PromptContext { parsed := ParseRunPayload(payload) pc := &PromptContext{Reason: reason} + pc.OneTimeInstructions = parsed[RunPayloadOneTimeInstructionsKey] if taskID := parsed["task_id"]; taskID != "" { si.enrichTaskContext(ctx, pc, taskID) diff --git a/apps/backend/internal/orchestrator/event_handlers_workflow.go b/apps/backend/internal/orchestrator/event_handlers_workflow.go index 43ed15f1f1..2d2f6c5600 100644 --- a/apps/backend/internal/orchestrator/event_handlers_workflow.go +++ b/apps/backend/internal/orchestrator/event_handlers_workflow.go @@ -28,10 +28,16 @@ import ( workflowadapters "github.com/kandev/kandev/internal/workflow/adapters" "github.com/kandev/kandev/internal/workflow/engine" wfmodels "github.com/kandev/kandev/internal/workflow/models" + workflowmove "github.com/kandev/kandev/internal/workflow/move" "github.com/kandev/kandev/internal/workflow/stepentry" v1 "github.com/kandev/kandev/pkg/api/v1" ) +// workflowMoveMarkerOptionsKey is the sub-key under the transient +// workflow_move_pending task marker that stores the encoded one-shot entry +// options. It must match the writer in task/service (workflowMoveOptionsKey). +const workflowMoveMarkerOptionsKey = "options" + type turnCompletionCause string var ( @@ -718,12 +724,70 @@ func (s *Service) handleTaskMoved(ctx context.Context, data watcher.TaskMovedEve } if prerequisites != nil && prerequisites.session != nil { - s.handleTaskMovedWithLoadedSession(ctx, data, prerequisites.session, prerequisites.fromStep, prerequisites.targetStep, manualBarrier, queuePromotionToken) + // A direct move can carry one-shot entry options on the transient + // marker. Overlay them onto the (pre-loaded) target step so the ordinary + // on_enter path applies the reset, profile, and appended instructions to + // the copy; the durable step is never mutated. Queued (not yet admitted) + // entries defer consumption until promotion, so skip them here. + targetStep := prerequisites.targetStep + if !queued { + if moveOptions := s.workflowMovePendingOptions(task); moveOptions != nil && targetStep != nil { + targetStep = workflowmove.OverlayStep(targetStep, moveOptions) + s.clearWorkflowMovePending(ctx, task.ID) + } + } + s.handleTaskMovedWithLoadedSession(ctx, data, prerequisites.session, prerequisites.fromStep, targetStep, manualBarrier, queuePromotionToken) return } s.handleTaskMovedWithBarrier(ctx, data, manualBarrier, queuePromotionToken) } +// workflowMovePendingOptions decodes the one-shot entry options carried on a +// task's transient workflow_move_pending marker. It returns nil when no marker +// is present or the payload is empty or undecodable (an ordinary move). +func (s *Service) workflowMovePendingOptions(task *models.Task) *workflowmove.EntryOptions { + if task == nil || task.Metadata == nil { + return nil + } + raw, ok := task.Metadata[models.MetaKeyWorkflowMovePending] + if !ok { + return nil + } + marker, ok := raw.(map[string]interface{}) + if !ok { + return nil + } + encoded, _ := marker[workflowMoveMarkerOptionsKey].(string) + if encoded == "" { + return nil + } + options, err := workflowmove.DecodeEntryOptionsJSON([]byte(encoded)) + if err != nil { + s.logger.Warn("failed to decode workflow move options marker", + zap.String("task_id", task.ID), zap.Error(err)) + return nil + } + return options +} + +// clearWorkflowMovePending removes the transient marker once the target entry +// has been dispatched. A missing marker is a no-op. The options are already +// captured by the caller (overlaid step or threaded value) before this runs. +func (s *Service) clearWorkflowMovePending(ctx context.Context, taskID string) { + task, err := s.repo.GetTask(ctx, taskID) + if err != nil || task == nil || task.Metadata == nil { + return + } + if _, ok := task.Metadata[models.MetaKeyWorkflowMovePending]; !ok { + return + } + delete(task.Metadata, models.MetaKeyWorkflowMovePending) + if err := s.repo.UpdateTask(ctx, task); err != nil { + s.logger.Warn("failed to clear workflow move pending marker", + zap.String("task_id", taskID), zap.Error(err)) + } +} + func (s *Service) loadTaskMovedLifecyclePrerequisites( ctx context.Context, data watcher.TaskMovedEventData, @@ -899,8 +963,21 @@ func (s *Service) handleTaskQueuePromotedWithAutoStartOnCreateClaimed(ctx contex } s.processParentChildrenCompletedForTerminalStepMove(ctx, task.ID, targetStep.ID) if session != nil { + // A WIP-queued optioned move persists its one-shot options on the + // transient marker during deferred admission for consumption here, once + // admission clears. Overlay them onto a copy of the durable step so + // the on_enter path applies the reset, profile, and appended + // instructions; the durable step is never mutated. Clear the marker so + // the options are applied exactly once and cannot strand and block a + // later optioned move (task/service rejects a new optioned move while a + // marker is present). + entryStep := targetStep + if moveOptions := s.workflowMovePendingOptions(task); moveOptions != nil { + entryStep = workflowmove.OverlayStep(targetStep, moveOptions) + s.clearWorkflowMovePending(ctx, task.ID) + } go func() { - if err := s.finalizeStepEnter(context.WithoutCancel(ctx), task.ID, session.ID, targetStep, task.Description, targetStep.HasOnEnterAction(wfmodels.OnEnterAutoStartAgent), sourceStep, data.StepTransitionID); err != nil { + if err := s.finalizeStepEnter(context.WithoutCancel(ctx), task.ID, session.ID, entryStep, task.Description, entryStep.HasOnEnterAction(wfmodels.OnEnterAutoStartAgent), sourceStep, data.StepTransitionID); err != nil { s.restoreTaskLifecycleToken(context.WithoutCancel(ctx), task.ID, models.MetaKeyQueuePromotionPending, queuePromotionToken, "task.queue_promoted") if autoStartOnCreateClaimed { s.restoreAutoStartOnCreate(context.WithoutCancel(ctx), task.ID, "task.queue_promoted") @@ -1889,8 +1966,34 @@ func (s *Service) autoStartTaskForLoadedStep(ctx context.Context, task *models.T } restoreAutoStartOnCreate := claimResult == autoStartOnCreateClaimOwned + // A direct no-session move (and the WIP-promotion path) can carry one-shot + // entry options on the transient marker. Consume it once here — before the + // office branch below — so an office task also applies (or suppresses) the + // options and the marker is never stranded (a stranded marker rejects a + // later optioned move). The profile override and appended instructions are + // threaded through the launch because the auto-start prompt is rebuilt from + // the durable step by ID; reset_context is a no-op for a brand-new session. + moveOptions := s.workflowMovePendingOptions(task) + if moveOptions != nil { + s.clearWorkflowMovePending(ctx, task.ID) + } + if moveOptions != nil && moveOptions.SkipStepPrompt && moveOptions.Instructions == "" { + // skip_step_prompt with no instructions suppresses the turn entirely. + // For a task with no session that means preparing nothing: leave it idle + // exactly as a step without auto_start_agent would, so the user starts + // the agent manually. This covers the office path too, which is why the + // suppression check precedes the office branch. + s.logger.Info(eventName+": skip_step_prompt with no instructions; leaving task idle without auto-start", + zap.String("task_id", task.ID), + zap.String("to_step_id", step.ID)) + if restoreAutoStartOnCreate { + s.discardAutoStartOnCreate(ctx, task.ID, eventName) + } + return + } + if s.isOfficeTask(ctx, task.ID) { - s.autoStartOfficeTaskForLoadedStep(ctx, task, step, eventName, restoreQueuePromotion, stepTransitionID, restoreAutoStartOnCreate) + s.autoStartOfficeTaskForLoadedStep(ctx, task, step, eventName, restoreQueuePromotion, stepTransitionID, moveOptions, restoreAutoStartOnCreate) return } @@ -1934,7 +2037,10 @@ func (s *Service) autoStartTaskForLoadedStep(ctx context.Context, task *models.T if workflowAgentProfileID != "" { startAgentProfileID = "" } - _, err := s.startTask(asyncCtx, task.ID, startAgentProfileID, executorID, executorProfileID, "", task.Description, step.ID, planMode, true, nil, startTaskOptions{WorkflowEntryID: stepTransitionID}) + _, err := s.startTask(asyncCtx, task.ID, startAgentProfileID, executorID, executorProfileID, "", task.Description, step.ID, planMode, true, nil, startTaskOptions{ + EntryOptions: moveOptions, + WorkflowEntryID: stepTransitionID, + }) if err != nil { s.logger.Error(eventName+": failed to auto-start task", zap.String("task_id", task.ID), @@ -1960,7 +2066,7 @@ func (s *Service) autoStartTaskForLoadedStep(ctx context.Context, task *models.T // a run through the engine's Office adapters instead, the same mechanism the // scheduler's recovery sweep and the "assign task" flow already use to start // Office work (internal/office/service/scheduler_recovery.go). -func (s *Service) autoStartOfficeTaskForLoadedStep(ctx context.Context, task *models.Task, step *wfmodels.WorkflowStep, eventName string, restoreQueuePromotion bool, stepTransitionID int64, restoreAutoStartOnCreate bool) { +func (s *Service) autoStartOfficeTaskForLoadedStep(ctx context.Context, task *models.Task, step *wfmodels.WorkflowStep, eventName string, restoreQueuePromotion bool, stepTransitionID int64, moveOptions *workflowmove.EntryOptions, restoreAutoStartOnCreate bool) { queuePromotionToken := queuePromotionLifecycleToken(task) // Async for the same reason as the kanban path above: the event bus // delivers synchronously and blocking here would stall the HTTP handler @@ -1977,7 +2083,7 @@ func (s *Service) autoStartOfficeTaskForLoadedStep(ctx context.Context, task *mo } } - outcome, err := s.queueOfficeAutoStartRun(asyncCtx, task, step, stepTransitionID) + outcome, err := s.queueOfficeAutoStartRun(asyncCtx, task, step, stepTransitionID, moveOptions) if err != nil { s.logger.Error(eventName+": failed to queue office auto-start run", zap.String("task_id", task.ID), @@ -2023,11 +2129,19 @@ func (s *Service) autoStartOfficeTaskForLoadedStep(ctx context.Context, task *mo // shared reason string is duplicated rather than pulled across the boundary. const officeAutoStartRunReason = "task_assigned" +// officeRunPayloadOneTimeInstructionsKey names the run-payload field carrying a +// move's one-shot instructions into an office run. It must match the reader in +// office/service (officeservice.RunPayloadOneTimeInstructionsKey); the two +// packages intentionally avoid importing each other, so the key is duplicated +// as a literal rather than shared across the boundary (same rationale as +// officeAutoStartRunReason above). +const officeRunPayloadOneTimeInstructionsKey = "one_time_instructions" + // queueOfficeAutoStartRun makes auto_start_agent Office-aware: instead of // StartTask (kanban-only), it resolves an agent the same way a "primary" // queue_run target would — preferring the task's current runner participant, // falling back to its assignee — and queues a run through engineRunQueue. -func (s *Service) queueOfficeAutoStartRun(ctx context.Context, task *models.Task, step *wfmodels.WorkflowStep, stepTransitionID int64) (engine.QueueOutcome, error) { +func (s *Service) queueOfficeAutoStartRun(ctx context.Context, task *models.Task, step *wfmodels.WorkflowStep, stepTransitionID int64, moveOptions *workflowmove.EntryOptions) (engine.QueueOutcome, error) { if s.engineRunQueue == nil { return "", fmt.Errorf("office run queue is not wired") } @@ -2044,13 +2158,23 @@ func (s *Service) queueOfficeAutoStartRun(ctx context.Context, task *models.Task if agentProfileID == "" { return "", fmt.Errorf("no agent profile resolved for office auto-start on task %s", task.ID) } + payload := map[string]any{metaKeyTaskID: task.ID} + // An office run builds its prompt from a per-reason template, not the + // durable step prompt, so a move's one-shot instructions ride on the run + // payload and are appended by the office prompt builder (BuildPrompt reads + // officeRunPayloadOneTimeInstructionsKey). reset_context is a no-op for a + // fresh office run, and skip_step_prompt with no instructions was already + // suppressed by the caller before this path was reached. + if moveOptions != nil && moveOptions.Instructions != "" { + payload[officeRunPayloadOneTimeInstructionsKey] = moveOptions.Instructions + } return s.engineRunQueue.QueueRun(ctx, engine.QueueRunRequest{ AgentProfileID: agentProfileID, TaskID: task.ID, WorkflowStepID: step.ID, Reason: officeAutoStartRunReason, IdempotencyKey: officeAutoStartIdempotencyKey(task, agentProfileID, step.ID, stepTransitionID), - Payload: map[string]any{"task_id": task.ID}, + Payload: payload, }) } @@ -3859,6 +3983,7 @@ func (s *Service) processOnEnter(ctx context.Context, taskID string, session *mo if session.State == models.TaskSessionStateRunning || session.State == models.TaskSessionStateStarting { return } + s.queueMoveInstructionsForSession(ctx, taskID, sessionID, step) s.setSessionWaitingForInput(ctx, taskID, sessionID, session) s.publishSessionWaitingEvent(ctx, taskID, sessionID, step.ID, session) // This step entry never reaches buildWorkflowEntryPrompt or @@ -3871,12 +3996,22 @@ func (s *Service) processOnEnter(ctx context.Context, taskID string, session *mo // Process reset_agent_context FIRST — must complete before auto_start_agent. // Context reset works for both ACP and passthrough sessions. if step.HasOnEnterAction(wfmodels.OnEnterResetAgentContext) { + // A CREATED session has no prior conversation, so resetAgentContext + // skips the actual reset and no "Context reset" divider should show. + // Capture this before the call, which may flip session.State. + hadConversation := session.State != models.TaskSessionStateCreated if !s.resetAgentContext(ctx, taskID, session, step.Name) { s.setSessionWaitingForInput(ctx, taskID, sessionID, session) s.publishSessionWaitingEvent(ctx, taskID, sessionID, step.ID, session) return } s.markIdleAfterReset(ctx, taskID, sessionID, session, step, isPassthrough) + // Mirror the manual reset path so a workflow-driven reset (durable + // reset_agent_context action or a one-time move override) shows the + // same chat divider as the toolbar reset button. + if hadConversation { + s.createContextResetMessage(ctx, taskID, sessionID) + } } // Conditional session configuration is applied after a context reset so the @@ -4086,6 +4221,10 @@ func (s *Service) launchAfterOnEnterDispatch( if session.State == models.TaskSessionStateRunning || session.State == models.TaskSessionStateStarting { return } + // An existing session that neither auto-starts nor switches profile would + // never receive the overlaid step prompt: queue the one-shot move + // instructions once so the agent's next turn picks them up. + s.queueMoveInstructionsForSession(ctx, taskID, sessionID, step) s.setSessionWaitingForInput(ctx, taskID, sessionID, session) s.publishSessionWaitingEvent(ctx, taskID, sessionID, step.ID, session) // handleAgentReady early-returns when a workflow transition occurs (#677), @@ -4157,6 +4296,27 @@ func (s *Service) handleWorkflowEntryPromptError( s.publishSessionWaitingEvent(ctx, taskID, session.ID, step.ID, session) } +// queueMoveInstructionsForSession delivers one-shot workflow-move instructions +// to an existing session that will not otherwise receive the overlaid step +// prompt (no auto-start, no profile switch). It is a no-op for ordinary steps +// (no sentinel) and when no message queue is configured. +func (s *Service) queueMoveInstructionsForSession(ctx context.Context, taskID, sessionID string, step *wfmodels.WorkflowStep) { + if step == nil || s.messageQueue == nil { + return + } + block := workflowmove.ExtractInstructions(step.Prompt) + if block == "" { + return + } + if _, err := s.messageQueue.QueueMessageWithMetadata( + ctx, sessionID, taskID, block, "", + messagequeue.QueuedByUser, false, nil, nil, + ); err != nil { + s.logger.Warn("failed to queue one-shot workflow move instructions", + zap.String("task_id", taskID), zap.String("session_id", sessionID), zap.Error(err)) + } +} + // dispatchEngineOwnedOnEnterAction executes an engine-owned on_enter action // once for this step-entry. The marker CAS prevents duplicate execution. func (s *Service) dispatchEngineOwnedOnEnterAction( @@ -4457,6 +4617,9 @@ func (s *Service) applyPendingMove(ctx context.Context, taskID, sessionID string zap.String("from_step_id", fromStepID), zap.String("to_step_id", move.WorkflowStepID)) if stored, loadErr := s.repo.GetTask(ctx, taskID); loadErr == nil && stored != nil && stored.QueuedForStepID == move.WorkflowStepID && !stored.WIPAdmitted { + // The repository persisted the one-shot marker in the same transaction + // that admitted the queued destination and consumed PendingMove. The + // source exit can now run without a second, lossy task write. go s.processStepExitForDeferredMove(context.WithoutCancel(ctx), identity, freshSession, fromStepID) return } @@ -4465,7 +4628,7 @@ func (s *Service) applyPendingMove(ctx context.Context, taskID, sessionID string taskDescription := task.Description go s.processStepExitAndEnterForDeferredMove( context.WithoutCancel(ctx), identity, freshSession, - fromStepID, move.WorkflowStepID, taskDescription, + fromStepID, move.WorkflowStepID, taskDescription, move.EntryOptions, ) } @@ -4513,6 +4676,7 @@ func (s *Service) processStepExitAndEnterForDeferredMove( identity messagequeue.QueueSessionIdentity, session *models.TaskSession, fromStepID, toStepID, taskDescription string, + entryOptions *workflowmove.EntryOptions, ) { current, err := s.messageQueue.ResolveSessionIdentity(ctx, identity.TaskID, identity.SessionID) if err != nil || current != identity || session.QueueIncarnationID != identity.SessionIncarnationID { @@ -4536,7 +4700,14 @@ func (s *Service) processStepExitAndEnterForDeferredMove( if err != nil { return } - s.processOnEnter(ctx, identity.TaskID, fresh, targetStep, taskDescription, 0, fromStep) + // Overlay one-shot move options onto a transient copy of the target step so + // the ordinary on_enter path applies the reset and appended instructions; + // the durable step is never mutated. + entryStep := targetStep + if entryOptions != nil { + entryStep = workflowmove.OverlayStep(targetStep, entryOptions) + } + s.processOnEnter(ctx, identity.TaskID, fresh, entryStep, taskDescription, 0, fromStep) } func (s *Service) removePendingMoveHandoffPromptForSession( diff --git a/apps/backend/internal/orchestrator/event_handlers_workflow_move_override_findings_test.go b/apps/backend/internal/orchestrator/event_handlers_workflow_move_override_findings_test.go new file mode 100644 index 0000000000..124b2dabf5 --- /dev/null +++ b/apps/backend/internal/orchestrator/event_handlers_workflow_move_override_findings_test.go @@ -0,0 +1,214 @@ +package orchestrator + +import ( + "context" + "testing" + "time" + + "github.com/kandev/kandev/internal/orchestrator/watcher" + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/workflow/engine" + wfmodels "github.com/kandev/kandev/internal/workflow/models" + workflowmove "github.com/kandev/kandev/internal/workflow/move" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +// encodeMoveMarker builds a workflow_move_pending marker map carrying the given +// one-shot options, in the shape the orchestrator reader (workflowMovePendingOptions) +// and the task/service writer both use. +func encodeMoveMarker(t *testing.T, fromStepID, moveID string, opts *workflowmove.EntryOptions) map[string]interface{} { + t.Helper() + encoded, err := workflowmove.EncodeEntryOptionsJSON(opts) + if err != nil { + t.Fatalf("encode entry options: %v", err) + } + return map[string]interface{}{ + "from_step_id": fromStepID, + "move_id": moveID, + "options": string(encoded), + } +} + +// waitForRestartCall polls the mock agent manager until at least one +// RestartAgentProcess call is recorded, tolerating whether the on-enter reset +// runs synchronously or on a nested goroutine. +func waitForRestartCall(t *testing.T, agentMgr *mockAgentManager) []string { + t.Helper() + deadline := time.After(2 * time.Second) + for { + agentMgr.mu.Lock() + calls := append([]string(nil), agentMgr.restartProcessCalls...) + agentMgr.mu.Unlock() + if len(calls) > 0 { + return calls + } + select { + case <-deadline: + t.Fatal("timed out waiting for reset_context overlay to restart the agent process") + case <-time.After(10 * time.Millisecond): + } + } +} + +// TestQueuePromotionExistingSessionAppliesAndClearsMoveMarker proves finding B: +// when a WIP-queued optioned move's destination is admitted while the task still +// has an active session, the promotion path must apply the one-shot options +// (here reset_context, observed as the on-enter agent-process restart) and clear +// the marker so it cannot strand and reject a later optioned move. +func TestQueuePromotionExistingSessionAppliesAndClearsMoveMarker(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + seedSession(t, repo, "promote-session", "promote-session-s1", "destination-step") + + session, err := repo.GetTaskSession(ctx, "promote-session-s1") + if err != nil { + t.Fatalf("load session: %v", err) + } + session.AgentExecutionID = "exec-promote" + if err := repo.UpdateTaskSession(ctx, session); err != nil { + t.Fatalf("update session: %v", err) + } + seedExecutorRunning(t, repo, session.ID, session.TaskID, "exec-promote") + + task, err := repo.GetTask(ctx, "promote-session") + if err != nil { + t.Fatalf("load task: %v", err) + } + task.WIPAdmitted = true + task.QueuedForStepID = "" + task.Metadata = map[string]interface{}{ + models.MetaKeyQueuePromotionPending: true, + models.MetaKeyWorkflowMovePending: encodeMoveMarker(t, "source-step", "move-1", &workflowmove.EntryOptions{ResetContext: true}), + } + if err := repo.UpdateTask(ctx, task); err != nil { + t.Fatalf("persist task: %v", err) + } + + steps := newMockStepGetter() + // The durable destination step carries NO reset action; only the marker's + // reset_context option should introduce it via the overlay. + steps.steps["destination-step"] = &wfmodels.WorkflowStep{ + ID: "destination-step", WorkflowID: "wf1", Name: "Destination", + } + agentMgr := &mockAgentManager{repoForExecutionLookup: repo} + svc := createTestServiceWithAgent(repo, steps, newMockTaskRepo(), agentMgr) + entryCompleted := make(chan struct{}) + svc.onTaskQueuePromotionEntryComplete = func() { close(entryCompleted) } + + svc.handleTaskQueuePromoted(ctx, watcher.TaskEventData{TaskID: "promote-session"}) + + select { + case <-entryCompleted: + case <-time.After(2 * time.Second): + t.Fatal("destination entry did not complete") + } + + restartCalls := waitForRestartCall(t, agentMgr) + if len(restartCalls) != 1 || restartCalls[0] != "exec-promote" { + t.Fatalf("expected reset_context overlay to restart exec-promote once, got %v", restartCalls) + } + + stored, err := repo.GetTask(ctx, "promote-session") + if err != nil { + t.Fatalf("reload task: %v", err) + } + if _, pending := stored.Metadata[models.MetaKeyWorkflowMovePending]; pending { + t.Fatal("workflow move marker must be cleared after the existing-session promotion entry") + } +} + +// seedOfficeTaskWithMarker creates an Office task (ProjectID makes IsFromOffice +// true) carrying a pending move marker with the given options. +func seedOfficeTaskWithMarker(t *testing.T, repo interface { + CreateWorkspace(context.Context, *models.Workspace) error + CreateWorkflow(context.Context, *models.Workflow) error + CreateTask(context.Context, *models.Task) error +}, taskID string, opts *workflowmove.EntryOptions) { + t.Helper() + ctx := context.Background() + now := time.Now().UTC() + requireNoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws1", Name: "Test", CreatedAt: now, UpdatedAt: now})) + requireNoError(t, repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf1", WorkspaceID: "ws1", Name: "WF", CreatedAt: now, UpdatedAt: now})) + requireNoError(t, repo.CreateTask(ctx, &models.Task{ + ID: taskID, WorkspaceID: "ws1", WorkflowID: "wf1", WorkflowStepID: "step1", + ProjectID: "proj1", Title: "Office", Description: "prompt", State: v1.TaskStateCreated, + AssigneeAgentProfileID: "assignee-profile", CreatedAt: now, UpdatedAt: now, + Metadata: map[string]interface{}{ + models.MetaKeyWorkflowMovePending: encodeMoveMarker(t, "step1", "move-1", opts), + }, + })) +} + +func officeAutoStartStep() *mockStepGetter { + steps := newMockStepGetter() + steps.steps["step2"] = &wfmodels.WorkflowStep{ + ID: "step2", WorkflowID: "wf1", Name: "Work", Position: 1, + Events: wfmodels.StepEvents{OnEnter: []wfmodels.OnEnterAction{{Type: wfmodels.OnEnterAutoStartAgent}}}, + } + return steps +} + +// TestOfficeAutoStartThreadsMoveInstructionsAndClearsMarker proves finding C: +// a move's one-shot instructions ride on the office run payload, and the marker +// is consumed (cleared) rather than stranded for an Office auto-start. +func TestOfficeAutoStartThreadsMoveInstructionsAndClearsMarker(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + seedOfficeTaskWithMarker(t, repo, "t-office-instr", &workflowmove.EntryOptions{Instructions: "focus on the auth bug"}) + + svc := createTestServiceWithAgent(repo, officeAutoStartStep(), newMockTaskRepo(), failIfLaunched(t)) + queued := &fakeRunQueueAdapter{calls: make(chan engine.QueueRunRequest, 1)} + svc.engineRunQueue = queued + svc.enginePrimary = &fakePrimaryAgentResolver{agentProfileID: "resolved-primary"} + + svc.handleTaskMovedNoSession(ctx, watcher.TaskMovedEventData{TaskID: "t-office-instr", ToStepID: "step2", StepTransitionID: 7}) + + select { + case req := <-queued.calls: + got, _ := req.Payload["one_time_instructions"].(string) + if got != "focus on the auth bug" { + t.Fatalf("office run payload one_time_instructions = %q, want the move instructions", got) + } + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for office QueueRun") + } + + stored, err := repo.GetTask(ctx, "t-office-instr") + if err != nil { + t.Fatalf("reload task: %v", err) + } + if _, pending := stored.Metadata[models.MetaKeyWorkflowMovePending]; pending { + t.Fatal("office auto-start must clear the move marker") + } +} + +// TestOfficeAutoStartSuppressesSkipPromptNoInstructions proves the office side +// of the skip_step_prompt-with-no-instructions suppression: no run is queued and +// the marker is still consumed. Suppression is synchronous (it returns before +// the office branch), so a default select safely asserts no queued run. +func TestOfficeAutoStartSuppressesSkipPromptNoInstructions(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + seedOfficeTaskWithMarker(t, repo, "t-office-skip", &workflowmove.EntryOptions{SkipStepPrompt: true}) + + svc := createTestServiceWithAgent(repo, officeAutoStartStep(), newMockTaskRepo(), failIfLaunched(t)) + queued := &fakeRunQueueAdapter{calls: make(chan engine.QueueRunRequest, 1)} + svc.engineRunQueue = queued + svc.enginePrimary = &fakePrimaryAgentResolver{agentProfileID: "resolved-primary"} + + svc.handleTaskMovedNoSession(ctx, watcher.TaskMovedEventData{TaskID: "t-office-skip", ToStepID: "step2", StepTransitionID: 8}) + + select { + case <-queued.calls: + t.Fatal("skip_step_prompt with no instructions must not queue an office run") + default: + } + + stored, err := repo.GetTask(ctx, "t-office-skip") + if err != nil { + t.Fatalf("reload task: %v", err) + } + if _, pending := stored.Metadata[models.MetaKeyWorkflowMovePending]; pending { + t.Fatal("suppressed office auto-start must still clear the move marker") + } +} diff --git a/apps/backend/internal/orchestrator/event_handlers_workflow_triggers_test.go b/apps/backend/internal/orchestrator/event_handlers_workflow_triggers_test.go index adf1a7955f..f526dab028 100644 --- a/apps/backend/internal/orchestrator/event_handlers_workflow_triggers_test.go +++ b/apps/backend/internal/orchestrator/event_handlers_workflow_triggers_test.go @@ -12,6 +12,7 @@ import ( "github.com/kandev/kandev/internal/orchestrator/messagequeue" "github.com/kandev/kandev/internal/task/models" wfmodels "github.com/kandev/kandev/internal/workflow/models" + workflowmove "github.com/kandev/kandev/internal/workflow/move" v1 "github.com/kandev/kandev/pkg/api/v1" ) @@ -465,6 +466,48 @@ func TestProcessOnEnter(t *testing.T) { } }) + // A one-shot instruction overlay must use the same queue path when the + // destination has no on_enter actions. The durable step prompt is not sent + // through this path, so the extracted instruction block is the assertion. + t.Run("queues move instructions when destination has no on_enter actions", func(t *testing.T) { + repo := setupTestRepo(t) + seedSession(t, repo, "t1", "s1", "step1") + + session, _ := repo.GetTaskSession(ctx, "s1") + session.State = models.TaskSessionStateWaitingForInput + session.AgentExecutionID = "exec-1" + seedExecutorRunning(t, repo, session.ID, session.TaskID, "exec-1") + _ = repo.UpdateTaskSession(ctx, session) + + agentMgr := &mockAgentManager{isAgentRunning: true, promptDone: make(chan struct{})} + svc := createTestServiceWithAgent(repo, newMockStepGetter(), newMockTaskRepo(), agentMgr) + svc.executor = executor.NewExecutor(agentMgr, repo, testLogger(), executor.ExecutorConfig{}) + + step := workflowmove.OverlayStep(&wfmodels.WorkflowStep{ + ID: "step2", WorkflowID: "wf1", Name: "Review", Position: 1, + }, &workflowmove.EntryOptions{Instructions: "focus on the auth bug"}) + + session, _ = repo.GetTaskSession(ctx, "s1") + svc.processOnEnter(ctx, "t1", session, step, "task description", 0, nil) + + select { + case <-agentMgr.promptDone: + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for move instructions to be sent to agent") + } + + agentMgr.mu.Lock() + got := agentMgr.capturedPrompts[0] + agentMgr.mu.Unlock() + want := workflowmove.WrapInstructions("focus on the auth bug") + if got != want { + t.Fatalf("agent received %q, want one-time instruction block %q", got, want) + } + if status := svc.messageQueue.GetStatus(ctx, "s1"); status.Count != 0 { + t.Fatalf("expected move instruction queue to be drained, count=%d entries=%+v", status.Count, status.Entries) + } + }) + t.Run("auto_start_agent queues prompt when session is already running", func(t *testing.T) { repo := setupTestRepo(t) seedSession(t, repo, "t1", "s1", "step1") @@ -1054,6 +1097,87 @@ func TestProcessOnEnterResetAgentContext(t *testing.T) { } }) + // A workflow-driven reset (durable reset_agent_context action or a one-time + // move override) must surface the same "Context reset" chat divider that the + // manual toolbar reset button emits, so the user sees the context was cleared. + t.Run("workflow reset emits the context reset divider", func(t *testing.T) { + repo := setupTestRepo(t) + seedSession(t, repo, "t1", "s1", "step1") + session, _ := repo.GetTaskSession(ctx, "s1") + seedExecutorRunning(t, repo, session.ID, session.TaskID, "exec-reset") + + svc := createTestServiceWithAgent( + repo, + newMockStepGetter(), + newMockTaskRepo(), + &mockAgentManager{repoForExecutionLookup: repo}, + ) + messages := &mockMessageCreator{} + svc.messageCreator = messages + + step := &wfmodels.WorkflowStep{ + ID: "step2", WorkflowID: "wf1", Name: "Review Step", + Events: wfmodels.StepEvents{OnEnter: []wfmodels.OnEnterAction{ + {Type: wfmodels.OnEnterResetAgentContext}, + }}, + } + + session, _ = repo.GetTaskSession(ctx, "s1") + svc.processOnEnter(ctx, "t1", session, step, "review task", 0, nil) + + dividers := 0 + for _, m := range messages.sessionMessages { + if m.content != contextResetMessage { + continue + } + if m.messageType != string(v1.MessageTypeStatus) { + t.Fatalf("context reset divider must be a status message, got %q", m.messageType) + } + dividers++ + } + if dividers != 1 { + t.Fatalf("expected exactly one context reset divider, got %d", dividers) + } + }) + + // A CREATED session has no prior conversation, so resetAgentContext skips the + // actual reset; the divider must be suppressed to match the manual path, + // which cannot even reach a CREATED session (it requires WAITING_FOR_INPUT). + t.Run("workflow reset on a CREATED session emits no divider", func(t *testing.T) { + repo := setupTestRepo(t) + seedSession(t, repo, "t1", "s1", "step1") + session, _ := repo.GetTaskSession(ctx, "s1") + session.State = models.TaskSessionStateCreated + session.AgentExecutionID = "exec-created" + seedExecutorRunning(t, repo, session.ID, session.TaskID, "exec-created") + _ = repo.UpdateTaskSession(ctx, session) + + svc := createTestServiceWithAgent( + repo, + newMockStepGetter(), + newMockTaskRepo(), + &mockAgentManager{repoForExecutionLookup: repo}, + ) + messages := &mockMessageCreator{} + svc.messageCreator = messages + + step := &wfmodels.WorkflowStep{ + ID: "step2", WorkflowID: "wf1", Name: "Work", + Events: wfmodels.StepEvents{OnEnter: []wfmodels.OnEnterAction{ + {Type: wfmodels.OnEnterResetAgentContext}, + }}, + } + + session, _ = repo.GetTaskSession(ctx, "s1") + svc.processOnEnter(ctx, "t1", session, step, "review task", 0, nil) + + for _, m := range messages.sessionMessages { + if m.content == contextResetMessage { + t.Fatal("a never-prompted CREATED session must not emit a context reset divider") + } + } + }) + // Same skip for passthrough: the CLI has no conversation to reset before // its first prompt either. t.Run("reset_agent_context skipped for CREATED passthrough session", func(t *testing.T) { diff --git a/apps/backend/internal/orchestrator/messagequeue/repository_sqlite.go b/apps/backend/internal/orchestrator/messagequeue/repository_sqlite.go index b23a0a25a0..90f4422469 100644 --- a/apps/backend/internal/orchestrator/messagequeue/repository_sqlite.go +++ b/apps/backend/internal/orchestrator/messagequeue/repository_sqlite.go @@ -16,6 +16,7 @@ import ( "github.com/jmoiron/sqlx" internaldb "github.com/kandev/kandev/internal/db" "github.com/kandev/kandev/internal/task/models" + workflowmove "github.com/kandev/kandev/internal/workflow/move" ) // sqliteRepository persists queued messages and pending moves. @@ -459,7 +460,8 @@ func (r *sqliteRepository) initSchema() error { step_position INTEGER NOT NULL DEFAULT 0, queued_at TIMESTAMP NOT NULL, actor TEXT NOT NULL DEFAULT '', - sender_session_id TEXT NOT NULL DEFAULT '' + sender_session_id TEXT NOT NULL DEFAULT '', + entry_options_json TEXT NOT NULL DEFAULT '{}' ); -- Per-session cross-process mutex. Every queue mutation takes this row @@ -497,6 +499,9 @@ func (r *sqliteRepository) initSchema() error { if _, alterErr := r.db.Exec(`ALTER TABLE pending_moves ADD COLUMN move_id TEXT NOT NULL DEFAULT ''`); alterErr != nil && !internaldb.IsDuplicateColumnError(alterErr) { return alterErr } + if _, alterErr := r.db.Exec(`ALTER TABLE pending_moves ADD COLUMN entry_options_json TEXT NOT NULL DEFAULT '{}'`); alterErr != nil && !internaldb.IsDuplicateColumnError(alterErr) { + return alterErr + } if _, alterErr := r.db.Exec(`ALTER TABLE pending_moves ADD COLUMN session_incarnation_id TEXT NOT NULL DEFAULT ''`); alterErr != nil && !internaldb.IsDuplicateColumnError(alterErr) { return alterErr } @@ -558,6 +563,21 @@ func (r *sqliteRepository) bumpQueuePositionTx(ctx context.Context, tx *sqlx.Tx, return nil } +// marshalEntryOptions encodes one-shot move overrides for the pending_moves +// row. Option-less moves round-trip through the empty object so an existing +// row (or an ordinary move) decodes back to nil. +func marshalEntryOptions(options *workflowmove.EntryOptions) string { + encoded, err := workflowmove.EncodeEntryOptionsJSON(options) + if err != nil { + return "{}" + } + return string(encoded) +} + +func decodeEntryOptions(encoded string) (*workflowmove.EntryOptions, error) { + return workflowmove.DecodeEntryOptionsJSON([]byte(encoded)) +} + // Insert appends a new entry at the tail of the session's FIFO queue. func (r *sqliteRepository) Insert(ctx context.Context, msg *QueuedMessage, maxPerSession int) error { return r.insert(ctx, nil, msg, nil, maxPerSession, nil) @@ -2217,15 +2237,15 @@ func (r *sqliteRepository) getPendingMoveTx(ctx context.Context, tx *sqlx.Tx, se moveID, sessionIncarnationID, taskID, workflowID, workflowStepID string position int queuedAt time.Time - actor, senderSessionID string + actor, senderSessionID, optionsJSON string ) err := tx.QueryRowxContext(ctx, r.db.Rebind(` SELECT move_id, session_incarnation_id, task_id, workflow_id, workflow_step_id, - step_position, queued_at, actor, sender_session_id + step_position, queued_at, actor, sender_session_id, entry_options_json FROM pending_moves WHERE session_id = ? `), sessionID).Scan( &moveID, &sessionIncarnationID, &taskID, &workflowID, &workflowStepID, - &position, &queuedAt, &actor, &senderSessionID, + &position, &queuedAt, &actor, &senderSessionID, &optionsJSON, ) if errors.Is(err, sql.ErrNoRows) { return nil, nil @@ -2233,10 +2253,15 @@ func (r *sqliteRepository) getPendingMoveTx(ctx context.Context, tx *sqlx.Tx, se if err != nil { return nil, fmt.Errorf("read pending move snapshot: %w", err) } + entryOptions, err := decodeEntryOptions(optionsJSON) + if err != nil { + return nil, fmt.Errorf("decode pending move entry options: %w", err) + } return &PendingMove{ MoveID: moveID, SessionIncarnationID: sessionIncarnationID, TaskID: taskID, WorkflowID: workflowID, WorkflowStepID: workflowStepID, Position: position, QueuedAt: queuedAt, Actor: actor, SenderSessionID: senderSessionID, + EntryOptions: entryOptions, }, nil } @@ -5937,13 +5962,14 @@ func (r *sqliteRepository) restorePendingMoveTx( if _, err := tx.ExecContext(ctx, r.db.Rebind(` INSERT INTO pending_moves ( id, move_id, session_incarnation_id, session_id, task_id, workflow_id, - workflow_step_id, step_position, queued_at, actor, sender_session_id + workflow_step_id, step_position, queued_at, actor, sender_session_id, entry_options_json ) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `), uuid.New().String(), pendingMove.MoveID, pendingMove.SessionIncarnationID, sessionID, pendingMove.TaskID, pendingMove.WorkflowID, pendingMove.WorkflowStepID, pendingMove.Position, queuedAt, pendingMove.Actor, pendingMove.SenderSessionID, + marshalEntryOptions(pendingMove.EntryOptions), ); err != nil { return fmt.Errorf("restore pending move: %w", err) } @@ -6070,9 +6096,9 @@ func (r *sqliteRepository) SetPendingMove(ctx context.Context, sessionID string, if _, err := tx.ExecContext(ctx, r.db.Rebind(` INSERT INTO pending_moves ( id, move_id, session_incarnation_id, session_id, task_id, workflow_id, - workflow_step_id, step_position, queued_at, actor, sender_session_id + workflow_step_id, step_position, queued_at, actor, sender_session_id, entry_options_json ) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(session_id) DO UPDATE SET task_id = excluded.task_id, session_incarnation_id = excluded.session_incarnation_id, @@ -6082,11 +6108,12 @@ func (r *sqliteRepository) SetPendingMove(ctx context.Context, sessionID string, queued_at = excluded.queued_at, actor = excluded.actor, sender_session_id = excluded.sender_session_id, - move_id = excluded.move_id + move_id = excluded.move_id, + entry_options_json = excluded.entry_options_json `), uuid.New().String(), move.MoveID, move.SessionIncarnationID, sessionID, move.TaskID, move.WorkflowID, move.WorkflowStepID, move.Position, move.QueuedAt, move.Actor, - move.SenderSessionID, + move.SenderSessionID, marshalEntryOptions(move.EntryOptions), ); err != nil { return fmt.Errorf("upsert pending move: %w", err) } @@ -6099,21 +6126,25 @@ func (r *sqliteRepository) GetPendingMove(ctx context.Context, sessionID string) moveID, sessionIncarnationID, taskID, workflowID, workflowStepID string position int queuedAt time.Time - actor, senderSessionID string + actor, senderSessionID, optionsJSON string ) if err := r.ro.QueryRowxContext(ctx, r.db.Rebind(` SELECT move_id, session_incarnation_id, task_id, workflow_id, workflow_step_id, - step_position, queued_at, actor, sender_session_id + step_position, queued_at, actor, sender_session_id, entry_options_json FROM pending_moves WHERE session_id = ? `), sessionID).Scan( &moveID, &sessionIncarnationID, &taskID, &workflowID, &workflowStepID, - &position, &queuedAt, &actor, &senderSessionID, + &position, &queuedAt, &actor, &senderSessionID, &optionsJSON, ); err != nil { if errors.Is(err, sql.ErrNoRows) { return nil, nil } return nil, fmt.Errorf("read pending move: %w", err) } + entryOptions, err := decodeEntryOptions(optionsJSON) + if err != nil { + return nil, fmt.Errorf("decode pending move entry options: %w", err) + } return &PendingMove{ MoveID: moveID, SessionIncarnationID: sessionIncarnationID, @@ -6124,6 +6155,7 @@ func (r *sqliteRepository) GetPendingMove(ctx context.Context, sessionID string) QueuedAt: queuedAt, Actor: actor, SenderSessionID: senderSessionID, + EntryOptions: entryOptions, }, nil } @@ -6144,21 +6176,25 @@ func (r *sqliteRepository) TakePendingMove(ctx context.Context, sessionID string moveID, sessionIncarnationID, taskID, workflowID, workflowStepID string position int queuedAt time.Time - actor, senderSessionID string + actor, senderSessionID, optionsJSON string ) if err := tx.QueryRowxContext(ctx, r.db.Rebind(` SELECT move_id, session_incarnation_id, task_id, workflow_id, workflow_step_id, - step_position, queued_at, actor, sender_session_id + step_position, queued_at, actor, sender_session_id, entry_options_json FROM pending_moves WHERE session_id = ? `), sessionID).Scan( &moveID, &sessionIncarnationID, &taskID, &workflowID, &workflowStepID, - &position, &queuedAt, &actor, &senderSessionID, + &position, &queuedAt, &actor, &senderSessionID, &optionsJSON, ); err != nil { if errors.Is(err, sql.ErrNoRows) { return nil, nil } return nil, fmt.Errorf("read pending move: %w", err) } + entryOptions, err := decodeEntryOptions(optionsJSON) + if err != nil { + return nil, fmt.Errorf("decode pending move entry options: %w", err) + } if _, err := tx.ExecContext(ctx, r.db.Rebind(`DELETE FROM pending_moves WHERE session_id = ?`), sessionID); err != nil { return nil, fmt.Errorf("delete pending move: %w", err) } @@ -6175,6 +6211,7 @@ func (r *sqliteRepository) TakePendingMove(ctx context.Context, sessionID string QueuedAt: queuedAt, Actor: actor, SenderSessionID: senderSessionID, + EntryOptions: entryOptions, }, nil } diff --git a/apps/backend/internal/orchestrator/messagequeue/repository_sqlite_test.go b/apps/backend/internal/orchestrator/messagequeue/repository_sqlite_test.go index d51ec7fe99..b7803caad5 100644 --- a/apps/backend/internal/orchestrator/messagequeue/repository_sqlite_test.go +++ b/apps/backend/internal/orchestrator/messagequeue/repository_sqlite_test.go @@ -11,6 +11,7 @@ import ( "github.com/jmoiron/sqlx" internaldb "github.com/kandev/kandev/internal/db" + workflowmove "github.com/kandev/kandev/internal/workflow/move" _ "github.com/mattn/go-sqlite3" ) @@ -1090,6 +1091,42 @@ func TestSQLiteRepository_PendingMove(t *testing.T) { } } +func TestSQLiteRepository_PendingMoveEntryOptions(t *testing.T) { + repo := newTestSQLiteRepo(t) + ctx := context.Background() + + // A move without one-shot options round-trips as nil so pre-existing rows + // and ordinary moves decode back to an option-less move. + plain := &PendingMove{MoveID: "m-plain", TaskID: "t1", WorkflowID: "w1", WorkflowStepID: "step-A"} + if err := repo.SetPendingMove(ctx, "s-plain", plain); err != nil { + t.Fatalf("set plain: %v", err) + } + if got, err := repo.GetPendingMove(ctx, "s-plain"); err != nil || got == nil || got.EntryOptions != nil { + t.Fatalf("expected nil entry options, got %+v err=%v", got, err) + } + + // A move with options round-trips every field through both Get and Take. + opts := &workflowmove.EntryOptions{ResetContext: true, Instructions: "carry on", SkipStepPrompt: true} + move := &PendingMove{MoveID: "m-opts", TaskID: "t2", WorkflowID: "w1", WorkflowStepID: "step-B", EntryOptions: opts} + if err := repo.SetPendingMove(ctx, "s-opts", move); err != nil { + t.Fatalf("set opts: %v", err) + } + got, err := repo.GetPendingMove(ctx, "s-opts") + if err != nil || got == nil || got.EntryOptions == nil { + t.Fatalf("expected entry options via get, got %+v err=%v", got, err) + } + if !got.EntryOptions.ResetContext || got.EntryOptions.Instructions != "carry on" || !got.EntryOptions.SkipStepPrompt { + t.Errorf("entry options not preserved via get: %+v", got.EntryOptions) + } + taken, err := repo.TakePendingMove(ctx, "s-opts") + if err != nil || taken == nil || taken.EntryOptions == nil { + t.Fatalf("expected entry options via take, got %+v err=%v", taken, err) + } + if !taken.EntryOptions.SkipStepPrompt || taken.EntryOptions.Instructions != "carry on" { + t.Errorf("entry options not preserved via take: %+v", taken.EntryOptions) + } +} + func TestSQLiteRepository_PendingMoveSenderSessionMigration(t *testing.T) { raw, err := sql.Open("sqlite3", "file:pending-move-migration?mode=memory&cache=shared") if err != nil { diff --git a/apps/backend/internal/orchestrator/messagequeue/types.go b/apps/backend/internal/orchestrator/messagequeue/types.go index 6bc0e9aaed..18cdda5a8d 100644 --- a/apps/backend/internal/orchestrator/messagequeue/types.go +++ b/apps/backend/internal/orchestrator/messagequeue/types.go @@ -6,6 +6,7 @@ import ( "github.com/google/uuid" "github.com/kandev/kandev/internal/task/plancomments" + workflowmove "github.com/kandev/kandev/internal/workflow/move" ) // DefaultMaxPerSession is the default cap for queued messages per session @@ -545,6 +546,11 @@ type PendingMove struct { // distinct from the session owning this queue, which is only the execution // context used to apply the deferred move. SenderSessionID string `json:"sender_session_id,omitempty"` + // EntryOptions carries the complete typed one-shot move overrides for a + // deferred move so the target-step entry can apply them once the source + // turn ends. It survives the queue's normal restart/reload path; existing + // rows decode as nil (an ordinary move). + EntryOptions *workflowmove.EntryOptions `json:"entry_options,omitempty"` } // PendingMoveTTL bounds how long a deferred move may stay armed before it is diff --git a/apps/backend/internal/orchestrator/prompt_launch_fallback_test.go b/apps/backend/internal/orchestrator/prompt_launch_fallback_test.go index 317fd25980..288f4cc2c8 100644 --- a/apps/backend/internal/orchestrator/prompt_launch_fallback_test.go +++ b/apps/backend/internal/orchestrator/prompt_launch_fallback_test.go @@ -109,6 +109,7 @@ func TestApplyWorkflowAndPlanMode_ExpandsWithoutWorkflowComposition(t *testing.T false, test.isEphemeral, false, + false, "", ) @@ -142,7 +143,7 @@ func TestApplyWorkflowAndPlanMode_PreservesAcceptedContextWithoutWorkflow(t *tes svc := createTestService(setupTestRepo(t), newMockStepGetter(), newMockTaskRepo()) svc.promptExpander = promptService got, _, gotTrustedContext := svc.applyWorkflowAndPlanModeWithPromptContext( - ctx, preparedPrompt, "task-1", "session-1", "", false, false, false, trustedContext, + ctx, preparedPrompt, "task-1", "session-1", "", false, false, false, false, trustedContext, ) require.Equal(t, trustedContext, gotTrustedContext) @@ -209,7 +210,7 @@ func TestApplyWorkflowAndPlanMode_WithoutWorkflowGuards(t *testing.T) { svc.promptExpander = test.expander got, _, trustedContext := svc.applyWorkflowAndPlanModeWithPromptContext( ctx, test.prompt, "task-1", "session-1", "", false, false, - test.isPassthrough, "", + test.isPassthrough, false, "", ) require.Contains(t, got, test.wantPromptContent) diff --git a/apps/backend/internal/orchestrator/task_operations.go b/apps/backend/internal/orchestrator/task_operations.go index 13f4bbc750..56690075ac 100644 --- a/apps/backend/internal/orchestrator/task_operations.go +++ b/apps/backend/internal/orchestrator/task_operations.go @@ -32,6 +32,7 @@ import ( "github.com/kandev/kandev/internal/sysprompt" "github.com/kandev/kandev/internal/task/models" wfmodels "github.com/kandev/kandev/internal/workflow/models" + workflowmove "github.com/kandev/kandev/internal/workflow/move" v1 "github.com/kandev/kandev/pkg/api/v1" ) @@ -743,7 +744,7 @@ func (s *Service) startCreatedSession( var generatedPromptReferenceContext string effectivePrompt, planModeActive, generatedPromptReferenceContext = s.applyWorkflowAndPlanModeWithPromptContext( ctx, effectivePrompt, taskID, sessionID, dbTask.WorkflowStepID, - planMode, task.IsEphemeral, session.IsPassthrough, promptReferenceContext, + planMode, task.IsEphemeral, session.IsPassthrough, false, promptReferenceContext, ) if generatedPromptReferenceContext != "" { // The workflow helper preserves a direct message's acceptance-time @@ -1056,6 +1057,12 @@ type startTaskOptions struct { // SpawnOrigin is set when another agent session spawned this launch; it // produces the spawner-attribution system block on the first turn. SpawnOrigin *SpawnOrigin + // EntryOptions carries one-shot workflow-move overrides for a fresh + // auto-started session. The profile override is applied via the profile + // argument (with ProfileExplicit); the instructions are appended once to the + // composed workflow prompt below because the prompt is rebuilt from the + // durable step by ID and cannot see a transient step overlay. + EntryOptions *workflowmove.EntryOptions // WorkflowEntryID pins source bindings and explicit route retries to the // immutable step-entry ledger row that initiated an automatic launch. WorkflowEntryID int64 @@ -1421,11 +1428,26 @@ func (s *Service) startTask(ctx context.Context, taskID string, agentProfileID s // wrapping and launch behavior. isPassthrough = launchSession.IsPassthrough + skipStepPrompt := opts.EntryOptions != nil && opts.EntryOptions.SkipStepPrompt effectivePrompt, planModeActive, promptReferenceContext := s.applyWorkflowAndPlanMode( ctx, effectivePrompt, task.ID, sessionID, workflowStepID, - planMode, task.IsEphemeral, isPassthrough, + planMode, task.IsEphemeral, isPassthrough, skipStepPrompt, ) + // Append one-shot workflow-move instructions to a fresh auto-started + // session. The prompt was rebuilt from the durable step by ID above, so a + // transient step overlay could not carry them; append the same sentinel + // block once here. The sentinel guards against a double append on retry. + if opts.EntryOptions != nil && opts.EntryOptions.Instructions != "" && + !strings.Contains(effectivePrompt, workflowmove.InstructionsEnd) { + wrapped := workflowmove.WrapInstructions(opts.EntryOptions.Instructions) + if effectivePrompt == "" { + effectivePrompt = wrapped + } else { + effectivePrompt = effectivePrompt + "\n\n" + wrapped + } + } + // Inject config context for config-mode sessions (dedicated settings chat) configMode := false if cm, ok := launchSession.Metadata["config_mode"].(bool); ok && cm { @@ -2137,10 +2159,11 @@ func (s *Service) applyWorkflowAndPlanMode( planMode bool, isEphemeral bool, isPassthrough bool, + skipStepPrompt bool, ) (string, bool, string) { return s.applyWorkflowAndPlanModeWithPromptContext( ctx, prompt, taskID, sessionID, workflowStepID, planMode, - isEphemeral, isPassthrough, "", + isEphemeral, isPassthrough, skipStepPrompt, "", ) } @@ -2158,6 +2181,7 @@ func (s *Service) applyWorkflowAndPlanModeWithPromptContext( planMode bool, isEphemeral bool, isPassthrough bool, + skipStepPrompt bool, trustedPromptContext string, ) (string, bool, string) { effectivePrompt := prompt @@ -2187,11 +2211,11 @@ func (s *Service) applyWorkflowAndPlanModeWithPromptContext( stepHasPlanMode = step.HasOnEnterAction(wfmodels.OnEnterEnablePlanMode) if trustedPromptContext != "" { effectivePrompt, promptReferenceContext = s.buildWorkflowPromptWithTrustedContext( - ctx, effectivePrompt, step, taskID, sessionID, isPassthrough, trustedPromptContext, + ctx, effectivePrompt, step, taskID, sessionID, isPassthrough, skipStepPrompt, trustedPromptContext, ) } else { effectivePrompt, promptReferenceContext = s.buildWorkflowPromptWithContext( - ctx, effectivePrompt, step, taskID, sessionID, isPassthrough, + ctx, effectivePrompt, step, taskID, sessionID, isPassthrough, skipStepPrompt, ) } } @@ -2269,7 +2293,7 @@ func (s *Service) recordInitialMessage(ctx context.Context, taskID, sessionID, p // If the step has enable_plan_mode in on_enter events, plan mode prefix is also prepended. // Only true internal instructions are wrapped in tags so they can be stripped from the visible chat. func (s *Service) buildWorkflowPrompt(ctx context.Context, basePrompt string, step *wfmodels.WorkflowStep, taskID string, sessionID string, isPassthrough bool) string { - prompt, _ := s.buildWorkflowPromptWithContext(ctx, basePrompt, step, taskID, sessionID, isPassthrough) + prompt, _ := s.buildWorkflowPromptWithContext(ctx, basePrompt, step, taskID, sessionID, isPassthrough, false) return prompt } @@ -2317,9 +2341,10 @@ func (s *Service) buildWorkflowPromptWithContext( taskID string, sessionID string, isPassthrough bool, + skipStepPrompt bool, ) (string, string) { return s.buildWorkflowPromptWithTrustedContext( - ctx, basePrompt, step, taskID, sessionID, isPassthrough, "", + ctx, basePrompt, step, taskID, sessionID, isPassthrough, skipStepPrompt, "", ) } @@ -2334,6 +2359,7 @@ func (s *Service) buildWorkflowPromptWithTrustedContext( taskID string, sessionID string, isPassthrough bool, + skipStepPrompt bool, trustedPromptContext string, ) (string, string) { _ = sessionID @@ -2343,20 +2369,11 @@ func (s *Service) buildWorkflowPromptWithTrustedContext( parts = append(parts, block) } - // Build the prompt from step.Prompt template and base prompt - if step.Prompt != "" { - interpolatedPrompt := sysprompt.InterpolatePlaceholders(step.Prompt, taskID) - if strings.Contains(interpolatedPrompt, "{{task_prompt}}") { - // Replace placeholder with base prompt - combined := strings.Replace(interpolatedPrompt, "{{task_prompt}}", basePrompt, 1) - parts = append(parts, combined) - } else { - // A step prompt without {{task_prompt}} is treated as the full visible prompt. - parts = append(parts, interpolatedPrompt) - } - } else { - // No step prompt, just use base prompt - parts = append(parts, basePrompt) + // skip_step_prompt suppresses the step prompt and its task-description + // fallback for this one entry; only the workflow-level block above (and any + // one-time move instructions appended by the caller) remain. + if !skipStepPrompt { + parts = append(parts, stepPromptBody(step, taskID, basePrompt)) } joined := strings.Join(parts, "\n\n") @@ -2370,6 +2387,22 @@ func (s *Service) buildWorkflowPromptWithTrustedContext( return s.expandPromptReferencesWithContext(ctx, joined, isPassthrough) } +// stepPromptBody renders the visible step prompt for one entry: the step's +// prompt template with {{task_prompt}} resolved to basePrompt, a step prompt +// without that placeholder used verbatim, or the base prompt when the step has +// no prompt of its own. +func stepPromptBody(step *wfmodels.WorkflowStep, taskID, basePrompt string) string { + if step.Prompt == "" { + return basePrompt + } + interpolatedPrompt := sysprompt.InterpolatePlaceholders(step.Prompt, taskID) + if strings.Contains(interpolatedPrompt, "{{task_prompt}}") { + return strings.Replace(interpolatedPrompt, "{{task_prompt}}", basePrompt, 1) + } + // A step prompt without {{task_prompt}} is treated as the full visible prompt. + return interpolatedPrompt +} + // workflowInstructionsBlock returns the visible "## Workflow instructions" // section when the step's workflow has a non-empty prompt. Empty/whitespace // prompts and missing getters/workflows omit the section entirely. @@ -7945,6 +7978,31 @@ func (s *Service) CompleteTask(ctx context.Context, taskID string) error { return nil } +// contextResetMessage is the synthetic status message the chat renders as a +// "Context reset" divider. Both the manual reset path and the workflow-driven +// on-enter reset emit this exact text so the two paths cannot drift. +const contextResetMessage = "Context reset — new conversation started" + +// createContextResetMessage inserts the synthetic contextResetMessage status +// row for a session. Callers must only invoke it when an actual reset occurred +// (e.g. not for a never-started CREATED session, which has no prior +// conversation to clear). +func (s *Service) createContextResetMessage(ctx context.Context, taskID, sessionID string) { + if s.messageCreator == nil { + return + } + if err := s.messageCreator.CreateSessionMessage( + ctx, taskID, + contextResetMessage, + sessionID, string(v1.MessageTypeStatus), + s.getActiveTurnID(sessionID), + nil, false, + ); err != nil { + s.logger.Warn("failed to create context reset message", + zap.String("session_id", sessionID), zap.Error(err)) + } +} + // ResetAgentContext resets the agent's conversation context for a session, // clearing conversation history while preserving the workspace environment. func (s *Service) ResetAgentContext(ctx context.Context, sessionID string) error { @@ -7975,18 +8033,7 @@ func (s *Service) ResetAgentContext(ctx context.Context, sessionID string) error // Restore WAITING_FOR_INPUT — handleAgentReady ignores events during reset s.setSessionWaitingForInput(ctx, session.TaskID, sessionID) - if s.messageCreator != nil { - if err := s.messageCreator.CreateSessionMessage( - ctx, session.TaskID, - "Context reset — new conversation started", - sessionID, string(v1.MessageTypeStatus), - s.getActiveTurnID(sessionID), - nil, false, - ); err != nil { - s.logger.Warn("failed to create context reset message", - zap.String("session_id", sessionID), zap.Error(err)) - } - } + s.createContextResetMessage(ctx, session.TaskID, sessionID) return nil } diff --git a/apps/backend/internal/orchestrator/watcher/watcher.go b/apps/backend/internal/orchestrator/watcher/watcher.go index dd439f961c..aa091390e2 100644 --- a/apps/backend/internal/orchestrator/watcher/watcher.go +++ b/apps/backend/internal/orchestrator/watcher/watcher.go @@ -91,6 +91,7 @@ type GitEventData = lifecycle.GitEventPayload type TaskMovedEventData struct { TaskID string `json:"task_id"` StepTransitionID int64 `json:"step_transition_id,omitempty"` + MoveID string `json:"move_id,omitempty"` FromStepID string `json:"from_step_id"` ToStepID string `json:"to_step_id"` SessionID string `json:"session_id"` diff --git a/apps/backend/internal/orchestrator/workflow_prompt_test.go b/apps/backend/internal/orchestrator/workflow_prompt_test.go index 18e4ef2a0b..a95389a5bc 100644 --- a/apps/backend/internal/orchestrator/workflow_prompt_test.go +++ b/apps/backend/internal/orchestrator/workflow_prompt_test.go @@ -364,6 +364,7 @@ func TestApplyWorkflowAndPlanMode_KeepsWorkflowPromptVisibleWhenStepEnablesPlanM false, false, // isEphemeral false, // isPassthrough + false, // skipStepPrompt ) if !planModeActive { @@ -380,6 +381,35 @@ func TestApplyWorkflowAndPlanMode_KeepsWorkflowPromptVisibleWhenStepEnablesPlanM } } +func TestApplyWorkflowAndPlanMode_SkipStepPromptSuppressesStepAndBase(t *testing.T) { + repo := setupTestRepo(t) + stepGetter := newMockStepGetter() + stepGetter.steps["step-1"] = &wfmodels.WorkflowStep{ + ID: "step-1", + Prompt: "Commit the changes, push and create a draft PR.", + } + svc := createTestService(repo, stepGetter, newMockTaskRepo()) + + got, _, _ := svc.applyWorkflowAndPlanMode( + context.Background(), + "Migrate Atlantis datasource.", + "task-1", + "session-1", + "step-1", + false, + false, // isEphemeral + false, // isPassthrough + true, // skipStepPrompt + ) + + if strings.Contains(got, "Commit the changes") { + t.Fatalf("skip_step_prompt must drop the step prompt, got %q", got) + } + if strings.Contains(got, "Migrate Atlantis datasource.") { + t.Fatalf("skip_step_prompt must drop the task-description fallback, got %q", got) + } +} + func TestApplyWorkflowAndPlanMode_PassthroughSkipsReferenceExpansion(t *testing.T) { repo := setupTestRepo(t) stepGetter := newMockStepGetter() @@ -400,6 +430,7 @@ func TestApplyWorkflowAndPlanMode_PassthroughSkipsReferenceExpansion(t *testing. false, false, // isEphemeral true, // isPassthrough + false, // skipStepPrompt ) want := "Use @my-prompt for context." @@ -432,6 +463,7 @@ func TestApplyWorkflowAndPlanMode_PreservesAcceptanceTimePromptContext(t *testin false, false, // isEphemeral false, // isPassthrough + false, // skipStepPrompt fakeResolvedPromptReferenceContext, ) diff --git a/apps/backend/internal/task/dto/dto.go b/apps/backend/internal/task/dto/dto.go index b43763b760..8cdac1209c 100644 --- a/apps/backend/internal/task/dto/dto.go +++ b/apps/backend/internal/task/dto/dto.go @@ -8,6 +8,7 @@ import ( "github.com/kandev/kandev/internal/task/service" "github.com/kandev/kandev/internal/task/statussummary" wfmodels "github.com/kandev/kandev/internal/workflow/models" + workflowmove "github.com/kandev/kandev/internal/workflow/move" v1 "github.com/kandev/kandev/pkg/api/v1" ) @@ -1216,8 +1217,17 @@ type StepActionDTO struct { // MoveTaskResponse includes the task and the target workflow step info type MoveTaskResponse struct { - Task TaskDTO `json:"task"` - WorkflowStep WorkflowStepDTO `json:"workflow_step"` + Task TaskDTO `json:"task"` + WorkflowStep WorkflowStepDTO `json:"workflow_step"` + MoveID string `json:"move_id,omitempty"` + EntryOptions *workflowmove.EntryOptions `json:"entry_options,omitempty"` + // Disposition reports how an MCP move_task call was resolved: "applied" when + // the move committed immediately, or "deferred" when it was recorded to run + // at the source session's turn-end. It lets an agent distinguish deferred + // acceptance from an immediate move and correlate the retained one-shot + // EntryOptions (via MoveID) with the eventual step entry. The HTTP move path + // always applies immediately and leaves this empty. + Disposition string `json:"disposition,omitempty"` } // Session Workflow Review DTOs diff --git a/apps/backend/internal/task/handlers/errors.go b/apps/backend/internal/task/handlers/errors.go index b730fe62cd..22d68e4d42 100644 --- a/apps/backend/internal/task/handlers/errors.go +++ b/apps/backend/internal/task/handlers/errors.go @@ -10,6 +10,7 @@ import ( "github.com/kandev/kandev/internal/common/logger" taskrepo "github.com/kandev/kandev/internal/task/repository/sqlite" "github.com/kandev/kandev/internal/task/service" + workflowmove "github.com/kandev/kandev/internal/workflow/move" ws "github.com/kandev/kandev/pkg/websocket" "go.uber.org/zap" ) @@ -24,6 +25,7 @@ const ( moveConflictCodeDifferentWorkspace = "task_move_different_workspace" moveConflictCodeWorkflowStep = "task_move_workflow_step" moveConflictCodeWIPLimit = "task_move_wip_limit" + moveConflictCodePending = "task_move_pending" ) func handleNotFound(c *gin.Context, log *logger.Logger, err error, fallback string) { @@ -185,10 +187,30 @@ func isMoveConflict(err error) bool { return moveConflictCode(err) != "" } +// moveEntryOptionsWSError classifies one-shot move-entry validation and +// conflict failures for the WebSocket move handler, returning a stable +// error code and the safe error message (which never carries option values). +func moveEntryOptionsWSError(err error) (string, string, bool) { + switch { + case errors.Is(err, workflowmove.ErrMoveConflict): + return ws.ErrorCodeConflict, err.Error(), true + case errors.Is(err, workflowmove.ErrConflictingInstructions), + errors.Is(err, workflowmove.ErrEntryOptionsRequireStepChange), + errors.Is(err, workflowmove.ErrEntryOptionsUnsupported), + errors.Is(err, workflowmove.ErrEntryTargetUnavailable): + return ws.ErrorCodeValidation, err.Error(), true + default: + return "", "", false + } +} + func moveConflictCode(err error) string { if err == nil { return "" } + if errors.Is(err, workflowmove.ErrMoveConflict) { + return moveConflictCodePending + } msg := strings.ToLower(err.Error()) switch { case strings.Contains(msg, "active session"): @@ -219,6 +241,12 @@ func isValidationError(err error) bool { if errors.Is(err, service.ErrExternalIDInvalid) { return true } + if errors.Is(err, workflowmove.ErrConflictingInstructions) || + errors.Is(err, workflowmove.ErrEntryOptionsRequireStepChange) || + errors.Is(err, workflowmove.ErrEntryOptionsUnsupported) || + errors.Is(err, workflowmove.ErrEntryTargetUnavailable) { + return true + } msg := strings.ToLower(err.Error()) return strings.Contains(msg, "pending approval") || strings.Contains(msg, "validation") || diff --git a/apps/backend/internal/task/handlers/task_http_handlers.go b/apps/backend/internal/task/handlers/task_http_handlers.go index 5a13cf4e80..84fb3b0555 100644 --- a/apps/backend/internal/task/handlers/task_http_handlers.go +++ b/apps/backend/internal/task/handlers/task_http_handlers.go @@ -25,6 +25,7 @@ import ( "github.com/kandev/kandev/internal/task/statussummary" usermodels "github.com/kandev/kandev/internal/user/models" wfmodels "github.com/kandev/kandev/internal/workflow/models" + workflowmove "github.com/kandev/kandev/internal/workflow/move" "github.com/kandev/kandev/internal/worktree" v1 "github.com/kandev/kandev/pkg/api/v1" "go.uber.org/zap" @@ -1725,9 +1726,10 @@ func (h *TaskHandlers) httpUpdateTaskRepository(c *gin.Context) { } type httpMoveTaskRequest struct { - WorkflowID string `json:"workflow_id"` - WorkflowStepID string `json:"workflow_step_id"` - Position int `json:"position"` + WorkflowID string `json:"workflow_id"` + WorkflowStepID string `json:"workflow_step_id"` + Position int `json:"position"` + EntryOptions *workflowmove.EntryOptions `json:"entry_options,omitempty"` } func (h *TaskHandlers) httpMoveTask(c *gin.Context) { @@ -1743,7 +1745,11 @@ func (h *TaskHandlers) httpMoveTask(c *gin.Context) { result, err := h.service.MoveTaskWithOptions( c.Request.Context(), c.Param("id"), body.WorkflowID, body.WorkflowStepID, body.Position, - service.MoveTaskOptions{AllowActivePrimarySession: true, StepHistoryActor: wfmodels.StepTransitionActorHuman}, + service.MoveTaskOptions{ + AllowActivePrimarySession: true, + StepHistoryActor: wfmodels.StepTransitionActorHuman, + EntryOptions: body.EntryOptions, + }, ) if err != nil { handleSelectedMoveError(c, h.logger, err) @@ -1751,7 +1757,9 @@ func (h *TaskHandlers) httpMoveTask(c *gin.Context) { } response := dto.MoveTaskResponse{ - Task: dto.FromTask(result.Task), + Task: dto.FromTask(result.Task), + MoveID: result.MoveID, + EntryOptions: result.EntryOptions, } if result.WorkflowStep != nil { response.WorkflowStep = dto.FromWorkflowStep(result.WorkflowStep) diff --git a/apps/backend/internal/task/handlers/task_ws_handlers.go b/apps/backend/internal/task/handlers/task_ws_handlers.go index 826d71c6a2..d8c0631c9e 100644 --- a/apps/backend/internal/task/handlers/task_ws_handlers.go +++ b/apps/backend/internal/task/handlers/task_ws_handlers.go @@ -10,6 +10,7 @@ import ( "github.com/kandev/kandev/internal/task/models" "github.com/kandev/kandev/internal/task/service" wfmodels "github.com/kandev/kandev/internal/workflow/models" + workflowmove "github.com/kandev/kandev/internal/workflow/move" v1 "github.com/kandev/kandev/pkg/api/v1" ws "github.com/kandev/kandev/pkg/websocket" "go.uber.org/zap" @@ -399,10 +400,11 @@ func (h *TaskHandlers) wsArchiveTask(ctx context.Context, msg *ws.Message) (*ws. } type wsMoveTaskRequest struct { - ID string `json:"id"` - WorkflowID string `json:"workflow_id"` - WorkflowStepID string `json:"workflow_step_id"` - Position int `json:"position"` + ID string `json:"id"` + WorkflowID string `json:"workflow_id"` + WorkflowStepID string `json:"workflow_step_id"` + Position int `json:"position"` + EntryOptions *workflowmove.EntryOptions `json:"entry_options,omitempty"` } func (h *TaskHandlers) wsMoveTask(ctx context.Context, msg *ws.Message) (*ws.Message, error) { @@ -426,15 +428,24 @@ func (h *TaskHandlers) wsMoveTask(ctx context.Context, msg *ws.Message) (*ws.Mes req.WorkflowID, req.WorkflowStepID, req.Position, - service.MoveTaskOptions{AllowActivePrimarySession: true, StepHistoryActor: wfmodels.StepTransitionActorHuman}, + service.MoveTaskOptions{ + AllowActivePrimarySession: true, + StepHistoryActor: wfmodels.StepTransitionActorHuman, + EntryOptions: req.EntryOptions, + }, ) if err != nil { + if code, msgText, ok := moveEntryOptionsWSError(err); ok { + return ws.NewError(msg.ID, msg.Action, code, msgText, nil) + } h.logger.Error("failed to move task", zap.Error(err)) return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "Failed to move task", nil) } response := dto.MoveTaskResponse{ - Task: dto.FromTask(result.Task), + Task: dto.FromTask(result.Task), + MoveID: result.MoveID, + EntryOptions: result.EntryOptions, } if result.WorkflowStep != nil { response.WorkflowStep = dto.FromWorkflowStep(result.WorkflowStep) diff --git a/apps/backend/internal/task/models/models.go b/apps/backend/internal/task/models/models.go index ec40d46132..e5f5ede16c 100644 --- a/apps/backend/internal/task/models/models.go +++ b/apps/backend/internal/task/models/models.go @@ -120,6 +120,13 @@ const ( // after queue promotion. It prevents duplicate task.queue_promoted events // from repeating on_enter or auto-start behavior. MetaKeyQueuePromotionPending = "queue_promotion_pending" + // MetaKeyWorkflowMovePending carries the one-shot entry options of a direct + // (immediately applied) workflow move whose target-step entry has not yet + // run. Its value records the source step, the move ID, and the encoded + // entry options. The public task.moved event carries only the move ID; the + // instructions and profile choice ride on this transient marker and are + // cleared once the target entry is dispatched. + MetaKeyWorkflowMovePending = "workflow_move_pending" // MetaKeyManualMoveLifecyclePending identifies an admitted manual move whose // task.moved lifecycle must finish before feeder reconciliation. Its value // records the source step so stale deliveries cannot run the wrong exit. diff --git a/apps/backend/internal/task/models/public_metadata.go b/apps/backend/internal/task/models/public_metadata.go index 05e5f2d73d..afa34b992f 100644 --- a/apps/backend/internal/task/models/public_metadata.go +++ b/apps/backend/internal/task/models/public_metadata.go @@ -4,12 +4,16 @@ import "maps" // PublicTaskMetadata returns a detached task metadata projection for API // responses. Deferred-launch attribution and step-handoff carry are -// server-owned and must not expose their internal state to clients. +// server-owned and must not expose their internal state to clients. The +// transient workflow-move marker carries encoded one-shot instructions and an +// internal move ID; it is server-owned lifecycle state and must never reach a +// public event or DTO. func PublicTaskMetadata(metadata map[string]interface{}) map[string]interface{} { if metadata == nil { return nil } public := maps.Clone(metadata) + delete(public, MetaKeyWorkflowMovePending) delete(public, MetaKeyStepHandoffCarry) deferred, ok := public[MetaKeyDeferredLaunch].(map[string]interface{}) if !ok { diff --git a/apps/backend/internal/task/models/public_metadata_test.go b/apps/backend/internal/task/models/public_metadata_test.go index 4e02b7a2bf..c6ac2615c8 100644 --- a/apps/backend/internal/task/models/public_metadata_test.go +++ b/apps/backend/internal/task/models/public_metadata_test.go @@ -2,6 +2,33 @@ package models import "testing" +// The transient workflow-move marker carries an internal move ID and encoded +// one-shot instruction text. A public projection (task.updated events, REST +// DTOs while a WIP move is pending) must never expose it. +func TestPublicTaskMetadataStripsWorkflowMoveMarker(t *testing.T) { + metadata := map[string]interface{}{ + "title_generated": true, + MetaKeyWorkflowMovePending: map[string]interface{}{ + "from_step_id": "step-1", + "move_id": "move-abc", + "options": `{"instructions":"secret reviewer note"}`, + }, + } + + public := PublicTaskMetadata(metadata) + + if _, leaked := public[MetaKeyWorkflowMovePending]; leaked { + t.Fatal("public metadata must not carry the transient workflow_move_pending marker") + } + if _, ok := public["title_generated"]; !ok { + t.Fatal("public metadata must preserve unrelated keys") + } + // The source map is never mutated by the projection. + if _, ok := metadata[MetaKeyWorkflowMovePending]; !ok { + t.Fatal("PublicTaskMetadata must not mutate the source metadata map") + } +} + func TestPublicTaskMetadataRedactsStepHandoffCarry(t *testing.T) { metadata := map[string]interface{}{ "ordinary": "visible", @@ -23,3 +50,13 @@ func TestPublicTaskMetadataRedactsStepHandoffCarry(t *testing.T) { t.Fatal("redaction must not mutate stored metadata") } } + +func TestPublicTaskMetadataNilAndEmpty(t *testing.T) { + if got := PublicTaskMetadata(nil); got != nil { + t.Fatalf("nil metadata must project to nil, got %v", got) + } + got := PublicTaskMetadata(map[string]interface{}{}) + if got == nil || len(got) != 0 { + t.Fatalf("empty metadata must project to an empty map, got %v", got) + } +} diff --git a/apps/backend/internal/task/repository/sqlite/task.go b/apps/backend/internal/task/repository/sqlite/task.go index 91d07c8d6c..ab48845842 100644 --- a/apps/backend/internal/task/repository/sqlite/task.go +++ b/apps/backend/internal/task/repository/sqlite/task.go @@ -22,6 +22,7 @@ import ( "github.com/kandev/kandev/internal/task/models" usermodels "github.com/kandev/kandev/internal/user/models" wfmodels "github.com/kandev/kandev/internal/workflow/models" + workflowmove "github.com/kandev/kandev/internal/workflow/move" v1 "github.com/kandev/kandev/pkg/api/v1" ) @@ -970,6 +971,13 @@ func (r *Repository) rebaseTaskForStepAdmissionCAS( return true, nil } +// Inner descriptor keys for the one-shot MetaKeyWorkflowMovePending marker. +const ( + pendingMoveFromStepIDKey = "from_step_id" + pendingMoveIDKey = "move_id" + pendingMoveOptionsKey = "options" +) + func (r *Repository) updateTaskWithWorkflowStepAdmission( ctx context.Context, task *models.Task, @@ -1062,6 +1070,21 @@ func (r *Repository) updateTaskWithWorkflowStepAdmission( } } } + // A deferred move that cannot enter because the target is full has already + // consumed its pending-move row in this transaction. Keep its one-shot + // options on the queued task before the task update and pending-row delete + // commit, so promotion cannot observe the destination without its options. + if deferredMove != nil && !admitted && deferredMove.Move.EntryOptions != nil { + encoded, err := workflowmove.EncodeEntryOptionsJSON(deferredMove.Move.EntryOptions) + if err != nil { + return false, false, fmt.Errorf("encode deferred workflow move options: %w", err) + } + task.Metadata[models.MetaKeyWorkflowMovePending] = map[string]interface{}{ + pendingMoveFromStepIDKey: expectedStepID, + pendingMoveIDKey: deferredMove.Move.MoveID, + pendingMoveOptionsKey: string(encoded), + } + } metadata, err := json.Marshal(task.Metadata) if err != nil { metadata = []byte("{}") diff --git a/apps/backend/internal/task/repository/sqlite/task_deferred_move_test.go b/apps/backend/internal/task/repository/sqlite/task_deferred_move_test.go index 3c7766404a..bf0eaa8e6f 100644 --- a/apps/backend/internal/task/repository/sqlite/task_deferred_move_test.go +++ b/apps/backend/internal/task/repository/sqlite/task_deferred_move_test.go @@ -8,8 +8,98 @@ import ( "github.com/kandev/kandev/internal/orchestrator/messagequeue" "github.com/kandev/kandev/internal/task/models" + workflowmove "github.com/kandev/kandev/internal/workflow/move" ) +func TestUpdateTaskWithWorkflowStepAdmissionForDeferredMovePersistsQueuedEntryOptionsAtomically(t *testing.T) { + repo := newRepoForEntityTests(t) + ctx := context.Background() + seedWorkspace(t, repo, "workspace-deferred-options") + if err := repo.CreateWorkflow(ctx, &models.Workflow{ID: "workflow-deferred-options", WorkspaceID: "workspace-deferred-options", Name: "Workflow"}); err != nil { + t.Fatal(err) + } + seedCASWorkflowStep(t, repo, "workflow-deferred-options", "step-source-options", 0) + seedCASWorkflowStep(t, repo, "workflow-deferred-options", "step-target-options", 1) + + occupant := &models.Task{ + ID: "task-deferred-options-occupant", WorkspaceID: "workspace-deferred-options", WorkflowID: "workflow-deferred-options", + WorkflowStepID: "step-target-options", Title: "Occupant", WIPAdmitted: true, + } + if err := repo.CreateTask(ctx, occupant); err != nil { + t.Fatal(err) + } + candidate := &models.Task{ + ID: "task-deferred-options", WorkspaceID: "workspace-deferred-options", WorkflowID: "workflow-deferred-options", + WorkflowStepID: "step-source-options", Title: "Deferred candidate", WIPAdmitted: true, + } + if err := repo.CreateTask(ctx, candidate); err != nil { + t.Fatal(err) + } + session := &models.TaskSession{ + ID: "session-deferred-options", TaskID: candidate.ID, QueueIncarnationID: "incarnation-options", + State: models.TaskSessionStateWaitingForInput, StartedAt: time.Now().UTC(), UpdatedAt: time.Now().UTC(), + } + if err := repo.CreateTaskSession(ctx, session); err != nil { + t.Fatal(err) + } + queueRepo, err := messagequeue.NewSQLiteRepository(repo.db, repo.ro) + if err != nil { + t.Fatal(err) + } + move := messagequeue.PendingMove{ + MoveID: "move-deferred-options", SessionIncarnationID: session.QueueIncarnationID, + TaskID: candidate.ID, WorkflowID: candidate.WorkflowID, WorkflowStepID: "step-target-options", + QueuedAt: time.Now().UTC(), EntryOptions: &workflowmove.EntryOptions{ + ResetContext: true, Instructions: "focus on the auth bug", SkipStepPrompt: true, + }, + } + if err := queueRepo.SetPendingMove(ctx, session.ID, &move); err != nil { + t.Fatal(err) + } + + admitted, applied, err := repo.UpdateTaskWithWorkflowStepAdmissionForDeferredMove( + ctx, candidate, "step-source-options", "step-target-options", 1, + messagequeue.PendingMoveRecord{SessionID: session.ID, Move: move}, + ) + if err != nil { + t.Fatal(err) + } + if admitted || !applied { + t.Fatalf("admitted=%t applied=%t, want queued applied move", admitted, applied) + } + + stored, err := repo.GetTask(ctx, candidate.ID) + if err != nil { + t.Fatal(err) + } + if stored.WorkflowStepID != "step-target-options" || stored.QueuedForStepID != "step-target-options" || stored.WIPAdmitted { + t.Fatalf("candidate placement = %+v, want queued at target", stored) + } + marker, ok := stored.Metadata[models.MetaKeyWorkflowMovePending].(map[string]interface{}) + if !ok { + t.Fatalf("queued deferred move must retain a workflow move marker, metadata=%+v", stored.Metadata) + } + if marker["from_step_id"] != "step-source-options" || marker["move_id"] != move.MoveID { + t.Fatalf("marker identity = %+v, want source and move ID", marker) + } + encoded, ok := marker["options"].(string) + if !ok || encoded == "" { + t.Fatalf("marker options = %v, want encoded entry options", marker["options"]) + } + decoded, err := workflowmove.DecodeEntryOptionsJSON([]byte(encoded)) + if err != nil { + t.Fatal(err) + } + if *decoded != *move.EntryOptions { + t.Fatalf("decoded marker options = %+v, want %+v", decoded, move.EntryOptions) + } + if pending, err := queueRepo.GetPendingMove(ctx, session.ID); err != nil { + t.Fatal(err) + } else if pending != nil { + t.Fatalf("deferred move row must be consumed atomically, got %+v", pending) + } +} + func TestUpdateTaskWithWorkflowStepAdmissionForDeferredMoveRejectsReplacementSession(t *testing.T) { repo := newRepoForEntityTests(t) ctx := context.Background() diff --git a/apps/backend/internal/task/service/service_events.go b/apps/backend/internal/task/service/service_events.go index 7d2239903a..88b8bd9192 100644 --- a/apps/backend/internal/task/service/service_events.go +++ b/apps/backend/internal/task/service/service_events.go @@ -819,7 +819,7 @@ func serializeTaskWorkspaceFolders(folders []*models.TaskWorkspaceFolder) []map[ // publishTaskMovedEvent publishes a task.moved event so the orchestrator can process // on_exit/on_enter actions for the new workflow step. -func (s *Service) publishTaskMovedEvent(ctx context.Context, task *models.Task, fromWorkflowID, fromStepID, toStepID, sessionID string) { +func (s *Service) publishTaskMovedEvent(ctx context.Context, task *models.Task, fromWorkflowID, fromStepID, toStepID, sessionID, moveID string) { if s.eventBus == nil { return } @@ -830,6 +830,7 @@ func (s *Service) publishTaskMovedEvent(ctx context.Context, task *models.Task, data := map[string]interface{}{ "task_id": task.ID, "step_transition_id": task.WorkflowStepTransitionID, + "move_id": moveID, "from_workflow_id": fromWorkflowID, "to_workflow_id": task.WorkflowID, "from_step_id": fromStepID, diff --git a/apps/backend/internal/task/service/service_workflow.go b/apps/backend/internal/task/service/service_workflow.go index a077caa7ad..5d8a616b84 100644 --- a/apps/backend/internal/task/service/service_workflow.go +++ b/apps/backend/internal/task/service/service_workflow.go @@ -2,10 +2,12 @@ package service import ( "context" + "encoding/json" "errors" "fmt" "time" + "github.com/google/uuid" "go.uber.org/zap" v1 "github.com/kandev/kandev/pkg/api/v1" @@ -18,6 +20,13 @@ import ( "github.com/kandev/kandev/internal/task/models" "github.com/kandev/kandev/internal/task/repository/repoerrors" wfmodels "github.com/kandev/kandev/internal/workflow/models" + workflowmove "github.com/kandev/kandev/internal/workflow/move" +) + +const ( + workflowMoveFromStepIDKey = "from_step_id" + workflowMoveIDKey = "move_id" + workflowMoveOptionsKey = "options" ) // ApproveSessionResult contains the result of approving a session @@ -401,6 +410,12 @@ type MoveTaskResult struct { // this call's earlier pre-move snapshot — see Task.FromStepID's doc. FromStepID string Transitioned bool + // MoveID correlates the one-shot entry options carried on the transient + // move marker with the target-step entry. Empty for an option-less move. + MoveID string + // EntryOptions is the normalized one-shot override accepted for this move, + // or nil for an ordinary move. + EntryOptions *workflowmove.EntryOptions } // MoveTaskOptions controls non-default move behavior for trusted callers. @@ -436,6 +451,10 @@ type MoveTaskOptions struct { // performs and fails with ErrWorkflowResolutionConflict on a mismatch, // instead of silently reverting whatever the concurrent move just did. ExpectedWorkflowID *string + // EntryOptions are one-shot values applied when the orchestrator enters the + // target workflow step. They are persisted privately on a transient task + // marker and are never included in task.moved event payloads. + EntryOptions *workflowmove.EntryOptions } // ErrWorkflowResolutionConflict indicates a caller's pre-resolved "current @@ -491,6 +510,14 @@ type workflowMoveConflictRepository interface { UpdateTaskIfWorkflowMatches(ctx context.Context, task *models.Task, expectedWorkflowID string) error } +// metadataKeyRemoverRepository clears a single task metadata key in its own +// statement, without rewriting the concurrent fields a full UpdateTask would +// carry. Used to strip a stranded workflow_move_pending marker that a committed +// optioned move left behind when its write produced no step transition. +type metadataKeyRemoverRepository interface { + RemoveTaskMetadataKey(ctx context.Context, taskID, key string) (bool, error) +} + type workflowQueuedTaskPromoter interface { PromoteQueuedTaskIfWorkflowStepHasCapacity(ctx context.Context, task *models.Task, fromStepID, destinationStepID string, limit int) (bool, error) } @@ -550,6 +577,40 @@ func (s *Service) MoveTaskWithOptions( oldStepID := task.WorkflowStepID oldState := task.State stepChanged := oldStepID != workflowStepID + + entryOptions, err := workflowmove.NormalizeEntryOptions(opts.EntryOptions, "") + if err != nil { + return nil, err + } + change := workflowmove.MoveChangePositionOnly + if stepChanged { + change = workflowmove.MoveChangeStep + } + if err := workflowmove.ValidateEntryOptions(entryOptions, change); err != nil { + return nil, err + } + // Only a new optioned move conflicts with a marker already in flight. A + // plain move falls through and clears a stranded marker below (moveID == "") + // so a stale marker can never permanently block ordinary moves. + if entryOptions != nil && stepChanged && task.Metadata != nil { + if _, pending := task.Metadata[models.MetaKeyWorkflowMovePending]; pending { + return nil, workflowmove.ErrMoveConflict + } + } + // Entry options need a recipient. A step without auto-start can still + // receive a one-shot hand-off through an active task session, but a + // session-less move would otherwise accept and permanently drop the + // instructions/profile after the task update is committed. + if entryOptions != nil && targetStep != nil && + !targetStep.HasOnEnterAction(wfmodels.OnEnterAutoStartAgent) && + s.resolvePrimaryOrActiveSession(ctx, id) == nil { + return nil, workflowmove.ErrEntryTargetUnavailable + } + moveID, optionsJSON, err := prepareWorkflowMoveMarker(entryOptions) + if err != nil { + return nil, err + } + stateAfterAdmission := *task if stepChanged { if err := s.syncTaskStateForWorkflowMove(ctx, &stateAfterAdmission, oldStepID, workflowStepID, opts); err != nil { @@ -570,6 +631,18 @@ func (s *Service) MoveTaskWithOptions( task.Metadata[models.MetaKeyQueuedMoveExitPending] = map[string]interface{}{ "from_step_id": oldStepID, } + // The one-shot options ride on the pending marker itself: it is the + // sole live transport for a direct optioned move. task.moved publishes + // only the move ID; the orchestrator reads this marker at target entry. + if moveID != "" { + task.Metadata[models.MetaKeyWorkflowMovePending] = map[string]interface{}{ + workflowMoveFromStepIDKey: oldStepID, + workflowMoveIDKey: moveID, + workflowMoveOptionsKey: string(optionsJSON), + } + } else { + delete(task.Metadata, models.MetaKeyWorkflowMovePending) + } delete(task.Metadata, models.MetaKeyQueuedMoveExitCompleted) delete(task.Metadata, models.MetaKeyQueuePromotionPending) delete(task.Metadata, models.MetaKeyManualMoveLifecycleCompleted) @@ -642,6 +715,19 @@ func (s *Service) MoveTaskWithOptions( resultFromWorkflowID = oldWorkflowID } + // An optioned move (moveID != "") always changed step at read time — + // ValidateEntryOptions rejects a position-only optioned move — so a + // committed write with no transition means the task already occupied + // workflowStepID when the atomic write ran: a concurrent move landed it + // there between this call's read and its write. The one-shot options were + // persisted on the pending marker for a target entry that will never fire + // task.moved, so the marker would strand and the override silently never + // apply. Clear it and report the conflict instead of returning a MoveID and + // EntryOptions the orchestrator will never consume. + if moveID != "" && !resultTransitioned { + return s.rejectStrandedOptionedMove(ctx, task, resultFromWorkflowID) + } + s.publishTaskEvent(ctx, events.TaskUpdated, task, nil, resultFromWorkflowID) if oldState != task.State { s.publishTaskEvent(ctx, events.TaskStateChanged, task, &oldState) @@ -661,7 +747,7 @@ func (s *Service) MoveTaskWithOptions( // are always the source step and workflow the task actually left on this // commit. if resultTransitioned { - s.publishTaskMovedEvent(ctx, task, resultFromWorkflowID, resultFromStepID, workflowStepID, sessionID) + s.publishTaskMovedEvent(ctx, task, resultFromWorkflowID, resultFromStepID, workflowStepID, sessionID, moveID) historySessionID := opts.StepHistorySessionID if historySessionID == "" { historySessionID = sessionID @@ -686,7 +772,7 @@ func (s *Service) MoveTaskWithOptions( zap.String("workflow_step_id", workflowStepID), zap.Int("position", position)) - result := &MoveTaskResult{Task: task, FromStepID: resultFromStepID, Transitioned: resultTransitioned} + result := &MoveTaskResult{Task: task, FromStepID: resultFromStepID, Transitioned: resultTransitioned, MoveID: moveID, EntryOptions: entryOptions} // Fetch the workflow step info if getter is available if s.workflowStepGetter != nil { @@ -704,6 +790,40 @@ func (s *Service) MoveTaskWithOptions( return result, nil } +// rejectStrandedOptionedMove undoes an optioned move whose committed write +// produced no step transition (the task already occupied the target step). The +// write persisted the workflow_move_pending marker, but the zero-transition +// result means task.moved never fires to consume it, so the marker is removed +// with a single-key delete that does not clobber the concurrent move's fields. +// A TaskUpdated event keeps subscribers converged on the cleared row, and the +// caller receives ErrMoveConflict rather than a false success. +func (s *Service) rejectStrandedOptionedMove(ctx context.Context, task *models.Task, fromWorkflowID string) (*MoveTaskResult, error) { + remover, ok := s.tasks.(metadataKeyRemoverRepository) + if !ok { + return nil, fmt.Errorf("metadata key remover repository unavailable for task %s", task.ID) + } + if _, err := remover.RemoveTaskMetadataKey(ctx, task.ID, models.MetaKeyWorkflowMovePending); err != nil { + return nil, fmt.Errorf("clear stranded workflow move marker for task %s: %w", task.ID, err) + } + delete(task.Metadata, models.MetaKeyWorkflowMovePending) + s.publishTaskEvent(ctx, events.TaskUpdated, task, nil, fromWorkflowID) + return nil, workflowmove.ErrMoveConflict +} + +// prepareWorkflowMoveMarker mints a correlation ID and encodes the one-shot +// options for the workflow_move_pending marker. The marker is the sole live +// transport for a direct optioned move; nil options mean an ordinary move. +func prepareWorkflowMoveMarker(entryOptions *workflowmove.EntryOptions) (string, json.RawMessage, error) { + if entryOptions == nil { + return "", nil, nil + } + optionsJSON, err := workflowmove.EncodeEntryOptionsJSON(entryOptions) + if err != nil { + return "", nil, fmt.Errorf("encode workflow move options: %w", err) + } + return uuid.NewString(), optionsJSON, nil +} + func (s *Service) terminalWorkflowStep(ctx context.Context, workflowStepID string) (bool, error) { if s.workflowStepGetter == nil || workflowStepID == "" { return false, nil @@ -974,7 +1094,7 @@ func (s *Service) promoteFeederQueuedTask(ctx context.Context, candidate *models s.publishTaskEvent(ctx, events.TaskStateChanged, candidate, &oldState) } s.recordQueuedPromotion(ctx, candidate.ID, fromStepID, targetStep.ID) - s.publishTaskMovedEvent(ctx, candidate, oldWorkflowID, fromStepID, targetStep.ID, sessionID) + s.publishTaskMovedEvent(ctx, candidate, oldWorkflowID, fromStepID, targetStep.ID, sessionID, "") return true } else if admissionRepo, ok := s.tasks.(workflowMoveAdmissionRepository); ok { claimed, err := admissionRepo.UpdateTaskWithWorkflowStepAdmission(ctx, candidate, targetStep.ID, targetStep.WIPLimit) @@ -992,7 +1112,7 @@ func (s *Service) promoteFeederQueuedTask(ctx context.Context, candidate *models s.publishTaskEvent(ctx, events.TaskStateChanged, candidate, &oldState) } s.recordQueuedPromotion(ctx, candidate.ID, fromStepID, targetStep.ID) - s.publishTaskMovedEvent(ctx, candidate, oldWorkflowID, fromStepID, targetStep.ID, sessionID) + s.publishTaskMovedEvent(ctx, candidate, oldWorkflowID, fromStepID, targetStep.ID, sessionID, "") return true } // ctx here still carries the identity of whoever triggered the move that diff --git a/apps/backend/internal/task/service/service_workflow_stranded_marker_test.go b/apps/backend/internal/task/service/service_workflow_stranded_marker_test.go new file mode 100644 index 0000000000..7e4a638356 --- /dev/null +++ b/apps/backend/internal/task/service/service_workflow_stranded_marker_test.go @@ -0,0 +1,130 @@ +package service + +import ( + "context" + "errors" + "testing" + + "github.com/kandev/kandev/internal/task/models" + workflowmove "github.com/kandev/kandev/internal/workflow/move" +) + +// seedStrandedMarker plants a workflow_move_pending marker on a task so tests +// can exercise recovery from a move that never cleared its own marker. +func seedStrandedMarker(t *testing.T, ctx context.Context, repo interface { + GetTask(context.Context, string) (*models.Task, error) + UpdateTask(context.Context, *models.Task) error +}, taskID, fromStepID string) { + t.Helper() + task, err := repo.GetTask(ctx, taskID) + if err != nil { + t.Fatalf("GetTask(%s): %v", taskID, err) + } + if task.Metadata == nil { + task.Metadata = map[string]interface{}{} + } + task.Metadata[models.MetaKeyWorkflowMovePending] = map[string]interface{}{ + "from_step_id": fromStepID, + "move_id": "stranded-move", + "options": "{}", + } + if err := repo.UpdateTask(ctx, task); err != nil { + t.Fatalf("UpdateTask(%s): %v", taskID, err) + } +} + +// TestService_MoveTaskClearsStrandedPendingMarker proves a plain move (no entry +// options) is never blocked by a stranded marker and clears it, so a task can +// never get permanently stuck behind a pending marker that failed to complete. +func TestService_MoveTaskClearsStrandedPendingMarker(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + seedMoveWorkflows(t, ctx, repo) + seedMoveSteps(svc) + createMoveTask(t, ctx, repo, "task-stranded", "wf-source", "step-source", nil) + seedStrandedMarker(t, ctx, repo, "task-stranded", "step-source") + + moved, err := svc.MoveTask(ctx, "task-stranded", "wf-source", "step-review-target", 0) + if err != nil { + t.Fatalf("plain move should clear a stranded marker, got: %v", err) + } + if moved.Task.WorkflowStepID != "step-review-target" { + t.Fatalf("expected step-review-target, got %s", moved.Task.WorkflowStepID) + } + stored, err := repo.GetTask(ctx, "task-stranded") + if err != nil { + t.Fatalf("GetTask: %v", err) + } + if _, pending := stored.Metadata[models.MetaKeyWorkflowMovePending]; pending { + t.Fatalf("expected stranded marker cleared, metadata=%+v", stored.Metadata) + } +} + +// TestService_MoveTaskWithOptionsConflictsWithPendingMarker proves a new +// optioned move still fails closed when a marker is already in flight, so two +// concurrent one-shot moves cannot both persist their options. +func TestService_MoveTaskWithOptionsConflictsWithPendingMarker(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + seedMoveWorkflows(t, ctx, repo) + seedMoveSteps(svc) + createMoveTask(t, ctx, repo, "task-conflict", "wf-source", "step-source", nil) + createMoveSession(t, ctx, repo, "session-conflict", "task-conflict", models.TaskSessionStateWaitingForInput, models.ReviewStatusNone) + seedStrandedMarker(t, ctx, repo, "task-conflict", "step-source") + + _, err := svc.MoveTaskWithOptions(ctx, "task-conflict", "wf-source", "step-review-target", 0, MoveTaskOptions{ + AllowActivePrimarySession: true, + EntryOptions: &workflowmove.EntryOptions{Instructions: "please review"}, + }) + if !errors.Is(err, workflowmove.ErrMoveConflict) { + t.Fatalf("expected ErrMoveConflict, got %v", err) + } +} + +// TestService_MoveTaskWithOptionsRejectsZeroTransitionCommit proves that when an +// optioned move's atomic write finds the task already on the target step (a +// concurrent move landed it there between this call's read and its write), the +// committed workflow_move_pending marker is cleared and the move is reported as +// ErrMoveConflict. Left unfixed the marker strands in metadata — task.moved +// never fires on a zero-transition write, so nothing consumes it — while the +// caller was told its one-shot override was accepted. +func TestService_MoveTaskWithOptionsRejectsZeroTransitionCommit(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + seedMoveWorkflows(t, ctx, repo) + seedMoveSteps(svc) + createMoveTask(t, ctx, repo, "task-zero-transition", "wf-source", "step-source", nil) + createMoveSession(t, ctx, repo, "session-zero", "task-zero-transition", models.TaskSessionStateWaitingForInput, models.ReviewStatusNone) + + raceRepo := &workflowMoveRaceRepo{Repository: repo} + raceRepo.inject = func() { + // A concurrent, legitimate move lands the task on step-review-target + // before this call's own write commits, so that write produces no step + // transition. + if _, err := svc.MoveTaskWithOptions(ctx, "task-zero-transition", "wf-source", "step-review-target", 0, MoveTaskOptions{ + AllowActivePrimarySession: true, + }); err != nil { + t.Fatalf("concurrent legitimate move (injected mid-call) failed: %v", err) + } + } + svc.tasks = raceRepo + + _, err := svc.MoveTaskWithOptions(ctx, "task-zero-transition", "wf-source", "step-review-target", 0, MoveTaskOptions{ + AllowActivePrimarySession: true, + EntryOptions: &workflowmove.EntryOptions{Instructions: "please review"}, + }) + if !errors.Is(err, workflowmove.ErrMoveConflict) { + t.Fatalf("expected ErrMoveConflict on zero-transition committed write, got %v", err) + } + + stored, err := repo.GetTask(ctx, "task-zero-transition") + if err != nil { + t.Fatalf("GetTask: %v", err) + } + if _, pending := stored.Metadata[models.MetaKeyWorkflowMovePending]; pending { + t.Fatalf("expected stranded workflow_move_pending marker cleared, metadata=%+v", stored.Metadata) + } + if stored.WorkflowStepID != "step-review-target" { + t.Fatalf("concurrent move must survive the rejected optioned move: got step %s, want step-review-target", stored.WorkflowStepID) + } +} diff --git a/apps/backend/internal/task/service/service_workflow_test.go b/apps/backend/internal/task/service/service_workflow_test.go index 2f07b09957..cee119a919 100644 --- a/apps/backend/internal/task/service/service_workflow_test.go +++ b/apps/backend/internal/task/service/service_workflow_test.go @@ -10,6 +10,7 @@ import ( "github.com/kandev/kandev/internal/events/bus" "github.com/kandev/kandev/internal/task/models" wfmodels "github.com/kandev/kandev/internal/workflow/models" + workflowmove "github.com/kandev/kandev/internal/workflow/move" v1 "github.com/kandev/kandev/pkg/api/v1" ) @@ -168,6 +169,57 @@ func TestService_MoveTaskRejectsInvalidWorkflowTargets(t *testing.T) { } } +func TestService_MoveTaskWithEntryOptionsPersistsPendingMarker(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + seedMoveWorkflows(t, ctx, repo) + seedMoveSteps(svc) + createMoveTask(t, ctx, repo, "task-opts", "wf-source", "step-source", nil) + // An idle active session makes the target step (no auto-start) a valid + // recipient for the one-shot instructions. + createMoveSession(t, ctx, repo, "session-opts", "task-opts", models.TaskSessionStateWaitingForInput, models.ReviewStatusNone) + + result, err := svc.MoveTaskWithOptions(ctx, "task-opts", "wf-source", "step-review-target", 0, MoveTaskOptions{ + AllowActivePrimarySession: true, + EntryOptions: &workflowmove.EntryOptions{Instructions: "please review"}, + }) + if err != nil { + t.Fatalf("MoveTaskWithOptions: %v", err) + } + if result.MoveID == "" || result.EntryOptions == nil { + t.Fatalf("expected move id and entry options on result, got %+v", result) + } + stored, err := repo.GetTask(ctx, "task-opts") + if err != nil { + t.Fatalf("GetTask: %v", err) + } + marker, ok := stored.Metadata[models.MetaKeyWorkflowMovePending].(map[string]interface{}) + if !ok { + t.Fatalf("expected workflow_move_pending marker, metadata=%+v", stored.Metadata) + } + if marker["move_id"] != result.MoveID { + t.Errorf("marker move_id = %v, want %s", marker["move_id"], result.MoveID) + } + if encoded, _ := marker["options"].(string); encoded == "" { + t.Error("expected encoded options on marker") + } +} + +func TestService_MoveTaskWithEntryOptionsRejectsPositionOnly(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + seedMoveWorkflows(t, ctx, repo) + seedMoveSteps(svc) + createMoveTask(t, ctx, repo, "task-noop", "wf-source", "step-source", nil) + + _, err := svc.MoveTaskWithOptions(ctx, "task-noop", "wf-source", "step-source", 1, MoveTaskOptions{ + EntryOptions: &workflowmove.EntryOptions{Instructions: "noop"}, + }) + if !errors.Is(err, workflowmove.ErrEntryOptionsRequireStepChange) { + t.Fatalf("expected ErrEntryOptionsRequireStepChange, got %v", err) + } +} + func TestService_MoveTaskAllowsPendingReviewWhenSessionIdle(t *testing.T) { svc, _, repo := createTestService(t) ctx := context.Background() diff --git a/apps/backend/internal/workflow/move/errors.go b/apps/backend/internal/workflow/move/errors.go new file mode 100644 index 0000000000..2402998110 --- /dev/null +++ b/apps/backend/internal/workflow/move/errors.go @@ -0,0 +1,49 @@ +package move + +import ( + "errors" +) + +// Stable classification sentinels for move validation. Error messages never +// include option values because instructions may be sensitive user input. +var ( + ErrConflictingInstructions = errors.New("entry options instructions conflict with legacy prompt") + ErrEntryOptionsRequireStepChange = errors.New("entry options require a workflow step change") + ErrEntryTargetUnavailable = errors.New("workflow move entry target has no session or auto-start") + ErrEntryOptionsUnsupported = errors.New("workflow move entry options unsupported") + ErrMoveConflict = errors.New("another workflow move is already pending") + ErrPermanentPendingMoveMismatch = errors.New("pending workflow move is permanently invalid") + // ErrPendingMoveActiveSession is transient: a deferred move must remain + // queued until every session other than its source has stopped running. + ErrPendingMoveActiveSession = errors.New("pending workflow move has another active session") +) + +// ConflictingInstructionsError reports that the nested instructions and the +// legacy prompt alias both supplied different values. It intentionally carries +// no user-provided strings. +type ConflictingInstructionsError struct{} + +func (ConflictingInstructionsError) Error() string { + return ErrConflictingInstructions.Error() +} + +func (ConflictingInstructionsError) Unwrap() error { + return ErrConflictingInstructions +} + +// EntryOptionsNotAllowedError reports that options were supplied for a move +// which does not change workflow step. Change is a closed, non-sensitive enum. +type EntryOptionsNotAllowedError struct { + Change MoveChange +} + +func (e EntryOptionsNotAllowedError) Error() string { + if e.Change == MoveChangePositionOnly { + return "entry options are not allowed for a position-only move" + } + return "entry options are not allowed for a move without a workflow step change" +} + +func (EntryOptionsNotAllowedError) Unwrap() error { + return ErrEntryOptionsRequireStepChange +} diff --git a/apps/backend/internal/workflow/move/normalization.go b/apps/backend/internal/workflow/move/normalization.go new file mode 100644 index 0000000000..144372f461 --- /dev/null +++ b/apps/backend/internal/workflow/move/normalization.go @@ -0,0 +1,41 @@ +package move + +import "strings" + +// NormalizeEntryOptions trims all string fields, folds the legacy top-level +// prompt into instructions, and returns nil for an empty options object. +// +// The legacy prompt is accepted only when the nested instructions field is +// absent. Supplying both non-blank values is rejected, even when they match, +// so callers use one unambiguous request shape. +func NormalizeEntryOptions(options *EntryOptions, legacyPrompt string) (*EntryOptions, error) { + legacyPrompt = strings.TrimSpace(legacyPrompt) + normalized := normalizedCopy(options) + if normalized == nil { + normalized = &EntryOptions{} + } + + if normalized.Instructions != "" && legacyPrompt != "" { + return nil, &ConflictingInstructionsError{} + } + if normalized.Instructions == "" { + normalized.Instructions = legacyPrompt + } + if *normalized == (EntryOptions{}) { + return nil, nil + } + return normalized, nil +} + +// ValidateEntryOptions rejects agent-facing options for a move that does not +// actually change workflow step. Empty options are always a no-op and remain +// valid for position-only or no-step moves. +func ValidateEntryOptions(options *EntryOptions, change MoveChange) error { + if normalizedCopy(options) == nil { + return nil + } + if change != MoveChangeStep { + return &EntryOptionsNotAllowedError{Change: change} + } + return nil +} diff --git a/apps/backend/internal/workflow/move/overlay.go b/apps/backend/internal/workflow/move/overlay.go new file mode 100644 index 0000000000..6688a3391f --- /dev/null +++ b/apps/backend/internal/workflow/move/overlay.go @@ -0,0 +1,118 @@ +package move + +import ( + "strings" + + "github.com/kandev/kandev/internal/workflow/models" +) + +const ( + InstructionsHeading = "## One-time workflow move instructions" + InstructionsEnd = "" +) + +// OverlayStep returns a copy of step with one-shot move-entry options applied. +// The original step is never mutated. Reset is OR-ed onto the copy and never +// disables an existing reset_agent_context action. +// +// Without skip_step_prompt, instructions are appended after the durable prompt. +// With skip_step_prompt the durable prompt (and its task-description fallback) +// is suppressed for this entry: when instructions are present the copy's prompt +// becomes only those instructions, and when they are absent the copy's +// auto_start_agent action is dropped so no turn starts and the task lands idle. +func OverlayStep(step *models.WorkflowStep, opts *EntryOptions) *models.WorkflowStep { + if step == nil { + return nil + } + + copy := *step + copy.Events.OnEnter = append([]models.OnEnterAction(nil), step.Events.OnEnter...) + if opts == nil { + return © + } + + wrapped := wrapMoveInstructions(opts.Instructions) + switch { + case opts.SkipStepPrompt && wrapped != "": + // Skip the step prompt/task description; send only the one-time + // instructions. The workflow-level instructions block, if any, is still + // added downstream by buildWorkflowPrompt. + copy.Prompt = wrapped + case opts.SkipStepPrompt: + // Skip with no instructions: suppress the auto-started turn entirely by + // dropping auto_start_agent, so the on_enter path treats this like a + // step without auto-start and the agent waits for manual input. + copy.Prompt = "" + copy.Events.OnEnter = withoutOnEnterAction(copy.Events.OnEnter, models.OnEnterAutoStartAgent) + case wrapped != "": + if copy.Prompt == "" { + // Preserve empty-prompt composition: durable steps with no prompt + // still carry the task description via {{task_prompt}}. + copy.Prompt = "{{task_prompt}}\n\n" + wrapped + } else { + copy.Prompt = copy.Prompt + "\n\n" + wrapped + } + } + if opts.ResetContext && !copy.HasOnEnterAction(models.OnEnterResetAgentContext) { + copy.Events.OnEnter = append(copy.Events.OnEnter, models.OnEnterAction{ + Type: models.OnEnterResetAgentContext, + }) + } + return © +} + +// withoutOnEnterAction returns actions with every entry of the given type +// removed. The input slice is never mutated. +func withoutOnEnterAction(actions []models.OnEnterAction, drop models.OnEnterActionType) []models.OnEnterAction { + result := make([]models.OnEnterAction, 0, len(actions)) + for _, action := range actions { + if action.Type == drop { + continue + } + result = append(result, action) + } + return result +} + +// WrapInstructions renders one-shot move instructions with the same sentinel +// markers OverlayStep uses, so the auto-start prompt path (which rebuilds the +// prompt from the durable step and cannot see a step overlay) can append the +// identical block. Empty or whitespace-only input yields an empty string. +func WrapInstructions(instructions string) string { + return wrapMoveInstructions(instructions) +} + +func wrapMoveInstructions(instructions string) string { + instructions = strings.TrimSpace(instructions) + if instructions == "" { + return "" + } + return InstructionsHeading + "\n\n" + instructions + "\n\n" + InstructionsEnd +} + +// StepCarriesMoveInstructions reports whether step.Prompt already contains +// one-shot move instructions (the overlay sentinel). Used by on-enter +// dispatch to decide whether to queue those instructions without +// re-appending them. +func StepCarriesMoveInstructions(step *models.WorkflowStep) bool { + if step == nil { + return false + } + return strings.Contains(step.Prompt, InstructionsEnd) +} + +// ExtractInstructions returns the one-shot instructions block (including its +// sentinel markers) previously appended by OverlayStep/WrapInstructions, or "" +// when the prompt carries none. Used to re-deliver instructions to an existing +// session that will not receive the overlaid step prompt through a launch. +func ExtractInstructions(prompt string) string { + start := strings.Index(prompt, InstructionsHeading) + if start < 0 { + return "" + } + end := strings.Index(prompt, InstructionsEnd) + if end < 0 || end < start { + return "" + } + return prompt[start : end+len(InstructionsEnd)] +} diff --git a/apps/backend/internal/workflow/move/overlay_test.go b/apps/backend/internal/workflow/move/overlay_test.go new file mode 100644 index 0000000000..14088b774d --- /dev/null +++ b/apps/backend/internal/workflow/move/overlay_test.go @@ -0,0 +1,190 @@ +package move + +import ( + "testing" + + "github.com/kandev/kandev/internal/workflow/models" +) + +func TestOverlayStep_DoesNotMutateOriginal(t *testing.T) { + step := &models.WorkflowStep{ + ID: "step-qa", + Prompt: "review the diff", + AgentProfileID: "profile-impl", + Events: models.StepEvents{ + OnEnter: []models.OnEnterAction{{Type: models.OnEnterAutoStartAgent}}, + }, + } + + got := OverlayStep(step, &EntryOptions{ + ResetContext: true, + Instructions: "reproduce the checkout failure", + }) + if got == nil || got == step { + t.Fatalf("OverlayStep() returned %v, want a distinct copy", got) + } + if step.Prompt != "review the diff" || step.AgentProfileID != "profile-impl" { + t.Fatalf("OverlayStep() mutated original step: %+v", *step) + } + if step.HasOnEnterAction(models.OnEnterResetAgentContext) { + t.Fatal("OverlayStep() added reset to the original step") + } + if len(step.Events.OnEnter) != 1 { + t.Fatalf("original OnEnter len = %d, want 1", len(step.Events.OnEnter)) + } +} + +func TestOverlayStep_AppendsInstructionsAndORsReset(t *testing.T) { + step := &models.WorkflowStep{ + Prompt: "review the diff", + AgentProfileID: "profile-impl", + Events: models.StepEvents{ + OnEnter: []models.OnEnterAction{{Type: models.OnEnterAutoStartAgent}}, + }, + } + + got := OverlayStep(step, &EntryOptions{ + ResetContext: true, + Instructions: "reproduce the checkout failure", + }) + wantPrompt := "review the diff\n\n" + InstructionsHeading + "\n\nreproduce the checkout failure\n\n" + InstructionsEnd + if got.Prompt != wantPrompt { + t.Fatalf("prompt = %q, want %q", got.Prompt, wantPrompt) + } + if got.AgentProfileID != "profile-impl" { + t.Fatalf("agent profile = %q, want the durable profile-impl unchanged", got.AgentProfileID) + } + if !got.HasOnEnterAction(models.OnEnterResetAgentContext) { + t.Fatal("reset_context overlay did not add reset_agent_context") + } + if !got.HasOnEnterAction(models.OnEnterAutoStartAgent) { + t.Fatal("overlay dropped auto_start_agent") + } +} + +func TestOverlayStep_SkipStepPromptWithInstructionsReplacesPrompt(t *testing.T) { + step := &models.WorkflowStep{ + Prompt: "review the diff", + Events: models.StepEvents{ + OnEnter: []models.OnEnterAction{{Type: models.OnEnterAutoStartAgent}}, + }, + } + + got := OverlayStep(step, &EntryOptions{ + SkipStepPrompt: true, + Instructions: "reproduce the checkout failure", + }) + wantPrompt := InstructionsHeading + "\n\nreproduce the checkout failure\n\n" + InstructionsEnd + if got.Prompt != wantPrompt { + t.Fatalf("prompt = %q, want only the instructions %q", got.Prompt, wantPrompt) + } + if !got.HasOnEnterAction(models.OnEnterAutoStartAgent) { + t.Fatal("skip with instructions must keep auto_start_agent so the turn runs") + } + if step.Prompt != "review the diff" { + t.Fatalf("OverlayStep() mutated original prompt: %q", step.Prompt) + } +} + +func TestOverlayStep_SkipStepPromptWithoutInstructionsDropsAutoStart(t *testing.T) { + step := &models.WorkflowStep{ + Prompt: "review the diff", + Events: models.StepEvents{ + OnEnter: []models.OnEnterAction{ + {Type: models.OnEnterEnablePlanMode}, + {Type: models.OnEnterAutoStartAgent}, + }, + }, + } + + got := OverlayStep(step, &EntryOptions{SkipStepPrompt: true, ResetContext: true}) + if got.Prompt != "" { + t.Fatalf("skip without instructions must clear the durable prompt, got %q", got.Prompt) + } + if got.HasOnEnterAction(models.OnEnterAutoStartAgent) { + t.Fatal("skip without instructions must drop auto_start_agent so no turn starts") + } + if !got.HasOnEnterAction(models.OnEnterEnablePlanMode) { + t.Fatal("skip must not drop other on_enter actions") + } + if !got.HasOnEnterAction(models.OnEnterResetAgentContext) { + t.Fatal("reset_context stays independent of skip_step_prompt") + } + if len(step.Events.OnEnter) != 2 || !step.HasOnEnterAction(models.OnEnterAutoStartAgent) { + t.Fatalf("OverlayStep() mutated original OnEnter: %+v", step.Events.OnEnter) + } +} + +func TestOverlayStep_ResetNeverDisablesExistingReset(t *testing.T) { + step := &models.WorkflowStep{ + Events: models.StepEvents{ + OnEnter: []models.OnEnterAction{{Type: models.OnEnterResetAgentContext}}, + }, + } + + got := OverlayStep(step, &EntryOptions{Instructions: "handoff"}) + if !got.HasOnEnterAction(models.OnEnterResetAgentContext) { + t.Fatal("overlay disabled the step's existing reset_agent_context") + } +} + +func TestOverlayStep_EmptyPromptKeepsTaskPlaceholder(t *testing.T) { + step := &models.WorkflowStep{AgentProfileID: "profile-impl"} + + got := OverlayStep(step, &EntryOptions{Instructions: "handoff notes"}) + wantPrompt := "{{task_prompt}}\n\n" + InstructionsHeading + "\n\nhandoff notes\n\n" + InstructionsEnd + if got.Prompt != wantPrompt { + t.Fatalf("empty-prompt overlay = %q, want %q", got.Prompt, wantPrompt) + } +} + +func TestStepCarriesMoveInstructions(t *testing.T) { + if StepCarriesMoveInstructions(nil) { + t.Fatal("nil step should not carry move instructions") + } + plain := &models.WorkflowStep{Prompt: "review the diff"} + if StepCarriesMoveInstructions(plain) { + t.Fatal("plain prompt should not carry move instructions") + } + overlaid := OverlayStep(plain, &EntryOptions{Instructions: "handoff notes"}) + if !StepCarriesMoveInstructions(overlaid) { + t.Fatal("overlaid step should carry move instructions") + } +} + +func TestExtractInstructions_RoundTripsWrappedBlock(t *testing.T) { + if got := ExtractInstructions("review the diff"); got != "" { + t.Fatalf("plain prompt should carry no instructions, got %q", got) + } + if got := WrapInstructions(" "); got != "" { + t.Fatalf("whitespace-only instructions should wrap empty, got %q", got) + } + overlaid := OverlayStep(&models.WorkflowStep{Prompt: "review the diff"}, &EntryOptions{Instructions: "reproduce it"}) + block := ExtractInstructions(overlaid.Prompt) + want := WrapInstructions("reproduce it") + if block != want { + t.Fatalf("extracted block = %q, want %q", block, want) + } + // The extracted block is self-contained: re-extracting it yields itself. + if again := ExtractInstructions(block); again != block { + t.Fatalf("re-extracted block = %q, want %q", again, block) + } +} + +func TestOverlayStep_EmptyOptionsCopiesWithoutChangingKnobs(t *testing.T) { + step := &models.WorkflowStep{ + Prompt: "review the diff", + AgentProfileID: "profile-impl", + } + + got := OverlayStep(step, nil) + if got == nil || got == step { + t.Fatalf("OverlayStep(nil opts) returned %v, want a distinct copy", got) + } + if got.Prompt != step.Prompt || got.AgentProfileID != step.AgentProfileID { + t.Fatalf("empty overlay changed knobs: %+v", *got) + } + if OverlayStep(nil, &EntryOptions{ResetContext: true}) != nil { + t.Fatal("OverlayStep(nil step) should return nil") + } +} diff --git a/apps/backend/internal/workflow/move/types.go b/apps/backend/internal/workflow/move/types.go new file mode 100644 index 0000000000..abf7cee742 --- /dev/null +++ b/apps/backend/internal/workflow/move/types.go @@ -0,0 +1,144 @@ +// Package move contains the transport-neutral contract for a one-shot +// workflow-step move. It deliberately has no task, persistence, or +// orchestrator dependencies so every move entry point can share the same +// normalization and validation rules. +package move + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "io" + "strings" +) + +// EntryOptions are one-shot options applied when a task enters a workflow +// step. They do not modify the target step's durable configuration. +// +// The pointer to EntryOptions is the optional nested entry_options value on a +// move request. NormalizeEntryOptions returns nil when the value is empty. +type EntryOptions struct { + ResetContext bool `json:"reset_context,omitempty"` + Instructions string `json:"instructions,omitempty"` + // SkipStepPrompt suppresses the destination step's configured prompt (and + // its task-description fallback) for this one entry. With instructions, the + // agent auto-starts a turn carrying only those instructions; without + // instructions, no turn starts and the task lands idle. + SkipStepPrompt bool `json:"skip_step_prompt,omitempty"` +} + +// UnmarshalJSON keeps all request transports fail-closed when a caller sends +// a misspelled or otherwise unsupported option. Without a typed decoder, +// encoding/json silently drops unknown fields and the move would be accepted +// while applying a different contract than the caller requested. +func (o *EntryOptions) UnmarshalJSON(data []byte) error { + type entryOptions EntryOptions + decoder := json.NewDecoder(bytes.NewReader(data)) + decoder.DisallowUnknownFields() + var decoded entryOptions + if err := decoder.Decode(&decoded); err != nil { + return err + } + var extra interface{} + if err := decoder.Decode(&extra); !errors.Is(err, io.EOF) { + if err == nil { + return errors.New("multiple JSON values") + } + return err + } + *o = EntryOptions(decoded) + return nil +} + +// StepEntryOptions is the explicit workflow-step vocabulary for the nested +// entry_options value. It is an alias so transports do not need duplicate +// shapes while callers can use the name that matches their boundary. +type StepEntryOptions = EntryOptions + +// Options is a short vocabulary alias for callers that refer to the nested +// value as move options. +type Options = EntryOptions + +// MoveChange describes whether a move changes the workflow step or only its +// position metadata. Entry options are meaningful only for Step changes. +type MoveChange string + +const ( + MoveChangeNone MoveChange = "none" + MoveChangePositionOnly MoveChange = "position_only" + MoveChangeStep MoveChange = "step" +) + +// HasOverrides reports whether options contain at least one effective value. +// Whitespace-only strings are treated as absent, matching normalization. +func HasOverrides(options *EntryOptions) bool { + return normalizedCopy(options) != nil +} + +// HasOverrides reports whether this value contains at least one effective +// entry option. +func (o *EntryOptions) HasOverrides() bool { + return HasOverrides(o) +} + +// DecodeEntryOptionsJSON decodes a persisted entry payload strictly. Unknown +// fields and trailing JSON are rejected so corrupted or newer payloads cannot +// be silently treated as an option-less move. +func DecodeEntryOptionsJSON(encoded []byte) (*EntryOptions, error) { + trimmed := bytes.TrimSpace(encoded) + if len(trimmed) == 0 || bytes.Equal(trimmed, []byte("{}")) { + return nil, nil + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(trimmed, &fields); err != nil { + return nil, err + } + if fields == nil { + return nil, errors.New("workflow move entry options must be a JSON object") + } + for _, field := range []string{"reset_context", "instructions", "skip_step_prompt"} { + if raw, ok := fields[field]; ok && bytes.Equal(bytes.TrimSpace(raw), []byte("null")) { + return nil, fmt.Errorf("workflow move entry option %s cannot be null", field) + } + } + var options EntryOptions + decoder := json.NewDecoder(bytes.NewReader(trimmed)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&options); err != nil { + return nil, err + } + if err := decoder.Decode(&struct{}{}); !errors.Is(err, io.EOF) { + if err == nil { + return nil, errors.New("multiple JSON values") + } + return nil, err + } + if !options.HasOverrides() { + return nil, nil + } + return &options, nil +} + +// EncodeEntryOptionsJSON encodes one-shot move options for the pending marker. +// It round-trips with DecodeEntryOptionsJSON: option-less input encodes to the +// empty object, which decode treats as an ordinary move. +func EncodeEntryOptionsJSON(options *EntryOptions) (json.RawMessage, error) { + normalized := normalizedCopy(options) + if normalized == nil { + return json.RawMessage("{}"), nil + } + return json.Marshal(normalized) +} + +func normalizedCopy(options *EntryOptions) *EntryOptions { + if options == nil { + return nil + } + copy := *options + copy.Instructions = strings.TrimSpace(copy.Instructions) + if copy == (EntryOptions{}) { + return nil + } + return © +} diff --git a/apps/backend/internal/workflow/move/types_test.go b/apps/backend/internal/workflow/move/types_test.go new file mode 100644 index 0000000000..ec487c8752 --- /dev/null +++ b/apps/backend/internal/workflow/move/types_test.go @@ -0,0 +1,135 @@ +package move + +import ( + "encoding/json" + "errors" + "strings" + "testing" +) + +func TestNormalizeEntryOptions_TrimsFieldsAndDoesNotMutateInput(t *testing.T) { + input := &EntryOptions{ + ResetContext: true, + Instructions: " reproduce the checkout failure ", + SkipStepPrompt: true, + } + + got, err := NormalizeEntryOptions(input, "") + if err != nil { + t.Fatalf("NormalizeEntryOptions() error = %v", err) + } + if got == input { + t.Fatal("NormalizeEntryOptions() returned the input pointer") + } + want := EntryOptions{ + ResetContext: true, + Instructions: "reproduce the checkout failure", + SkipStepPrompt: true, + } + if *got != want { + t.Fatalf("normalized options = %+v, want %+v", *got, want) + } + if input.Instructions != " reproduce the checkout failure " { + t.Fatalf("NormalizeEntryOptions() mutated input: %+v", *input) + } +} + +func TestNormalizeEntryOptions_UsesLegacyPromptAlias(t *testing.T) { + got, err := NormalizeEntryOptions(nil, " inspect the failing test ") + if err != nil { + t.Fatalf("NormalizeEntryOptions() error = %v", err) + } + if got == nil || got.Instructions != "inspect the failing test" { + t.Fatalf("normalized legacy prompt = %+v, want instructions", got) + } +} + +func TestNormalizeEntryOptions_RejectsLegacyAndNestedPromptWithoutLeakingValues(t *testing.T) { + const secretNested = "nested private handoff" + for _, secretLegacy := range []string{secretNested, "legacy private handoff"} { + _, err := NormalizeEntryOptions(&EntryOptions{Instructions: secretNested}, secretLegacy) + if err == nil { + t.Fatal("NormalizeEntryOptions() error = nil, want conflict") + } + if !errors.Is(err, ErrConflictingInstructions) { + t.Fatalf("error = %v, want ErrConflictingInstructions", err) + } + var conflict *ConflictingInstructionsError + if !errors.As(err, &conflict) { + t.Fatalf("error = %T, want *ConflictingInstructionsError", err) + } + if strings.Contains(err.Error(), secretNested) || strings.Contains(err.Error(), secretLegacy) { + t.Fatalf("conflict error leaked prompt contents: %q", err) + } + } +} + +func TestNormalizeEntryOptions_BlankValuesBecomeAbsent(t *testing.T) { + got, err := NormalizeEntryOptions(&EntryOptions{ + Instructions: " \n\t", + }, " \n") + if err != nil { + t.Fatalf("NormalizeEntryOptions() error = %v", err) + } + if got != nil { + t.Fatalf("normalized blank options = %+v, want nil", *got) + } + if HasOverrides(&EntryOptions{Instructions: " \t"}) { + t.Fatal("HasOverrides(blank options) = true, want false") + } + if (&EntryOptions{ResetContext: true}).HasOverrides() != true { + t.Fatal("HasOverrides(reset_context) = false, want true") + } +} + +func TestValidateEntryOptions_RequiresStepChange(t *testing.T) { + options := &EntryOptions{Instructions: "run QA"} + for _, change := range []MoveChange{MoveChangeNone, MoveChangePositionOnly} { + err := ValidateEntryOptions(options, change) + if err == nil { + t.Fatalf("ValidateEntryOptions(%q) error = nil, want rejection", change) + } + if !errors.Is(err, ErrEntryOptionsRequireStepChange) { + t.Fatalf("ValidateEntryOptions(%q) error = %v, want ErrEntryOptionsRequireStepChange", change, err) + } + var notAllowed *EntryOptionsNotAllowedError + if !errors.As(err, ¬Allowed) { + t.Fatalf("ValidateEntryOptions(%q) error = %T, want *EntryOptionsNotAllowedError", change, err) + } + } +} + +func TestValidateEntryOptions_AllowsStepChangeAndEmptyOptions(t *testing.T) { + if err := ValidateEntryOptions(&EntryOptions{SkipStepPrompt: true}, MoveChangeStep); err != nil { + t.Fatalf("ValidateEntryOptions(step change) error = %v", err) + } + if err := ValidateEntryOptions(nil, MoveChangeNone); err != nil { + t.Fatalf("ValidateEntryOptions(nil) error = %v", err) + } + if err := ValidateEntryOptions(&EntryOptions{}, MoveChangePositionOnly); err != nil { + t.Fatalf("empty EntryOptions should be a no-op: %v", err) + } +} + +func TestValidateEntryOptions_ErrorDoesNotEchoUnknownChangeValue(t *testing.T) { + const sensitiveChange = "position-only-with-private-target" + err := ValidateEntryOptions(&EntryOptions{SkipStepPrompt: true}, MoveChange(sensitiveChange)) + if err == nil { + t.Fatal("ValidateEntryOptions() error = nil, want rejection") + } + if strings.Contains(err.Error(), sensitiveChange) { + t.Fatalf("validation error leaked change value: %q", err) + } +} + +func TestEntryOptionsJSONRejectsUnknownFieldsAndTrailingValues(t *testing.T) { + for _, encoded := range []string{ + `{"reset_contex":true}`, + `{"model":"gpt-5"}{"instructions":"unexpected"}`, + } { + var options EntryOptions + if err := json.Unmarshal([]byte(encoded), &options); err == nil { + t.Fatalf("json.Unmarshal(%s) error = nil, want rejection", encoded) + } + } +} diff --git a/apps/web/components/model-config-selector-content.tsx b/apps/web/components/model-config-selector-content.tsx index db0b066c97..0758f28bc1 100644 --- a/apps/web/components/model-config-selector-content.tsx +++ b/apps/web/components/model-config-selector-content.tsx @@ -44,6 +44,7 @@ function ModelRow({ keywords={[model.name, model.description ?? "", model.id]} onSelect={() => !model.disabled && onSelect(model.id)} disabled={model.disabled} + aria-label={model.name} data-testid={selected ? "model-config-selected-row" : undefined} className={selectorOptions.selectorOptionClassName(selected, model.disabled)} > diff --git a/apps/web/components/task-preview-panel-step-indicator.test.tsx b/apps/web/components/task-preview-panel-step-indicator.test.tsx index 44bcf89720..2c34d51fca 100644 --- a/apps/web/components/task-preview-panel-step-indicator.test.tsx +++ b/apps/web/components/task-preview-panel-step-indicator.test.tsx @@ -92,7 +92,7 @@ describe("TaskPreviewPanel step indicator", () => { await Promise.resolve(); }); - expect(onMoveStep).toHaveBeenCalledWith("b"); + expect(onMoveStep).toHaveBeenCalledWith("b", undefined); expect(screen.queryByTestId(DISCLOSURE_TEST_ID)).toBeNull(); }); @@ -149,7 +149,7 @@ describe("TaskPreviewPanel step indicator", () => { fireEvent.mouseEnter(screen.getByTestId(STEPPER_TEST_ID)); fireEvent.click(screen.getByTestId("workflow-step-disclosure-move-b")); - expect(onMoveStepForA).toHaveBeenCalledWith("b"); + expect(onMoveStepForA).toHaveBeenCalledWith("b", undefined); rerender( withProviders( diff --git a/apps/web/components/task-preview-panel.tsx b/apps/web/components/task-preview-panel.tsx index 742bdf5556..e12e166d80 100644 --- a/apps/web/components/task-preview-panel.tsx +++ b/apps/web/components/task-preview-panel.tsx @@ -8,7 +8,11 @@ import type { UseEnsureTaskSessionResult } from "@/hooks/domains/session/use-ens import type { Task } from "./kanban-card"; import { PreviewSessionTabs } from "./task/preview-session-tabs"; import { TaskMoveErrorBanner } from "./task/task-move-error-banner"; -import { MinimalWorkflowStepper, type WorkflowStepperStep } from "./task/workflow-step-disclosure"; +import { + MinimalWorkflowStepper, + type DisclosureMove, + type WorkflowStepperStep, +} from "./task/workflow-step-disclosure"; import { TaskActionsMenuTrigger } from "./task/task-actions-menu-trigger"; import { TaskActionsMenuDialogs } from "./task/task-actions-menu-dialogs"; import { useTaskActionsMenu, type TaskActionsMenuBoardRow } from "@/hooks/use-task-actions-menu"; @@ -27,7 +31,7 @@ interface TaskPreviewPanelProps { taskWorkflowId?: string | null; isArchived?: boolean; movingToStepId?: string | null; - onMoveStep?: (stepId: string) => Promise; + onMoveStep?: DisclosureMove; onDisclosureOpenChange?: (open: boolean) => void; moveError?: unknown; /** Lets the enclosing surface skip its own Escape-close while this menu is open. */ @@ -122,7 +126,7 @@ interface PreviewPanelHeaderProps { taskWorkflowId: string | null; isArchived: boolean; movingToStepId: string | null; - onMoveStep?: (stepId: string) => Promise; + onMoveStep?: DisclosureMove; onDisclosureOpenChange?: (open: boolean) => void; menuEntries: ReturnType["entries"]; triggerRef: ReturnType["triggerRef"]; diff --git a/apps/web/components/task/chat/chat-status-bar.tsx b/apps/web/components/task/chat/chat-status-bar.tsx index 868fd21db2..bf81f52ef1 100644 --- a/apps/web/components/task/chat/chat-status-bar.tsx +++ b/apps/web/components/task/chat/chat-status-bar.tsx @@ -10,12 +10,10 @@ * indicators, so nothing here participates in composing or sending a message. */ -import { IconArrowRight } from "@tabler/icons-react"; -import { useTranslation } from "react-i18next"; -import { Button } from "@kandev/ui/button"; -import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; import type { ReactNode } from "react"; import { useAppStore } from "@/components/state-provider"; +import { WorkflowMoveProceedButton } from "@/components/task/workflow-move-proceed-button"; +import type { WorkflowMoveEntryOptions } from "@/lib/api/domains/kanban-api"; import { PRStatusChip } from "@/components/github/pr-status-chip"; import { MRStatusChip } from "@/components/gitlab/mr-status-chip"; import { TaskDependencyChip } from "@/components/task/task-dependency-chip"; @@ -108,7 +106,7 @@ export type ChatStatusBarProps = { sessionId: string | null; sessionState: string | null; nextStepName: string | null; - onProceed: () => void; + onProceed: (options?: WorkflowMoveEntryOptions) => boolean | void | Promise; isAgentBusy: boolean; hasPendingClarification: boolean; isMoving: boolean; @@ -137,7 +135,6 @@ export function ChatStatusBar({ showScrollToStart, onScrollToStart, }: ChatStatusBarProps) { - const { t } = useTranslation(); const showTodos = todoItems.length > 0; const showProceed = shouldShowProceed(nextStepName, isAgentBusy, hasPendingClarification); const autopilot = useTaskAutopilot(taskId); @@ -202,24 +199,14 @@ export function ChatStatusBar({ /> )} - {showProceed && ( - - - - - {t("task:moveTaskToTheNextWorkflow")} - + {showProceed && nextStepName && ( + )} ); diff --git a/apps/web/components/task/chat/messages/user-message-body.tsx b/apps/web/components/task/chat/messages/user-message-body.tsx index 645d846a8c..e89b3a4308 100644 --- a/apps/web/components/task/chat/messages/user-message-body.tsx +++ b/apps/web/components/task/chat/messages/user-message-body.tsx @@ -8,7 +8,7 @@ import { Collapsible, CollapsibleContent, CollapsibleTrigger } from "@kandev/ui/ import { MemoizedMarkdown } from "@/components/shared/memoized-markdown"; import { cn } from "@/lib/utils"; import { t } from "@/lib/i18n"; -import { splitWorkflowInstructions } from "@/lib/utils/workflow-instructions"; +import { splitMessageSegments } from "@/lib/utils/workflow-instructions"; type UserMessageBodyOptions = { hasContent: boolean; @@ -48,54 +48,96 @@ function UserMessageMarkdown({ ); } -function CollapsedWorkflowInstructions({ +function CollapsedInstructions({ + label, + testId, instructions, - rest, promptMentionComponents, taskId, worktreePath, onOpenFile, }: { + label: string; + testId: string; instructions: string; - rest: string; + promptMentionComponents?: Components; + taskId: string; + worktreePath?: string; + onOpenFile?: (path: string) => void; +}) { + const [open, setOpen] = useState(false); + return ( + + + + {label} + + + + + + ); +} + +function MessageSegments({ + content, + promptMentionComponents, + taskId, + worktreePath, + onOpenFile, +}: { + content: string; promptMentionComponents?: Components; taskId: string; worktreePath?: string; onOpenFile?: (path: string) => void; }) { const { t } = useTranslation(); - const [open, setOpen] = useState(false); + const segments = splitMessageSegments(content); return (
- - - - {t("workflows:workflowInstructionsCollapsed")} - - - { + if (segment.type === "text") { + return ( + + ); + } + const isMove = segment.kind === "move"; + return ( + - - - {rest.trim() !== "" && ( - - )} + ); + })}
); } @@ -115,21 +157,8 @@ export function renderUserMessageBody({ return 
{rawContent || content}
; } if (hasContent) { - const split = splitWorkflowInstructions(content); - if (split.hasInstructions) { - return ( - - ); - } return ( - ; +}) { + const [request, setRequest] = useState(null); + const moveOptions = useTaskMoveOptions({ + taskId: request?.taskId ?? "", + workflowId: request?.workflowId, + steps: request ? [request.targetStep] : undefined, + }); + const handleRequest = useCallback( + (taskId: string, workflowId: string, targetStepId: string) => { + const targetStep = stepsByWorkflowId[workflowId]?.find((step) => step.id === targetStepId); + if (!targetStep) return; + const targetWithWorkflow = { ...targetStep, workflow_id: workflowId, task_id: taskId }; + setRequest({ taskId, workflowId, targetStep: targetWithWorkflow }); + moveOptions.openMoveOptionsStep(targetWithWorkflow); + }, + [moveOptions.openMoveOptionsStep, stepsByWorkflowId], + ); + const handleClose = useCallback(() => { + moveOptions.closeMoveOptions(); + setRequest(null); + }, [moveOptions.closeMoveOptions]); + const handleSubmit = useCallback( + async (entryOptions: Parameters[0]) => { + const moved = await moveOptions.submitMoveOptions(entryOptions); + if (moved) setRequest(null); + return moved; + }, + [moveOptions.submitMoveOptions], + ); + + useEffect(() => { + if (open || !request) return; + moveOptions.closeMoveOptions(); + setRequest(null); + }, [moveOptions.closeMoveOptions, open, request]); + + return { + request, + moveOptionsStep: moveOptions.moveOptionsStep, + isMoving: moveOptions.isMoving, + handleRequest, + handleClose, + handleSubmit, + }; +} + +export function MobileTaskMoveOptionsSurface({ + presentation, + step, + isMoving, + onBack, + onSubmit, +}: { + presentation: "sheet" | "drawer"; + step: TaskMoveStep; + isMoving: boolean; + onBack: () => void; + onSubmit: WorkflowMoveOptionsSubmit; +}) { + const { t } = useTranslation(); + const header = ( +
+ +
+ {presentation === "drawer" ? ( + + {t("task:workflowMoveOptionsTitle", { step: step.title })} + + ) : ( + + {t("task:workflowMoveOptionsTitle", { step: step.title })} + + )} +

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

+
+
+ ); + return ( + <> + {presentation === "drawer" ? ( + + {header} + + ) : ( + + {header} + + )} +
+ +
+ + ); +} diff --git a/apps/web/components/task/mobile/session-task-switcher-sheet-dialogs.tsx b/apps/web/components/task/mobile/session-task-switcher-sheet-dialogs.tsx new file mode 100644 index 0000000000..e62f828b93 --- /dev/null +++ b/apps/web/components/task/mobile/session-task-switcher-sheet-dialogs.tsx @@ -0,0 +1,121 @@ +import { NewSubtaskDialog } from "../new-subtask-dialog"; +import { TaskArchiveConfirmDialog } from "../task-archive-confirm-dialog"; +import { TaskDeleteConfirmDialog } from "../task-delete-confirm-dialog"; +import { TaskDetachTargetConfirmDialog } from "../task-detach-confirm-dialog"; +import { TaskRenameDialog } from "../task-rename-dialog"; +import { SidebarLinkDialogs } from "../task-session-sidebar-dialogs"; +import { TaskCreateDialog } from "@/components/task-create-dialog"; +import { SidebarTaskEditDialog, useSidebarTaskEdit } from "../task-session-sidebar-edit"; +import type { useSheetData, useSheetActions } from "./session-task-switcher-sheet-hooks"; +import type { useMobileTaskRename } from "./use-mobile-task-rename"; +import type { useMobileTaskLinking } from "./session-task-switcher-sheet"; + +export function MobileSubtaskDialog({ + target, + onTargetChange, +}: { + target: { id: string; title: string } | null; + onTargetChange: (next: { id: string; title: string } | null) => void; +}) { + return ( + { + if (!open) onTargetChange(null); + }} + parentTaskId={target?.id ?? ""} + parentTaskTitle={target?.title ?? ""} + /> + ); +} + +export function TaskSwitcherDialogs({ + dialogOpen, + onDialogOpenChange, + workspaceId, + workflowId, + data, + actions, + rename, + edit, + linking, + subtaskTarget, + onSubtaskTargetChange, +}: { + dialogOpen: boolean; + onDialogOpenChange: (open: boolean) => void; + workspaceId: string | null; + workflowId: string | null; + data: ReturnType; + actions: ReturnType; + rename: ReturnType; + edit: ReturnType; + linking: ReturnType; + subtaskTarget: { id: string; title: string } | null; + onSubtaskTargetChange: (target: { id: string; title: string } | null) => void; +}) { + return ( + <> + + + + { + if (!open) actions.setArchivingTask(null); + }} + taskTitle={actions.archivingTask?.title ?? ""} + taskId={actions.archivingTask?.id} + executorType={actions.archivingTask?.executorType} + isArchiving={actions.isArchiving} + onConfirm={({ cascade }) => actions.handleArchiveConfirm({ cascade })} + /> + { + if (!open) rename.setRenamingTask(null); + }} + currentTitle={rename.renamingTask?.title ?? ""} + onSubmit={rename.handleRenameSubmit} + /> + { + if (!open) actions.setDeletingTask(null); + }} + taskTitle={actions.deletingTask?.title ?? ""} + taskId={actions.deletingTask?.id} + executorType={actions.deletingTask?.executorType} + isDeleting={actions.isDeleting} + onConfirm={({ cascade, discardWorktreeChanges }) => + actions.handleDeleteConfirm({ cascade, discardWorktreeChanges }) + } + /> + actions.setDetachingTask(null)} + onConfirm={actions.handleDetachConfirm} + /> + + + ); +} diff --git a/apps/web/components/task/mobile/session-task-switcher-sheet-props.ts b/apps/web/components/task/mobile/session-task-switcher-sheet-props.ts index 222a967e03..961679b36a 100644 --- a/apps/web/components/task/mobile/session-task-switcher-sheet-props.ts +++ b/apps/web/components/task/mobile/session-task-switcher-sheet-props.ts @@ -24,6 +24,8 @@ export function buildMobileTaskSwitcherProps( stepsByWorkflowId: props.stepsByWorkflowId, activeTaskId: props.activeTaskId, selectedTaskId: props.selectedTaskId, + onRequestMoveOptions: props.onRequestMoveOptions, + onBeforeMoveOptionsOpen: props.onBeforeMoveOptionsOpen, collapsedGroupKeys: helpers.collapsedGroupKeys, showActivityTime: helpers.showActivityTime, taskRowPresentation: helpers.taskRowPresentation, diff --git a/apps/web/components/task/mobile/session-task-switcher-sheet.test.tsx b/apps/web/components/task/mobile/session-task-switcher-sheet.test.tsx index d892b9daa5..077a60f2b0 100644 --- a/apps/web/components/task/mobile/session-task-switcher-sheet.test.tsx +++ b/apps/web/components/task/mobile/session-task-switcher-sheet.test.tsx @@ -6,6 +6,7 @@ import { MobileTaskList, useTaskSheetSelectionController, } from "@/components/task/mobile/session-task-switcher-sheet"; +import type { MobileTaskListProps } from "@/components/task/mobile/session-task-switcher-sheet"; import { selectTaskFromSheet } from "@/components/task/mobile/session-task-switcher-sheet-selection"; import type { TaskSwitcherItem } from "@/components/task/task-switcher"; import type { TaskSession } from "@/lib/types/http"; @@ -169,6 +170,53 @@ describe("MobileTaskList", () => { }); }); +describe("MobileTaskList move options", () => { + it("hands move options to the owning mobile task surface", async () => { + const onRequestMoveOptions = vi.fn(); + const props: MobileTaskListProps = { + tasks: [task("movable")], + workflows: [{ id: "workflow-1", name: "Workflow" }], + stepsByWorkflowId: { + "workflow-1": [ + { id: "step-1", title: "Build" }, + { id: "step-2", title: "Review" }, + ], + }, + activeTaskId: null, + selectedTaskId: null, + onSelectTask: vi.fn(), + onArchiveTask: vi.fn(), + onDeleteTask: vi.fn(), + onDetachTask: vi.fn(), + deletingTaskId: null, + onRequestMoveOptions, + }; + + render( + + + , + ); + + const row = screen + .getByText("Task movable") + .closest("[data-testid='sidebar-task-item']"); + expect(row).not.toBeNull(); + fireEvent.click(within(row!).getByRole("button", { name: "Task actions" })); + const moveTo = await screen.findByTestId("task-context-move-to"); + fireEvent.pointerMove(moveTo, { pointerType: "mouse" }); + const step = await screen.findByTestId("task-context-step-step-2"); + fireEvent.pointerMove(step, { + pointerType: "mouse", + }); + fireEvent.click(await screen.findByTestId("task-context-step-options-step-2")); + + expect(onRequestMoveOptions).toHaveBeenCalledOnce(); + expect(onRequestMoveOptions).toHaveBeenCalledWith("movable", "workflow-1", "step-2"); + expect(screen.queryByTestId("workflow-move-options")).toBeNull(); + }); +}); + describe("SessionTaskSwitcherSheet lifecycle", () => { it("invalidates a deferred selection when responsive remount replaces the sheet", async () => { const phoneTaskId = "task-phone"; diff --git a/apps/web/components/task/mobile/session-task-switcher-sheet.tsx b/apps/web/components/task/mobile/session-task-switcher-sheet.tsx index da450e3eb7..b22dc13435 100644 --- a/apps/web/components/task/mobile/session-task-switcher-sheet.tsx +++ b/apps/web/components/task/mobile/session-task-switcher-sheet.tsx @@ -9,22 +9,16 @@ import { Button } from "@kandev/ui/button"; import { QuickChatSheetButton } from "./quick-chat-sheet-button"; import { TaskSwitcher } from "../task-switcher"; import type { TaskSwitcherItem } from "../task-switcher"; +import type { TaskMoveWorkflow } from "../task-move-context-menu"; +import { MobileTaskMoveOptionsSurface, useMobileTaskMoveOptions } from "./mobile-task-move-options"; import { SidebarFilterBar } from "../sidebar-filter/sidebar-filter-bar"; import type { StepDef } from "../task-switcher-context-menu"; -import type { TaskMoveWorkflow } from "../task-move-context-menu"; import { applyView } from "@/lib/sidebar/apply-view"; import { useAppStore, useAppStoreApi } from "@/components/state-provider"; import { useEffectiveSidebarView } from "@/hooks/domains/sidebar/use-effective-sidebar-view"; import { useSidebarTaskPrefs } from "@/hooks/domains/sidebar/use-sidebar-task-prefs"; import { useRepositories } from "@/hooks/domains/workspace/use-repositories"; import { WorkspaceSwitcher } from "../workspace-switcher"; -import { TaskCreateDialog } from "@/components/task-create-dialog"; -import { NewSubtaskDialog } from "../new-subtask-dialog"; -import { TaskArchiveConfirmDialog } from "../task-archive-confirm-dialog"; -import { TaskDeleteConfirmDialog } from "../task-delete-confirm-dialog"; -import { TaskDetachTargetConfirmDialog } from "../task-detach-confirm-dialog"; -import { TaskRenameDialog } from "../task-rename-dialog"; -import { SidebarLinkDialogs } from "../task-session-sidebar-dialogs"; import { PluginTaskLinkActionSurfaceProvider, useSidebarLinkActions, @@ -37,9 +31,10 @@ import { } from "./session-task-switcher-sheet-selection"; import { useQuickChatLauncher } from "@/hooks/use-quick-chat-launcher"; import { useMobileTaskRename } from "./use-mobile-task-rename"; -import { SidebarTaskEditDialog, useSidebarTaskEdit } from "../task-session-sidebar-edit"; +import { useSidebarTaskEdit } from "../task-session-sidebar-edit"; import { usePortForwardingVisibility } from "../port-forwarding-visibility-provider"; import { buildMobileTaskSwitcherProps } from "./session-task-switcher-sheet-props"; +import { TaskSwitcherDialogs } from "./session-task-switcher-sheet-dialogs"; type SessionTaskSwitcherSheetProps = { open: boolean; onOpenChange: (open: boolean) => void; @@ -57,7 +52,7 @@ export function useTaskSheetSelectionController() { ); return selectionController; } -function useMobileTaskLinking(workspaceId: string | null) { +export function useMobileTaskLinking(workspaceId: string | null) { const store = useAppStoreApi(); const actions = useSidebarLinkActions(store); const taskListHandlers = useSidebarTaskLinking(workspaceId, actions); @@ -85,6 +80,8 @@ export type MobileTaskListProps = { activeTaskId: string | null; selectedTaskId: string | null; onSelectTask: (taskId: string) => void; + onRequestMoveOptions?: (taskId: string, workflowId: string, targetStepId: string) => void; + onBeforeMoveOptionsOpen?: () => void; onEditTask?: (task: TaskSwitcherItem) => void; onRenameTask?: (taskId: string, currentTitle: string) => void; onCreateSubtask?: (taskId: string, taskTitle: string) => void; @@ -229,7 +226,67 @@ function surfaceAction( }; } +function buildMobileTaskListSurfaceActions({ + presentation, + onOpenChange, + actions, + rename, + edit, + linking, +}: Pick< + TaskSwitcherSurfaceContentProps, + "presentation" | "onOpenChange" | "actions" | "rename" | "edit" | "linking" +>): Pick< + MobileTaskListProps, + | "onEditTask" + | "onRenameTask" + | "onArchiveTask" + | "onDeleteTask" + | "onDetachTask" + | "onLinkPullRequest" + | "onLinkIssue" + | "onLinkMergeRequest" + | "onLinkJiraTicket" + | "onLinkLinearIssue" + | "onLinkSentryIssue" +> { + return { + onEditTask: surfaceAction(presentation, onOpenChange, edit.handleEditTask), + onRenameTask: surfaceAction(presentation, onOpenChange, rename.handleRenameTask), + onArchiveTask: surfaceAction(presentation, onOpenChange, actions.handleArchiveTask), + onDeleteTask: surfaceAction(presentation, onOpenChange, actions.handleDeleteTask), + onDetachTask: surfaceAction(presentation, onOpenChange, actions.handleDetachTask), + onLinkPullRequest: surfaceAction( + presentation, + onOpenChange, + linking.taskListHandlers.onLinkPullRequest, + ), + onLinkIssue: surfaceAction(presentation, onOpenChange, linking.taskListHandlers.onLinkIssue), + onLinkMergeRequest: surfaceAction( + presentation, + onOpenChange, + linking.taskListHandlers.onLinkMergeRequest, + ), + onLinkJiraTicket: surfaceAction( + presentation, + onOpenChange, + linking.taskListHandlers.onLinkJiraTicket, + ), + onLinkLinearIssue: surfaceAction( + presentation, + onOpenChange, + linking.taskListHandlers.onLinkLinearIssue, + ), + onLinkSentryIssue: surfaceAction( + presentation, + onOpenChange, + linking.taskListHandlers.onLinkSentryIssue, + ), + }; +} + type TaskSwitcherSurfaceContentProps = { + open: boolean; presentation: "sheet" | "drawer"; workspaceId: string | null; onOpenChange: (open: boolean) => void; @@ -243,26 +300,8 @@ type TaskSwitcherSurfaceContentProps = { linking: ReturnType; }; -function MobileSubtaskDialog({ - target, - onTargetChange, -}: { - target: { id: string; title: string } | null; - onTargetChange: (next: { id: string; title: string } | null) => void; -}) { - return ( - { - if (!open) onTargetChange(null); - }} - parentTaskId={target?.id ?? ""} - parentTaskTitle={target?.title ?? ""} - /> - ); -} - function TaskSwitcherSurfaceContent({ + open, presentation, workspaceId, onOpenChange, @@ -276,6 +315,29 @@ function TaskSwitcherSurfaceContent({ linking, }: TaskSwitcherSurfaceContentProps) { const { t } = useTranslation(); + const moveOptions = useMobileTaskMoveOptions({ + open, + stepsByWorkflowId: data.stepsByWorkflowId, + }); + if (moveOptions.moveOptionsStep && moveOptions.request) { + return ( + + ); + } + const taskListActions = buildMobileTaskListSurfaceActions({ + presentation, + onOpenChange, + actions, + rename, + edit, + linking, + }); return ( <> onOpenChange(false) : undefined + } + {...taskListActions} onCreateSubtask={onCreateSubtask} - onArchiveTask={surfaceAction(presentation, onOpenChange, actions.handleArchiveTask)} - onDeleteTask={surfaceAction(presentation, onOpenChange, actions.handleDeleteTask)} - onDetachTask={surfaceAction(presentation, onOpenChange, actions.handleDetachTask)} onNestTask={actions.handleNestTask} - onLinkPullRequest={surfaceAction( - presentation, - onOpenChange, - linking.taskListHandlers.onLinkPullRequest, - )} - onLinkIssue={surfaceAction( - presentation, - onOpenChange, - linking.taskListHandlers.onLinkIssue, - )} - onLinkMergeRequest={surfaceAction( - presentation, - onOpenChange, - linking.taskListHandlers.onLinkMergeRequest, - )} - onLinkJiraTicket={surfaceAction( - presentation, - onOpenChange, - linking.taskListHandlers.onLinkJiraTicket, - )} - onLinkLinearIssue={surfaceAction( - presentation, - onOpenChange, - linking.taskListHandlers.onLinkLinearIssue, - )} - onLinkSentryIssue={surfaceAction( - presentation, - onOpenChange, - linking.taskListHandlers.onLinkSentryIssue, - )} deletingTaskId={actions.deletingTaskId} archivingTaskId={actions.archivingTaskId} isArchiving={actions.isArchiving} @@ -388,97 +420,6 @@ function PortForwardingTaskAction({ ); } -function TaskSwitcherDialogs({ - dialogOpen, - onDialogOpenChange, - workspaceId, - workflowId, - data, - actions, - rename, - edit, - linking, - subtaskTarget, - onSubtaskTargetChange, -}: { - dialogOpen: boolean; - onDialogOpenChange: (open: boolean) => void; - workspaceId: string | null; - workflowId: string | null; - data: ReturnType; - actions: ReturnType; - rename: ReturnType; - edit: ReturnType; - linking: ReturnType; - subtaskTarget: { id: string; title: string } | null; - onSubtaskTargetChange: (target: { id: string; title: string } | null) => void; -}) { - return ( - <> - - - - { - if (!open) actions.setArchivingTask(null); - }} - taskTitle={actions.archivingTask?.title ?? ""} - taskId={actions.archivingTask?.id} - executorType={actions.archivingTask?.executorType} - isArchiving={actions.isArchiving} - onConfirm={({ cascade }) => actions.handleArchiveConfirm({ cascade })} - /> - { - if (!open) rename.setRenamingTask(null); - }} - currentTitle={rename.renamingTask?.title ?? ""} - onSubmit={rename.handleRenameSubmit} - /> - { - if (!open) actions.setDeletingTask(null); - }} - taskTitle={actions.deletingTask?.title ?? ""} - taskId={actions.deletingTask?.id} - executorType={actions.deletingTask?.executorType} - isDeleting={actions.isDeleting} - onConfirm={({ cascade, discardWorktreeChanges }) => - actions.handleDeleteConfirm({ cascade, discardWorktreeChanges }) - } - /> - actions.setDetachingTask(null)} - onConfirm={actions.handleDetachConfirm} - /> - - - ); -} - export const SessionTaskSwitcherSheet = memo(function SessionTaskSwitcherSheet({ open, onOpenChange, @@ -513,6 +454,7 @@ export const SessionTaskSwitcherSheet = memo(function SessionTaskSwitcherSheet({ const surfaceContent = ( ({ kanban: { workflowId: null, tasks: [] }, kanbanMulti: { snapshots: {} }, workflows: { items: [] }, + availableAgents: { items: [], loaded: true, loading: false }, + setAvailableAgents: vi.fn(), + setAvailableAgentsLoading: vi.fn(), userSettings: { keyboardShortcuts: mockKeyboardShortcuts, chatSubmitKey: "enter" }, }), useAppStoreApi: () => ({ diff --git a/apps/web/components/task/passthrough-toolbar.tsx b/apps/web/components/task/passthrough-toolbar.tsx index 234a8c8a49..bba995b03e 100644 --- a/apps/web/components/task/passthrough-toolbar.tsx +++ b/apps/web/components/task/passthrough-toolbar.tsx @@ -10,7 +10,6 @@ import { type SetStateAction, } from "react"; import { - IconArrowRight, IconMessageCircle, IconMessageDots, IconSend, @@ -43,6 +42,8 @@ import { PassthroughTerminal } from "./passthrough-terminal"; import { PassthroughComposerPanel, useSendPassthroughMessage } from "./passthrough-chat-composer"; import { hasPendingClarification, shouldShowProceed } from "./chat/types"; import { Trans, useTranslation } from "react-i18next"; +import { WorkflowMoveProceedButton } from "@/components/task/workflow-move-proceed-button"; +import type { WorkflowMoveEntryOptions } from "@/lib/api/domains/kanban-api"; function isEditableElement(element: Element | null) { if (element instanceof HTMLElement && element.closest(".xterm")) return false; @@ -528,7 +529,7 @@ type StatusRowProps = { taskId: string | null; sessionId?: string | null; nextStepName: string | null; - onProceed: () => void; + onProceed: (options?: WorkflowMoveEntryOptions) => boolean | void | Promise; isMoving: boolean; showProceed: boolean; composerOpen: boolean; @@ -555,7 +556,6 @@ function PassthroughStatusRow({ pendingCommentsCount, isTouch, }: StatusRowProps) { - const { t } = useTranslation(); return (
{taskId && } {showProceed && nextStepName && ( - - - - - {t("task:moveTaskToTheNextWorkflow")} - + )}
diff --git a/apps/web/components/task/task-move-context-menu.test.tsx b/apps/web/components/task/task-move-context-menu.test.tsx new file mode 100644 index 0000000000..2ca348f1b4 --- /dev/null +++ b/apps/web/components/task/task-move-context-menu.test.tsx @@ -0,0 +1,174 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { ContextMenu, ContextMenuContent, ContextMenuTrigger } from "@kandev/ui/context-menu"; +import { TaskMoveContextMenuItems } from "./task-move-context-menu"; + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +const steps = [ + { id: "step-1", title: "Todo", color: "bg-slate-500" }, + { + id: "step-2", + title: "Review", + color: "bg-blue-500", + events: { on_enter: [{ type: "auto_start_agent" }] }, + }, +]; + +const MOVE_TO_TEST_ID = "task-context-move-to"; +const STEP_2_TEST_ID = "task-context-step-step-2"; +const STEP_2_OPTIONS_TEST_ID = "task-context-step-options-step-2"; + +function renderMoveMenu({ + onMoveToStep = vi.fn(), + onMoveToStepWithOptions, +}: { + onMoveToStep?: (stepId: string) => void; + onMoveToStepWithOptions?: (stepId: string) => void; +} = {}) { + const menuProps: Parameters[0] = { + currentWorkflowId: "workflow-1", + currentStepId: "step-1", + workflows: [{ id: "workflow-1", name: "Workflow 1" }], + stepsByWorkflowId: { "workflow-1": steps }, + showSeparator: false, + onMoveToStep, + onMoveToStepWithOptions, + }; + render( + + Task + + + + , + ); + + fireEvent.contextMenu(screen.getByTestId("context-trigger")); + return { onMoveToStep, onMoveToStepWithOptions }; +} + +async function openSubmenu(testId: string, childTestId: string) { + fireEvent.pointerMove(screen.getByTestId(testId), { pointerType: "mouse" }); + return screen.findByTestId(childTestId); +} + +describe("TaskMoveContextMenuItems", () => { + it("keeps direct moves on the step row while nesting options under that row", async () => { + const direct = vi.fn(); + const options = vi.fn(); + renderMoveMenu({ onMoveToStep: direct, onMoveToStepWithOptions: options }); + + expect(screen.getByTestId(MOVE_TO_TEST_ID)).toBeTruthy(); + expect(screen.queryByTestId("task-context-move-with-options")).toBeNull(); + + const directStep = await openSubmenu(MOVE_TO_TEST_ID, STEP_2_TEST_ID); + expect(directStep.className).toContain("[@media(pointer:coarse)]:min-h-11"); + fireEvent.pointerDown(directStep, { pointerType: "mouse" }); + fireEvent.click(directStep); + + expect(direct).toHaveBeenCalledOnce(); + expect(direct).toHaveBeenCalledWith("step-2"); + expect(options).not.toHaveBeenCalled(); + }); + + it("exposes one localized options action under the hovered step", async () => { + const direct = vi.fn(); + const options = vi.fn(); + renderMoveMenu({ onMoveToStep: direct, onMoveToStepWithOptions: options }); + + const step = await openSubmenu(MOVE_TO_TEST_ID, STEP_2_TEST_ID); + fireEvent.pointerMove(step, { pointerType: "mouse" }); + + const optionsStep = await screen.findByTestId(STEP_2_OPTIONS_TEST_ID); + expect(optionsStep.textContent).toMatch(/Move with options/i); + expect(optionsStep.className).toContain("[@media(pointer:coarse)]:min-h-11"); + expect(screen.queryByTestId("task-context-move-with-options")).toBeNull(); + expect(direct).not.toHaveBeenCalled(); + expect(options).not.toHaveBeenCalled(); + }); + + it("selects the nested options action without invoking the direct move", async () => { + const direct = vi.fn(); + const options = vi.fn(); + renderMoveMenu({ onMoveToStep: direct, onMoveToStepWithOptions: options }); + + const step = await openSubmenu(MOVE_TO_TEST_ID, STEP_2_TEST_ID); + fireEvent.pointerMove(step, { pointerType: "mouse" }); + const optionsStep = await screen.findByTestId(STEP_2_OPTIONS_TEST_ID); + fireEvent.click(optionsStep); + + expect(options).toHaveBeenCalledOnce(); + expect(options).toHaveBeenCalledWith("step-2"); + expect(direct).not.toHaveBeenCalled(); + }); + + it("keeps a touch tap on the step label as a direct move", async () => { + const direct = vi.fn(); + renderMoveMenu({ onMoveToStep: direct, onMoveToStepWithOptions: vi.fn() }); + + const step = await openSubmenu(MOVE_TO_TEST_ID, STEP_2_TEST_ID); + fireEvent.pointerDown(step, { pointerType: "touch", clientX: 0 }); + fireEvent.click(step, { clientX: 0 }); + + expect(direct).toHaveBeenCalledOnce(); + expect(direct).toHaveBeenCalledWith("step-2"); + expect(screen.queryByTestId(STEP_2_OPTIONS_TEST_ID)).toBeNull(); + }); + + it("opens the nested options submenu from the step chevron on touch", async () => { + renderMoveMenu({ onMoveToStepWithOptions: vi.fn() }); + + const step = await openSubmenu(MOVE_TO_TEST_ID, STEP_2_TEST_ID); + vi.spyOn(step, "getBoundingClientRect").mockReturnValue({ + x: 0, + y: 0, + width: 100, + height: 44, + top: 0, + right: 100, + bottom: 44, + left: 0, + toJSON: () => ({}), + }); + fireEvent.pointerDown(step, { pointerType: "touch", clientX: 96 }); + fireEvent.click(step, { clientX: 96 }); + + expect(await screen.findByTestId(STEP_2_OPTIONS_TEST_ID)).toBeTruthy(); + }); + + it("keeps Radix keyboard navigation for the nested options submenu", async () => { + renderMoveMenu({ onMoveToStepWithOptions: vi.fn() }); + + const trigger = await openSubmenu(MOVE_TO_TEST_ID, STEP_2_TEST_ID); + trigger.focus(); + fireEvent.keyDown(trigger, { key: "ArrowRight" }); + + expect(await screen.findByTestId(STEP_2_OPTIONS_TEST_ID)).toBeTruthy(); + }); + + it("keeps the current step disabled when options are available", async () => { + renderMoveMenu({ onMoveToStepWithOptions: vi.fn() }); + + const currentStep = await openSubmenu(MOVE_TO_TEST_ID, "task-context-step-step-1"); + + expect(currentStep.getAttribute("aria-disabled")).toBe("true"); + }); + + it("keeps plain direct step items when no options callback is supplied", async () => { + const direct = vi.fn(); + renderMoveMenu({ onMoveToStep: direct }); + + const directStep = await openSubmenu(MOVE_TO_TEST_ID, STEP_2_TEST_ID); + fireEvent.pointerMove(directStep, { pointerType: "mouse" }); + expect( + directStep.querySelector('[data-testid="task-context-step-autostart-step-2"]'), + ).toBeTruthy(); + expect(screen.queryByTestId(STEP_2_OPTIONS_TEST_ID)).toBeNull(); + fireEvent.click(directStep); + expect(direct).toHaveBeenCalledWith("step-2"); + }); +}); diff --git a/apps/web/components/task/task-move-context-menu.tsx b/apps/web/components/task/task-move-context-menu.tsx index 4280acb1dd..2ed1ff231f 100644 --- a/apps/web/components/task/task-move-context-menu.tsx +++ b/apps/web/components/task/task-move-context-menu.tsx @@ -1,6 +1,7 @@ "use client"; -import { IconArrowRight, IconLogicBuffer } from "@tabler/icons-react"; +import { useRef, useState } from "react"; +import { IconAdjustments, IconArrowRight, IconLogicBuffer } from "@tabler/icons-react"; import { ContextMenuItem, ContextMenuSeparator, @@ -10,11 +11,24 @@ import { import { TaskContextMenuSubContent as ContextMenuSubContent } from "./task-context-menu-sub-content"; import { cn } from "@/lib/utils"; import { useTranslation } from "react-i18next"; +import { useWorkflowMove } from "@/hooks/domains/kanban/use-workflow-move"; +import { useToast } from "@/components/toast-provider"; +import { + WorkflowMoveDialog, + WorkflowMoveOptions, + WorkflowMoveOptionsForm, + type WorkflowMoveOptionsSubmit, +} from "./workflow-move-options"; +import { useTouchDrawer } from "@/hooks/use-compact-task-chrome"; +import type { WorkflowMoveEntryOptions } from "@/lib/api/domains/kanban-api"; export type TaskMoveStep = { id: string; title: string; + task_id?: string | null; color?: string | null; + workflow_id?: string | null; + agent_profile_id?: string | null; events?: { on_enter?: Array<{ type: string; config?: Record }> }; }; @@ -24,6 +38,109 @@ export type TaskMoveWorkflow = { hidden?: boolean; }; +export function useTaskMoveOptions({ + taskId, + workflowId, + steps, + closeMenu, +}: { + taskId: string; + workflowId?: string | null; + steps?: TaskMoveStep[]; + closeMenu?: () => void; +}) { + const [moveOptionsStep, setMoveOptionsStep] = useState(null); + const { move, isMoving } = useWorkflowMove(); + const { toast } = useToast(); + const { t } = useTranslation(); + + const openMoveOptionsStep = (targetStep: TaskMoveStep) => { + setMoveOptionsStep({ ...targetStep, task_id: targetStep.task_id ?? taskId }); + }; + + const openMoveOptions = (targetStepId: string) => { + const targetStep = steps?.find((step) => step.id === targetStepId); + if (!targetStep) return; + openMoveOptionsStep({ ...targetStep, workflow_id: workflowId ?? undefined }); + }; + + const runMove = async ( + targetStepId: string, + entryOptions: WorkflowMoveEntryOptions | undefined, + ) => { + if (!workflowId) return false; + const result = await move(taskId, { + workflow_id: workflowId, + workflow_step_id: targetStepId, + position: 0, + entry_options: entryOptions, + }); + if (result.disposition === "failed") { + const error = result.error; + toast({ + title: t("task:failedToMoveTask"), + description: error instanceof Error ? error.message : t("task:failedToMoveTask"), + variant: "error", + }); + return false; + } + closeMenu?.(); + return true; + }; + + const submitMoveOptions = async (entryOptions: WorkflowMoveEntryOptions | undefined) => { + if (!moveOptionsStep) return false; + const ok = await runMove(moveOptionsStep.id, entryOptions); + if (ok) setMoveOptionsStep(null); + return ok; + }; + + // Move a specific step directly, used by the fine-pointer inline options form + // rendered in the "Move to" submenu (no intermediate dialog/drawer surface). + const submitMoveOptionsForStep = ( + targetStepId: string, + entryOptions: WorkflowMoveEntryOptions | undefined, + ) => runMove(targetStepId, entryOptions); + + return { + moveOptionsStep, + isMoving, + openMoveOptions, + openMoveOptionsStep, + submitMoveOptions, + submitMoveOptionsForStep, + closeMoveOptions: () => { + setMoveOptionsStep(null); + }, + }; +} + +export function TaskMoveOptionsSurface({ + step, + isMoving, + onClose, + onSubmit, +}: { + step: TaskMoveStep | null; + isMoving: boolean; + onClose: () => void; + onSubmit: WorkflowMoveOptionsSubmit; +}) { + const usesTouchDrawer = useTouchDrawer(); + if (!step) return null; + const optionsProps = { + open: true, + onOpenChange: (open: boolean) => { + if (!open) onClose(); + }, + targetStepName: step.title, + isMoving, + onSubmit, + }; + const Options = usesTouchDrawer ? WorkflowMoveOptions : WorkflowMoveDialog; + return ; +} + type TaskMoveContextMenuItemsProps = { currentWorkflowId?: string | null; currentStepId?: string | null; @@ -32,6 +149,12 @@ type TaskMoveContextMenuItemsProps = { disabled?: boolean; showSeparator?: boolean; onMoveToStep?: (stepId: string) => void; + onMoveToStepWithOptions?: (stepId: string) => void; + onSubmitWithOptions?: ( + stepId: string, + entryOptions: WorkflowMoveEntryOptions | undefined, + ) => Promise; + isMoving?: boolean; onSendToWorkflow?: (workflowId: string, stepId: string) => void; }; @@ -51,43 +174,139 @@ export function taskMoveOptions( return { currentSteps, targets, canMove: currentSteps.length > 1, canSend: targets.length > 0 }; } -function StepMenuItem({ +function StepMenuLabel({ step, - currentStepId, - onSelect, + isCurrent, + testIdPrefix, }: { step: TaskMoveStep; - currentStepId?: string | null; - onSelect: (stepId: string) => void; + isCurrent: boolean; + testIdPrefix: string; }) { const { t } = useTranslation(); - const isCurrent = step.id === currentStepId; const hasAutoStart = stepHasAutoStart(step); return ( - { - event.preventDefault(); - if (!isCurrent) onSelect(step.id); - }} - > + <> {step.title} {(isCurrent || hasAutoStart) && ( {isCurrent && ( - {t("task:current2")} + {t("task:current2")} )} {hasAutoStart && ( - - {t("task:autoStart")} - + {t("task:autoStart")} )} )} - + + ); +} + +function StepMenuItem({ + step, + currentStepId, + onSelect, + onSelectWithOptions, + onSubmitWithOptions, + isMoving, + testIdPrefix = "task-context-step", +}: { + step: TaskMoveStep; + currentStepId?: string | null; + onSelect: (stepId: string) => void; + onSelectWithOptions?: (stepId: string) => void; + onSubmitWithOptions?: ( + stepId: string, + entryOptions: WorkflowMoveEntryOptions | undefined, + ) => Promise; + isMoving?: boolean; + testIdPrefix?: string; +}) { + const { t } = useTranslation(); + const pointerTypeRef = useRef(null); + const isCurrent = step.id === currentStepId; + const content = ; + + if (!onSelectWithOptions && !onSubmitWithOptions) { + return ( + { + event.preventDefault(); + if (!isCurrent) onSelect(step.id); + }} + > + {content} + + ); + } + + return ( + + { + pointerTypeRef.current = event.pointerType; + }} + onPointerCancel={() => { + pointerTypeRef.current = null; + }} + onClick={(event) => { + const pointerType = pointerTypeRef.current; + pointerTypeRef.current = null; + const rect = event.currentTarget.getBoundingClientRect(); + const tappedChevron = + (pointerType === "touch" || pointerType === "pen") && + rect.width > 0 && + event.clientX >= rect.right - 32; + if (!tappedChevron) { + event.preventDefault(); + if (!isCurrent) onSelect(step.id); + } + }} + onKeyDown={(event) => { + if (!isCurrent && (event.key === "Enter" || event.key === " ")) { + event.preventDefault(); + onSelect(step.id); + } + }} + > + {content} + + + {onSubmitWithOptions ? ( + // Interactive fields live inside a Radix menu popup. Stop key events at + // this wrapper (bubble phase, after the field handled them) so the + // menu's typeahead never yanks focus off the instructions field or the + // agent-profile combobox search while the user is typing. +
event.stopPropagation()}> + onSubmitWithOptions(step.id, entryOptions)} + /> +
+ ) : ( + { + event.preventDefault(); + onSelectWithOptions?.(step.id); + }} + > + + {t("task:moveWithOptions")} + + )} + + ); } @@ -96,17 +315,30 @@ function MoveToCurrentWorkflowSubmenu({ currentStepId, disabled, onMoveToStep, + onMoveToStepWithOptions, + onSubmitWithOptions, + isMoving, }: { steps: TaskMoveStep[]; currentStepId?: string | null; disabled?: boolean; onMoveToStep?: (stepId: string) => void; + onMoveToStepWithOptions?: (stepId: string) => void; + onSubmitWithOptions?: ( + stepId: string, + entryOptions: WorkflowMoveEntryOptions | undefined, + ) => Promise; + isMoving?: boolean; }) { const { t } = useTranslation(); if (!onMoveToStep || steps.length <= 1) return null; return ( - + {t("task:moveTo")} @@ -117,6 +349,9 @@ function MoveToCurrentWorkflowSubmenu({ step={step} currentStepId={currentStepId} onSelect={onMoveToStep} + onSelectWithOptions={onMoveToStepWithOptions} + onSubmitWithOptions={onSubmitWithOptions} + isMoving={isMoving} /> ))} @@ -154,6 +389,7 @@ function WorkflowTargetItem({ return ( @@ -187,7 +423,11 @@ function SendToWorkflowSubmenu({ if (!onSendToWorkflow || workflows.length === 0) return null; return ( - + {t("task:sendToWorkflow")} @@ -214,6 +454,9 @@ export function TaskMoveContextMenuItems({ disabled, showSeparator = true, onMoveToStep, + onMoveToStepWithOptions, + onSubmitWithOptions, + isMoving, onSendToWorkflow, }: TaskMoveContextMenuItemsProps) { const { currentSteps, targets, canMove, canSend } = taskMoveOptions( @@ -221,7 +464,9 @@ export function TaskMoveContextMenuItems({ workflows, stepsByWorkflowId, ); - const hasSameWorkflowMove = Boolean(onMoveToStep && canMove); + const hasSameWorkflowMove = Boolean( + (onMoveToStep || onMoveToStepWithOptions || onSubmitWithOptions) && canMove, + ); const hasCrossWorkflowMove = Boolean(onSendToWorkflow && canSend); if (!hasSameWorkflowMove && !hasCrossWorkflowMove) return null; @@ -234,6 +479,9 @@ export function TaskMoveContextMenuItems({ currentStepId={currentStepId} disabled={disabled} onMoveToStep={onMoveToStep} + onMoveToStepWithOptions={onMoveToStepWithOptions} + onSubmitWithOptions={onSubmitWithOptions} + isMoving={isMoving} /> ({ + move: vi.fn(), + isMoving: false, +})); +const touchDrawerMock = vi.hoisted(() => ({ enabled: false })); + +vi.mock("@/hooks/domains/kanban/use-workflow-move", () => ({ + useWorkflowMove: () => workflowMoveMock, +})); +vi.mock("@/hooks/use-compact-task-chrome", () => ({ + useTouchDrawer: () => touchDrawerMock.enabled, +})); afterEach(() => { cleanup(); pluginRegistry.unregisterPlugin(PLUGIN_ID); }); +beforeEach(() => { + workflowMoveMock.move.mockReset(); + workflowMoveMock.move.mockResolvedValue({ disposition: "committed", response: {} }); + touchDrawerMock.enabled = false; +}); + function task(overrides: Partial = {}): TaskSwitcherItem { return { id: "task-1", title: "Task 1", state: "IN_PROGRESS", ...overrides }; } @@ -110,6 +133,60 @@ describe("TaskItemWithContextMenu — plugin primary actions", () => { }); }); +function renderWorkflowMoveMenu({ + selectedTaskIds, + onMoveToStep, + onBulkMove, +}: { + selectedTaskIds?: Set; + onMoveToStep?: (taskId: string, workflowId: string, targetStepId: string) => void; + onBulkMove?: (taskIds: string[], targetWorkflowId: string, targetStepId: string) => void; +} = {}) { + const bulkMove = onBulkMove ?? vi.fn(); + render( + + + + +
Task 1
+ + + + , + ); + return { bulkMove }; +} + +// Open the "Move to" submenu and hover the target step so its nested content +// is revealed. On fine pointers that content is the inline options form; on +// coarse/touch pointers it is the "Move with options" item that opens a Drawer. +async function openMoveSubmenu() { + fireEvent.contextMenu(screen.getByTestId("task-row")); + await screen.findByTestId(MOVE_TO_TEST_ID); + fireEvent.pointerMove(screen.getByTestId(MOVE_TO_TEST_ID), { + pointerType: "mouse", + }); + const targetStep = await screen.findByTestId(STEP_2_TEST_ID); + fireEvent.pointerMove(targetStep, { + pointerType: "mouse", + }); +} + // happy-dom's TouchEvent drops the touches/changedTouches init, so the touch // events in the cancellation tests stub it faithfully. function stubTouchEvent() { @@ -210,6 +287,110 @@ describe("TaskItemWithContextMenu — pointer containment", () => { }); }); +describe("TaskItemWithContextMenu — single-task move options", () => { + it("keeps the existing direct move as a one-selection action", async () => { + const onMoveToStep = vi.fn(); + renderWorkflowMoveMenu({ onMoveToStep }); + + fireEvent.contextMenu(screen.getByTestId("task-row")); + fireEvent.pointerMove(screen.getByTestId(MOVE_TO_TEST_ID), { + pointerType: "mouse", + }); + fireEvent.click(await screen.findByTestId(STEP_2_TEST_ID)); + + expect(onMoveToStep).toHaveBeenCalledOnce(); + expect(onMoveToStep).toHaveBeenCalledWith("task-1", WORKFLOW_1_ID, "step-2"); + expect(workflowMoveMock.move).not.toHaveBeenCalled(); + }); + + it("submits one-shot options through the single-task move payload", async () => { + renderWorkflowMoveMenu(); + await openMoveSubmenu(); + + const instructions = await screen.findByTestId(INSTRUCTIONS_TEST_ID); + fireEvent.change(instructions, { target: { value: " start review with tests " } }); + fireEvent.click(screen.getByTestId("workflow-move-submit")); + + await waitFor(() => + expect(workflowMoveMock.move).toHaveBeenCalledWith("task-1", { + workflow_id: WORKFLOW_1_ID, + workflow_step_id: "step-2", + position: 0, + entry_options: { instructions: "start review with tests" }, + }), + ); + }); + + it("keeps desktop one-shot options when the move is rejected", async () => { + workflowMoveMock.move.mockResolvedValueOnce({ + disposition: "failed", + error: new Error("move rejected"), + response: {}, + }); + renderWorkflowMoveMenu(); + await openMoveSubmenu(); + + const instructions = await screen.findByTestId(INSTRUCTIONS_TEST_ID); + fireEvent.change(instructions, { target: { value: " retry review later " } }); + fireEvent.click(screen.getByTestId("workflow-move-submit")); + + await waitFor(() => expect(workflowMoveMock.move).toHaveBeenCalledOnce()); + expect((screen.getByTestId(INSTRUCTIONS_TEST_ID) as HTMLTextAreaElement).value).toBe( + " retry review later ", + ); + }); + + it("exposes inline move options under a one-task selection step", async () => { + renderWorkflowMoveMenu({ selectedTaskIds: new Set(["task-1"]) }); + + fireEvent.contextMenu(screen.getByTestId("task-row")); + await screen.findByTestId(MOVE_TO_TEST_ID); + + expect(screen.queryByTestId("task-context-move-with-options")).toBeNull(); + fireEvent.pointerMove(screen.getByTestId(MOVE_TO_TEST_ID), { + pointerType: "mouse", + }); + fireEvent.pointerMove(await screen.findByTestId(STEP_2_TEST_ID), { + pointerType: "mouse", + }); + // Fine pointers reveal the options form inline in the step's submenu, not a + // separate "Move with options" item that opens a dialog. + expect(await screen.findByTestId("workflow-move-skip-step-prompt")).toBeTruthy(); + expect(screen.getByTestId(INSTRUCTIONS_TEST_ID)).toBeTruthy(); + expect(screen.getByTestId("workflow-move-submit")).toBeTruthy(); + expect(screen.queryByTestId("task-context-step-options-step-2")).toBeNull(); + }); + + it("does not expose an options submenu for bulk selection moves", async () => { + const { bulkMove } = renderWorkflowMoveMenu({ + selectedTaskIds: new Set(["task-1", "task-2"]), + }); + fireEvent.contextMenu(screen.getByTestId("task-row")); + await screen.findByTestId(MOVE_TO_TEST_ID); + + expect(screen.queryByTestId("task-context-move-with-options")).toBeNull(); + expect(screen.getByTestId(MOVE_TO_TEST_ID)).toBeTruthy(); + expect(workflowMoveMock.move).not.toHaveBeenCalled(); + + fireEvent.pointerMove(screen.getByTestId(MOVE_TO_TEST_ID), { + pointerType: "mouse", + }); + fireEvent.click(await screen.findByTestId(STEP_2_TEST_ID)); + expect(bulkMove).toHaveBeenCalledWith(["task-1", "task-2"], WORKFLOW_1_ID, "step-2"); + }); + + it("uses the touch Drawer presentation for the shared form", async () => { + touchDrawerMock.enabled = true; + renderWorkflowMoveMenu(); + await openMoveSubmenu(); + + // Coarse/touch pointers keep the "Move with options" item that opens the + // shared form in a Drawer instead of rendering it inline. + fireEvent.click(await screen.findByTestId("task-context-step-options-step-2")); + expect(await screen.findByTestId("workflow-move-options")).toBeTruthy(); + }); +}); + describe("TaskItemWithContextMenu — touch drag cancellation", () => { it("opening the menu cancels an in-flight touch drag at the touchstart target", async () => { // A touch long-press arms the row's TouchSensor (250ms) before the menu diff --git a/apps/web/components/task/task-switcher-context-menu.tsx b/apps/web/components/task/task-switcher-context-menu.tsx index 9d9dc13133..b807441782 100644 --- a/apps/web/components/task/task-switcher-context-menu.tsx +++ b/apps/web/components/task/task-switcher-context-menu.tsx @@ -1,6 +1,6 @@ "use client"; -import { cloneElement, isValidElement, useRef, useState, type ReactNode } from "react"; +import { cloneElement, isValidElement, useState, type ReactNode } from "react"; import { useTranslation } from "react-i18next"; import { IconCopy, @@ -19,11 +19,16 @@ import { } from "@kandev/ui/context-menu"; import { TaskMoveContextMenuItems, + TaskMoveOptionsSurface, type TaskMoveWorkflow, + useTaskMoveOptions, } from "@/components/task/task-move-context-menu"; import { TaskNestContextMenuItems } from "@/components/task/task-nest-context-menu"; import { useTaskWorkflowMove } from "@/hooks/use-task-workflow-move"; import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint"; +import { useTouchDrawer } from "@/hooks/use-compact-task-chrome"; +import { isWorkflowMoveOptionsTarget } from "@/components/task/workflow-move-surface"; +import type { WorkflowMoveEntryOptions } from "@/lib/api/domains/kanban-api"; import { useUpdateTaskPriority } from "@/hooks/use-update-task-priority"; import { TaskColorMenu } from "./task-switcher-color-menu"; import { @@ -41,6 +46,7 @@ import type { StepDef, TaskSwitcherItem } from "./task-switcher-types"; import { TaskPluginPrimaryMenuItems } from "./task-switcher-plugin-menu-items"; import { useTaskSwitcherArchiveConfirmation } from "./task-switcher-archive-confirmation"; import { TaskPriorityContextMenu } from "./task-priority-context-menu"; +import { useMenuTouchDragCancel } from "./task-switcher-touch-drag-cancel"; export type { StepDef } from "./task-switcher-types"; export { createTaskLinkSelectAction } from "./task-switcher-link-menu"; @@ -57,6 +63,8 @@ type ContextMenuProps = TaskLinkHandlers & { onDeleteTask?: (taskId: string) => void; onDetachTask?: (taskId: string) => void; onMoveToStep?: (taskId: string, workflowId: string, targetStepId: string) => void; + onRequestMoveOptions?: (taskId: string, workflowId: string, targetStepId: string) => void; + onBeforeMoveOptionsOpen?: () => void; onTogglePin?: (taskId: string) => void; isPinned?: boolean; pinnedTaskIds?: string[]; @@ -73,96 +81,11 @@ type ContextMenuProps = TaskLinkHandlers & { isMixedWorkflowSelection?: boolean; }; -/** - * dnd-kit's TouchSensor arms on touchstart and activates after the 250ms - * delay — before a long-press (≈700ms) can open this context menu. A - * stationary long-press therefore starts a row drag that is still live when - * the menu opens. While a drag is active the TouchSensor listens for - * `touchcancel` on the element the touch started on, so dispatching one at - * that element aborts the drag (onDragCancel) instead of dropping it: the - * row stays put and the menu remains usable. Inert when no touch has started - * on this row (desktop right-click, or a sensor that already detached after a - * quick tap), because then nothing listens for the event. - */ -type CancelTouchDrag = (touchStartTarget: EventTarget | null) => void; - -const cancelTouchDrag: CancelTouchDrag = (touchStartTarget) => { - if (touchStartTarget instanceof Element && typeof TouchEvent === "function") { - touchStartTarget.dispatchEvent( - new TouchEvent("touchcancel", { bubbles: true, cancelable: true }), - ); - } -}; - -/** - * Coordinates the context menu with the row's touch-drag sensor: remembers - * the element the touch began on and cancels the in-flight drag when the menu - * opens. Returns the menu `onOpenChange` handler and the trigger-wrapper - * capture props. - */ -function useMenuTouchDragCancel(onOpenChange: (open: boolean) => void) { - const touchStartRef = useRef<{ target: EventTarget; identifier: number } | null>(null); - const menuOpenRef = useRef(false); - const handleOpenChange = (open: boolean) => { - onOpenChange(open); - menuOpenRef.current = open; - if (open) { - // A touch long-press has already armed the row's TouchSensor (250ms) - // when the menu opens (~700ms); cancel that drag at the touchstart - // target so the menu gesture never moves the row. - const target = touchStartRef.current?.target ?? null; - touchStartRef.current = null; - cancelTouchDrag(target); - } else { - touchStartRef.current = null; - } - }; - return { - handleOpenChange, - triggerProps: { - // The TouchSensor attaches its touchcancel listener to the element the - // touch began on while a drag is active. Track only the first touch of - // a single-touch gesture (dnd-kit's TouchSensor rejects multi-touch) - // and only while the menu is closed, and drop the target when that - // touch ends or the menu closes — not when another finger lifts — so a - // later open never dispatches a synthetic touchcancel for a gesture - // that is no longer active (pull-to-refresh and touch-scroll listen for - // bubbled touchcancel). - onTouchStartCapture: (event: React.TouchEvent) => { - if (menuOpenRef.current || event.touches.length !== 1) return; - if (!touchStartRef.current) { - touchStartRef.current = { - target: event.target, - identifier: event.touches[0].identifier, - }; - } - }, - onTouchEndCapture: (event: React.TouchEvent) => { - const tracked = touchStartRef.current; - if ( - tracked && - Array.from(event.changedTouches).some((t) => t.identifier === tracked.identifier) - ) { - touchStartRef.current = null; - } - }, - onTouchCancelCapture: (event: React.TouchEvent) => { - const tracked = touchStartRef.current; - if ( - tracked && - Array.from(event.changedTouches).some((t) => t.identifier === tracked.identifier) - ) { - touchStartRef.current = null; - } - }, - }, - }; -} - // This component coordinates the context menu and drag cancellation. Archive // state lives in its focused adapter so unavailable actions stay unavailable. export function TaskItemWithContextMenu(props: ContextMenuProps) { const { children, ...menuProps } = props; + const { task, stepsByWorkflowId, steps, onRequestMoveOptions, onBeforeMoveOptionsOpen } = props; const [contextOpen, setContextOpen] = useState(false); const [menuKey, setMenuKey] = useState(0); const moveTasks = useTaskWorkflowMove(); @@ -172,6 +95,7 @@ export function TaskItemWithContextMenu(props: ContextMenuProps) { }; const { handleOpenChange, triggerProps } = useMenuTouchDragCancel(setContextOpen); const { isFinePointer } = useResponsiveBreakpoint(); + const usesTouchDrawer = useTouchDrawer(); const archive = useTaskSwitcherArchiveConfirmation({ task: menuProps.task, onArchiveTask: menuProps.onArchiveTask, @@ -181,45 +105,124 @@ export function TaskItemWithContextMenu(props: ContextMenuProps) { const archiveConfirmation = archive.archiveOpen ? archive.archiveConfirmation : undefined; const inlineArchiveConfirmation = isFinePointer ? undefined : archiveConfirmation; const portaledArchiveConfirmation = isFinePointer ? archiveConfirmation : undefined; + const { + moveOptionsStep, + isMoving, + openMoveOptions, + submitMoveOptions, + submitMoveOptionsForStep, + closeMoveOptions, + } = useTaskMoveOptions({ + taskId: task.id, + workflowId: task.workflowId, + steps: task.workflowId ? (stepsByWorkflowId?.[task.workflowId] ?? steps) : steps, + closeMenu, + }); + const handleMoveToStepWithOptions = (targetStepId: string) => { + if (onRequestMoveOptions && task.workflowId) { + closeMenu(); + onRequestMoveOptions(task.id, task.workflowId, targetStepId); + return; + } + onBeforeMoveOptionsOpen?.(); + openMoveOptions(targetStepId); + }; + // Fine pointers get the options inline in the "Move to" submenu; coarse/touch + // pointers keep the long-press Drawer surface driven by + // handleMoveToStepWithOptions. A wired onRequestMoveOptions (mobile sheet) + // owns its own drawer, so the inline path stays off there too. + const inlineSubmitWithOptions = + !usesTouchDrawer && !onRequestMoveOptions ? submitMoveOptionsForStep : undefined; + const contextMenuProps = buildTaskContextMenuItemsProps(props, closeMenu, moveTasks, { + onMoveToStepWithOptions: handleMoveToStepWithOptions, + onSubmitWithOptions: inlineSubmitWithOptions, + isMoving, + }); return ( - - -
- {cloneWithMenuOpen(children, contextOpen, inlineArchiveConfirmation)} - {portaledArchiveConfirmation} -
- - event.stopPropagation()} - onPointerDown={(event) => event.stopPropagation()} - onTouchStart={(event) => event.stopPropagation()} - onClick={(event) => event.stopPropagation()} - > - - - + <> + + +
+ {cloneWithMenuOpen(children, contextOpen, inlineArchiveConfirmation)} + {portaledArchiveConfirmation} +
+ + { + if (isWorkflowMoveOptionsTarget(event.target)) { + event.preventDefault(); + } + }} + // The menu renders in a portal whose fiber ancestors include the + // dnd-kit drag handle that wraps the row. React synthetic events + // bubble through the fiber tree, not the DOM, so without these guards + // a mousedown/pointerdown/touchstart on any menu item (e.g. the Color + // submenu trigger or a swatch) reaches the handle's sensor listeners + // and starts a row drag, and a click activates the row. Bubble-phase + // guards run after the item's own handlers, so menu actions still work. + onMouseDown={(event) => event.stopPropagation()} + onPointerDown={(event) => event.stopPropagation()} + onTouchStart={(event) => event.stopPropagation()} + onClick={(event) => event.stopPropagation()} + > + + + + + ); } -type TaskContextMenuItemsProps = Omit & { +type TaskContextMenuItemsProps = Omit< + ContextMenuProps, + "children" | "onBeforeMoveOptionsOpen" | "onRequestMoveOptions" +> & { closeMenu: () => void; moveTasks: ReturnType; + onMoveToStepWithOptions?: (targetStepId: string) => void; + onSubmitWithOptions?: ( + stepId: string, + entryOptions: WorkflowMoveEntryOptions | undefined, + ) => Promise; + moveOptionsBusy?: boolean; }; +function buildTaskContextMenuItemsProps( + props: ContextMenuProps, + closeMenu: () => void, + moveTasks: ReturnType, + moveOptions: { + onMoveToStepWithOptions: (targetStepId: string) => void; + onSubmitWithOptions?: ( + stepId: string, + entryOptions: WorkflowMoveEntryOptions | undefined, + ) => Promise; + isMoving?: boolean; + }, +): TaskContextMenuItemsProps { + const { + children: _children, + onRequestMoveOptions: _onRequestMoveOptions, + onBeforeMoveOptionsOpen: _onBeforeMoveOptionsOpen, + ...rest + } = props; + return { + ...rest, + onMoveToStepWithOptions: moveOptions.onMoveToStepWithOptions, + onSubmitWithOptions: moveOptions.onSubmitWithOptions, + moveOptionsBusy: moveOptions.isMoving, + closeMenu, + moveTasks, + }; +} + function TaskContextMenuItems(props: TaskContextMenuItemsProps) { const { task, selectedTaskIds } = props; // Right-clicking any row that's part of the active selection acts on the whole @@ -243,38 +246,31 @@ function TaskContextMenuItems(props: TaskContextMenuItemsProps) { ); } -function SingleSelectionMenuItems({ +type TaskHeaderMenuItemsProps = Pick< + TaskContextMenuItemsProps, + | "task" + | "isDeleting" + | "isPinned" + | "onClearSelection" + | "onTogglePin" + | "onEditTask" + | "onRenameTask" + | "onCreateSubtask" +> & { actingOnSelection: boolean }; + +function TaskHeaderMenuItems({ task, - workflows, - stepsByWorkflowId, - steps, + isDeleting, + isPinned, + actingOnSelection, + onClearSelection, + onTogglePin, onEditTask, onRenameTask, - onArchiveTask, onCreateSubtask, - onDeleteTask, - onDetachTask, - onMoveToStep, - onTogglePin, - isPinned, - isDeleting, - isArchiving, - onBulkArchive, - onBulkMove, - onClearSelection, - isMixedWorkflowSelection, - closeMenu, - moveTasks, - actingIds, - actingOnSelection, - ...linkHandlers -}: TaskContextMenuItemsProps & { actingIds: string[]; actingOnSelection: boolean }) { +}: TaskHeaderMenuItemsProps) { const { t } = useTranslation(); const updateTaskPriority = useUpdateTaskPriority(); - // Acting on a lone selected row (Pin / Delete) must drop it from the selection - // so later plain clicks navigate instead of toggling. - const onDelete = withSelectionClear(actingOnSelection, onClearSelection, onDeleteTask); - const onDetach = withSelectionClear(actingOnSelection, onClearSelection, onDetachTask); return ( <> )} + + ); +} + +function SingleSelectionMenuItems({ + task, + workflows, + stepsByWorkflowId, + steps, + onEditTask, + onRenameTask, + onArchiveTask, + onCreateSubtask, + onDeleteTask, + onDetachTask, + onMoveToStep, + onMoveToStepWithOptions, + onSubmitWithOptions, + moveOptionsBusy, + onTogglePin, + isPinned, + isDeleting, + isArchiving, + onBulkArchive, + onBulkMove, + onClearSelection, + isMixedWorkflowSelection, + closeMenu, + moveTasks, + actingIds, + actingOnSelection, + ...linkHandlers +}: TaskContextMenuItemsProps & { actingIds: string[]; actingOnSelection: boolean }) { + // Acting on a lone selected row (Pin / Delete) must drop it from the selection + // so later plain clicks navigate instead of toggling. + const onDelete = withSelectionClear(actingOnSelection, onClearSelection, onDeleteTask); + const onDetach = withSelectionClear(actingOnSelection, onClearSelection, onDetachTask); + return ( + <> + { if (actingOnSelection) { runSelectionMove(targetWorkflowId, stepId, "workflow"); diff --git a/apps/web/components/task/task-switcher-row.tsx b/apps/web/components/task/task-switcher-row.tsx index 33d863d3e0..9693185d0f 100644 --- a/apps/web/components/task/task-switcher-row.tsx +++ b/apps/web/components/task/task-switcher-row.tsx @@ -41,6 +41,8 @@ export type TaskRowProps = { onLinkLinearIssue?: TaskLinkHandler; onLinkSentryIssue?: TaskLinkHandler; onMoveToStep?: (taskId: string, workflowId: string, targetStepId: string) => void; + onRequestMoveOptions?: (taskId: string, workflowId: string, targetStepId: string) => void; + onBeforeMoveOptionsOpen?: () => void; onTogglePin?: (taskId: string) => void; isPinned?: boolean; pinnedTaskIds?: string[]; @@ -77,6 +79,8 @@ function getContextMenuProps(props: TaskRowProps, isArchived: boolean) { onLinkLinearIssue: archiveAware(props.onLinkLinearIssue, isArchived), onLinkSentryIssue: archiveAware(props.onLinkSentryIssue, isArchived), onMoveToStep: archiveAware(props.onMoveToStep, isArchived), + onRequestMoveOptions: archiveAware(props.onRequestMoveOptions, isArchived), + onBeforeMoveOptionsOpen: props.onBeforeMoveOptionsOpen, onTogglePin: archiveAware(props.onTogglePin, isArchived), isPinned: isArchived ? false : props.isPinned, pinnedTaskIds: props.pinnedTaskIds, diff --git a/apps/web/components/task/task-switcher-touch-drag-cancel.ts b/apps/web/components/task/task-switcher-touch-drag-cancel.ts new file mode 100644 index 0000000000..dc367ea383 --- /dev/null +++ b/apps/web/components/task/task-switcher-touch-drag-cancel.ts @@ -0,0 +1,87 @@ +import { useRef } from "react"; + +/** + * dnd-kit's TouchSensor arms on touchstart and activates after the 250ms + * delay — before a long-press (≈700ms) can open this context menu. A + * stationary long-press therefore starts a row drag that is still live when + * the menu opens. While a drag is active the TouchSensor listens for + * `touchcancel` on the element the touch started on, so dispatching one at + * that element aborts the drag (onDragCancel) instead of dropping it: the + * row stays put and the menu remains usable. Inert when no touch has started + * on this row (desktop right-click, or a sensor that already detached after a + * quick tap), because then nothing listens for the event. + */ +type CancelTouchDrag = (touchStartTarget: EventTarget | null) => void; + +const cancelTouchDrag: CancelTouchDrag = (touchStartTarget) => { + if (touchStartTarget instanceof Element && typeof TouchEvent === "function") { + touchStartTarget.dispatchEvent( + new TouchEvent("touchcancel", { bubbles: true, cancelable: true }), + ); + } +}; + +/** + * Coordinates the context menu with the row's touch-drag sensor: remembers + * the element the touch began on and cancels the in-flight drag when the menu + * opens. Returns the menu `onOpenChange` handler and the trigger-wrapper + * capture props. + */ +export function useMenuTouchDragCancel(onOpenChange: (open: boolean) => void) { + const touchStartRef = useRef<{ target: EventTarget; identifier: number } | null>(null); + const menuOpenRef = useRef(false); + const handleOpenChange = (open: boolean) => { + onOpenChange(open); + menuOpenRef.current = open; + if (open) { + // A touch long-press has already armed the row's TouchSensor (250ms) + // when the menu opens (~700ms); cancel that drag at the touchstart + // target so the menu gesture never moves the row. + const target = touchStartRef.current?.target ?? null; + touchStartRef.current = null; + cancelTouchDrag(target); + } else { + touchStartRef.current = null; + } + }; + return { + handleOpenChange, + triggerProps: { + // The TouchSensor attaches its touchcancel listener to the element the + // touch began on while a drag is active. Track only the first touch of + // a single-touch gesture (dnd-kit's TouchSensor rejects multi-touch) + // and only while the menu is closed, and drop the target when that + // touch ends or the menu closes — not when another finger lifts — so a + // later open never dispatches a synthetic touchcancel for a gesture + // that is no longer active (pull-to-refresh and touch-scroll listen for + // bubbled touchcancel). + onTouchStartCapture: (event: React.TouchEvent) => { + if (menuOpenRef.current || event.touches.length !== 1) return; + if (!touchStartRef.current) { + touchStartRef.current = { + target: event.target, + identifier: event.touches[0].identifier, + }; + } + }, + onTouchEndCapture: (event: React.TouchEvent) => { + const tracked = touchStartRef.current; + if ( + tracked && + Array.from(event.changedTouches).some((t) => t.identifier === tracked.identifier) + ) { + touchStartRef.current = null; + } + }, + onTouchCancelCapture: (event: React.TouchEvent) => { + const tracked = touchStartRef.current; + if ( + tracked && + Array.from(event.changedTouches).some((t) => t.identifier === tracked.identifier) + ) { + touchStartRef.current = null; + } + }, + }, + }; +} diff --git a/apps/web/components/task/task-switcher-types.ts b/apps/web/components/task/task-switcher-types.ts index 200d990dfe..d20b28b3f4 100644 --- a/apps/web/components/task/task-switcher-types.ts +++ b/apps/web/components/task/task-switcher-types.ts @@ -17,6 +17,7 @@ export type StepDef = { id: string; title: string; color?: string; + agent_profile_id?: string | null; events?: { on_enter?: Array<{ type: string; config?: Record }> }; }; @@ -102,6 +103,8 @@ export type TaskSwitcherProps = { onLinkLinearIssue?: TaskLinkHandler; onLinkSentryIssue?: TaskLinkHandler; onMoveToStep?: (taskId: string, workflowId: string, targetStepId: string) => void; + onRequestMoveOptions?: (taskId: string, workflowId: string, targetStepId: string) => void; + onBeforeMoveOptionsOpen?: () => void; onTogglePin?: (taskId: string) => void; onReorderGroup?: (groupTaskIds: string[]) => void; onReorderSubtasks?: (parentTaskId: string, orderedSubtaskIds: string[]) => void; diff --git a/apps/web/components/task/task-switcher.tsx b/apps/web/components/task/task-switcher.tsx index e7f4cb6068..d9e102223a 100644 --- a/apps/web/components/task/task-switcher.tsx +++ b/apps/web/components/task/task-switcher.tsx @@ -39,6 +39,8 @@ const TASK_ROW_HANDLER_KEYS = [ "onLinkLinearIssue", "onLinkSentryIssue", "onMoveToStep", + "onRequestMoveOptions", + "onBeforeMoveOptionsOpen", "onTogglePin", "onToggleSelectTask", "onSelectTaskRange", @@ -109,6 +111,8 @@ function buildTaskRowProps( onLinkLinearIssue: optionalHandler(props, handlers, "onLinkLinearIssue"), onLinkSentryIssue: optionalHandler(props, handlers, "onLinkSentryIssue"), onMoveToStep: optionalHandler(props, handlers, "onMoveToStep"), + onRequestMoveOptions: optionalHandler(props, handlers, "onRequestMoveOptions"), + onBeforeMoveOptionsOpen: optionalHandler(props, handlers, "onBeforeMoveOptionsOpen"), onTogglePin: optionalHandler(props, handlers, "onTogglePin"), pinnedTaskIds: props.pinnedTaskIds, deletingTaskId: props.deletingTaskId, diff --git a/apps/web/components/task/workflow-move-options-form.test.tsx b/apps/web/components/task/workflow-move-options-form.test.tsx new file mode 100644 index 0000000000..6ea7bee2c8 --- /dev/null +++ b/apps/web/components/task/workflow-move-options-form.test.tsx @@ -0,0 +1,100 @@ +import { act, cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { TooltipProvider } from "@kandev/ui/tooltip"; +import { WorkflowMoveOptions } from "./workflow-move-options"; + +const touchMocks = vi.hoisted(() => ({ enabled: false })); + +const RESET_TEST_ID = "workflow-move-reset-context"; +const SKIP_TEST_ID = "workflow-move-skip-step-prompt"; +const INSTRUCTIONS_TEST_ID = "workflow-move-instructions"; +const SUBMIT_TEST_ID = "workflow-move-submit"; + +vi.mock("@/hooks/use-compact-task-chrome", () => ({ + useTouchDrawer: () => touchMocks.enabled, +})); + +afterEach(() => { + cleanup(); + touchMocks.enabled = false; +}); + +function renderOptions(onSubmit = vi.fn(), onOpenChange = vi.fn(), touch = true) { + touchMocks.enabled = touch; + render( + + + , + ); + return { onSubmit, onOpenChange }; +} + +describe("WorkflowMoveOptions", () => { + it("exposes the reset-context, skip-step-prompt, and instruction overrides", () => { + renderOptions(); + + expect(screen.getByTestId(RESET_TEST_ID)).toBeTruthy(); + expect(screen.getByTestId(SKIP_TEST_ID)).toBeTruthy(); + expect(screen.getByTestId(INSTRUCTIONS_TEST_ID)).toBeTruthy(); + }); + + it("submits the draft payload and keeps values when the move fails", async () => { + const onSubmit = vi.fn().mockResolvedValue(false); + renderOptions(onSubmit); + + fireEvent.change(screen.getByTestId(INSTRUCTIONS_TEST_ID), { + target: { value: " create the PR ready for review " }, + }); + fireEvent.click(screen.getByTestId(SUBMIT_TEST_ID)); + await act(async () => {}); + + expect(onSubmit).toHaveBeenCalledWith({ + instructions: "create the PR ready for review", + }); + expect((screen.getByTestId(INSTRUCTIONS_TEST_ID) as HTMLTextAreaElement).value).toBe( + " create the PR ready for review ", + ); + }); + + it("includes the skip-step-prompt flag in the one-time payload", async () => { + const onSubmit = vi.fn().mockResolvedValue(true); + renderOptions(onSubmit); + + fireEvent.click(screen.getByTestId(SKIP_TEST_ID)); + fireEvent.click(screen.getByTestId(SUBMIT_TEST_ID)); + await act(async () => {}); + + expect(onSubmit).toHaveBeenCalledWith({ skip_step_prompt: true }); + }); +}); + +describe("WorkflowMoveOptions presentation", () => { + it("closes through the cancel action", () => { + const { onOpenChange } = renderOptions(); + + fireEvent.click(screen.getByRole("button", { name: /cancel/i })); + + expect(onOpenChange).toHaveBeenCalledWith(false); + }); + + it("uses the Drawer presentation on touch surfaces", () => { + touchMocks.enabled = true; + renderOptions(); + + expect(screen.getByTestId("workflow-move-options")).toBeTruthy(); + expect(document.querySelector('[data-slot="drawer-content"]')).toBeTruthy(); + }); + + it("uses the Dialog presentation on fine-pointer surfaces", () => { + renderOptions(vi.fn(), vi.fn(), false); + + expect(screen.queryByTestId("workflow-move-options")).toBeNull(); + expect(document.querySelector('[data-slot="dialog-content"]')).toBeNull(); + expect(document.querySelector('[data-slot="drawer-content"]')).toBeNull(); + }); +}); diff --git a/apps/web/components/task/workflow-move-options.test.ts b/apps/web/components/task/workflow-move-options.test.ts new file mode 100644 index 0000000000..ca0adbc7d3 --- /dev/null +++ b/apps/web/components/task/workflow-move-options.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from "vitest"; +import { workflowMoveOptionsPayload } from "./workflow-move-options"; + +describe("workflowMoveOptionsPayload", () => { + it("trims values and omits empty overrides", () => { + expect( + workflowMoveOptionsPayload({ + resetContext: true, + instructions: " create the PR ready for review ", + skipStepPrompt: true, + }), + ).toEqual({ + reset_context: true, + instructions: "create the PR ready for review", + skip_step_prompt: true, + }); + }); + + it("returns undefined for a destination-only move", () => { + expect( + workflowMoveOptionsPayload({ + resetContext: false, + instructions: "", + skipStepPrompt: false, + }), + ).toBeUndefined(); + }); +}); diff --git a/apps/web/components/task/workflow-move-options.tsx b/apps/web/components/task/workflow-move-options.tsx new file mode 100644 index 0000000000..4cb9404bb3 --- /dev/null +++ b/apps/web/components/task/workflow-move-options.tsx @@ -0,0 +1,333 @@ +"use client"; + +import { useCallback, useState } from "react"; +import { IconInfoCircle } from "@tabler/icons-react"; +import { Button } from "@kandev/ui/button"; +import { Checkbox } from "@kandev/ui/checkbox"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogHeader, + DialogTitle, +} from "@kandev/ui/dialog"; +import { + Drawer, + DrawerContent, + DrawerDescription, + DrawerHeader, + DrawerTitle, +} from "@kandev/ui/drawer"; +import { Textarea } from "@kandev/ui/textarea"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; +import { useTouchDrawer } from "@/hooks/use-compact-task-chrome"; +import { cn } from "@/lib/utils"; +import { useTranslation } from "react-i18next"; +import type { WorkflowMoveEntryOptions } from "@/lib/api/domains/kanban-api"; + +export type WorkflowMoveOptionsDraft = { + resetContext: boolean; + instructions: string; + skipStepPrompt: boolean; +}; + +const EMPTY_DRAFT: WorkflowMoveOptionsDraft = { + resetContext: false, + instructions: "", + skipStepPrompt: false, +}; + +export function workflowMoveOptionsPayload( + draft: WorkflowMoveOptionsDraft, +): WorkflowMoveEntryOptions | undefined { + const payload: WorkflowMoveEntryOptions = {}; + if (draft.resetContext) payload.reset_context = true; + if (draft.skipStepPrompt) payload.skip_step_prompt = true; + if (draft.instructions.trim()) payload.instructions = draft.instructions.trim(); + return Object.keys(payload).length > 0 ? payload : undefined; +} + +/** + * Draft state shared by every move-options surface (stepper disclosure, + * proceed popover, dialog, drawer). + */ +export function useWorkflowMoveOptionsForm() { + const [draft, setDraft] = useState(EMPTY_DRAFT); + + const patchDraft = useCallback((patch: Partial) => { + setDraft((current) => ({ ...current, ...patch })); + }, []); + const resetDraft = useCallback(() => setDraft(EMPTY_DRAFT), []); + + return { + draft, + patchDraft, + resetDraft, + }; +} + +type WorkflowMoveOptionsFieldsProps = { + draft: WorkflowMoveOptionsDraft; + onDraftChange: (patch: Partial) => void; + isTouchSurface: boolean; + instructionsRows?: number; +}; + +/** + * Reset-context, skip-step-prompt, and one-time instructions fields. Font size + * is inherited from the hosting surface (Dialog, Drawer, and the stepper hover + * card all share the `text-xs/relaxed` popover base) so the fields never stand + * out larger than the surrounding chrome. Checkboxes align to the first text + * line so a wrapping label reads cleanly. The skip-step-prompt consequence is + * disclosed through an info-icon tooltip (hover on desktop, focus/tap on touch) + * rather than an always-visible paragraph. + */ +export function WorkflowMoveOptionsFields({ + draft, + onDraftChange, + isTouchSurface, + instructionsRows = 4, +}: WorkflowMoveOptionsFieldsProps) { + const { t } = useTranslation(); + const rowClass = isTouchSurface ? "flex items-center gap-3 min-h-11" : "flex items-start gap-2.5"; + const checkboxClass = cn("shrink-0", !isTouchSurface && "mt-0.5"); + const hintButtonClass = cn( + "inline-flex shrink-0 cursor-pointer items-center justify-center rounded-sm text-muted-foreground hover:text-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring", + isTouchSurface ? "size-11" : "mt-0.5 size-4", + ); + return ( +
+ +
+ + + + + + + {t("task:workflowMoveSkipStepPromptHint")} + + +
+