fix: complete same-step MCP task moves (#3872) (#3876)

This commit is contained in:
Carlos Florêncio
2026-09-23 10:57:22 +01:00
committed by GitHub
parent 41eeaf0c70
commit 0386e3e9b7
20 changed files with 1001 additions and 16 deletions
@@ -9,15 +9,18 @@ import (
"github.com/google/uuid"
"github.com/kandev/kandev/internal/authz"
"github.com/kandev/kandev/internal/automation"
"github.com/kandev/kandev/internal/orchestrator/messagequeue"
"github.com/kandev/kandev/internal/steptelemetry"
"github.com/kandev/kandev/internal/task/dto"
"github.com/kandev/kandev/internal/task/models"
"github.com/kandev/kandev/internal/task/repository/repoerrors"
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"
workflowservice "github.com/kandev/kandev/internal/workflow/service"
v1 "github.com/kandev/kandev/pkg/api/v1"
ws "github.com/kandev/kandev/pkg/websocket"
"go.uber.org/zap"
@@ -69,6 +72,9 @@ func (h *Handlers) handleMoveTask(ctx context.Context, msg *ws.Message) (*ws.Mes
}
req.EntryOptions = entryOptions
req.Prompt = ""
if response, handled, err := h.completeSameStepMove(ctx, msg, req); handled || err != nil {
return response, err
}
// 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
@@ -100,6 +106,111 @@ func (h *Handlers) handleMoveTask(ctx context.Context, msg *ws.Message) (*ws.Mes
return h.applyMoveTaskImmediate(ctx, msg, req, session)
}
func (h *Handlers) completeSameStepMove(ctx context.Context, msg *ws.Message, req moveTaskRequest) (*ws.Message, bool, error) {
if h.taskSvc == nil {
return moveTaskErrorResponse(msg, ws.ErrorCodeInternalError, "task service is not configured")
}
task, err := h.taskSvc.GetTask(ctx, req.TaskID)
if err != nil || task == nil {
h.logger.Error("move_task: failed to load task for destination classification",
zap.String("task_id", req.TaskID), zap.Error(err))
return moveTaskErrorResponse(msg, ws.ErrorCodeInternalError, "failed to load task for move validation")
}
if task.WorkflowID != req.WorkflowID || task.WorkflowStepID != req.WorkflowStepID {
return nil, false, nil
}
if code, message := h.validateSameStepMove(ctx, req, task); code != "" {
return moveTaskErrorResponse(msg, code, message)
}
response, err := ws.NewResponse(msg.ID, msg.Action, dto.MoveTaskResponse{
Task: dto.FromTask(task),
Disposition: moveDispositionApplied,
})
return response, true, err
}
func (h *Handlers) validateSameStepMove(ctx context.Context, req moveTaskRequest, task *models.Task) (string, string) {
if err := h.taskSvc.AuthorizeWorkspaceScope(ctx, task.WorkspaceID, authz.ScopeTaskWrite); err != nil {
if service.IsForbidden(err) {
return ws.ErrorCodeForbidden, "task is not writable"
}
h.logger.Error("move_task: failed to authorize task for same-step completion",
zap.String("task_id", req.TaskID), zap.Error(err))
return ws.ErrorCodeInternalError, "failed to authorize task for move"
}
if task.ArchivedAt != nil {
return ws.ErrorCodeConflict, "archived tasks cannot be moved"
}
if code, message := h.validateSameStepMoveTargets(ctx, req, task); code != "" {
return code, message
}
if err := workflowmove.ValidateEntryOptions(req.EntryOptions, workflowmove.MoveChangePositionOnly); err != nil {
return ws.ErrorCodeValidation, err.Error()
}
return "", ""
}
func (h *Handlers) validateSameStepMoveTargets(
ctx context.Context,
req moveTaskRequest,
task *models.Task,
) (string, string) {
if code, message := h.validateSameStepTargetWorkflow(ctx, req, task); code != "" {
return code, message
}
return h.validateSameStepTargetStep(ctx, req)
}
func (h *Handlers) validateSameStepTargetWorkflow(
ctx context.Context,
req moveTaskRequest,
task *models.Task,
) (string, string) {
targetWorkflow, err := h.taskSvc.GetWorkflow(ctx, req.WorkflowID)
if err != nil {
h.logger.Error("move_task: failed to look up target workflow",
zap.String("task_id", req.TaskID), zap.String("workflow_id", req.WorkflowID), zap.Error(err))
if errors.Is(err, repoerrors.ErrWorkflowNotFound) || errors.Is(err, workflowservice.ErrNotVisible) {
return ws.ErrorCodeValidation, "target workflow_id does not exist"
}
return ws.ErrorCodeInternalError, "failed to validate target workflow"
}
if targetWorkflow == nil {
return ws.ErrorCodeValidation, "target workflow_id does not exist"
}
if targetWorkflow.WorkspaceID != task.WorkspaceID {
return ws.ErrorCodeValidation, "target workflow is in a different workspace"
}
return "", ""
}
func (h *Handlers) validateSameStepTargetStep(ctx context.Context, req moveTaskRequest) (string, string) {
if h.workflowCtrl == nil {
return ws.ErrorCodeInternalError, "workflow validation is not configured"
}
stepResponse, err := h.workflowCtrl.GetStep(ctx, req.WorkflowStepID)
if err != nil {
h.logger.Error("move_task: failed to look up target workflow step",
zap.String("task_id", req.TaskID), zap.String("workflow_step_id", req.WorkflowStepID), zap.Error(err))
if errors.Is(err, workflowservice.ErrNotVisible) || errors.Is(err, wfmodels.ErrWorkflowStepNotFound) {
return ws.ErrorCodeValidation, "target workflow_step_id does not exist"
}
return ws.ErrorCodeInternalError, "failed to validate target workflow step"
}
if stepResponse == nil || stepResponse.Step == nil {
return ws.ErrorCodeValidation, "target workflow_step_id does not exist"
}
if stepResponse.Step.WorkflowID != req.WorkflowID {
return ws.ErrorCodeValidation, "target workflow_step_id does not belong to the requested workflow_id"
}
return "", ""
}
func moveTaskErrorResponse(msg *ws.Message, code, message string) (*ws.Message, bool, error) {
response, err := ws.NewError(msg.ID, msg.Action, code, message, nil)
return response, true, err
}
// deferMoveTask records a PendingMove for the agent's turn-end handler to
// apply. Optionally queues a hand-off prompt when the caller supplied one.
// Returns a synthetic moved-task DTO so the agent's tool call resolves
@@ -0,0 +1,390 @@
package handlers
import (
"context"
"encoding/json"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/jmoiron/sqlx"
"github.com/kandev/kandev/internal/auth/authn"
"github.com/kandev/kandev/internal/authz"
"github.com/kandev/kandev/internal/events/bus"
"github.com/kandev/kandev/internal/orchestrator/messagequeue"
"github.com/kandev/kandev/internal/task/dto"
"github.com/kandev/kandev/internal/task/models"
sqliterepo "github.com/kandev/kandev/internal/task/repository/sqlite"
"github.com/kandev/kandev/internal/task/service"
workflowcontroller "github.com/kandev/kandev/internal/workflow/controller"
workflowmodels "github.com/kandev/kandev/internal/workflow/models"
workflowmove "github.com/kandev/kandev/internal/workflow/move"
workflowrepo "github.com/kandev/kandev/internal/workflow/repository"
v1 "github.com/kandev/kandev/pkg/api/v1"
ws "github.com/kandev/kandev/pkg/websocket"
)
type sameStepMoveSession struct {
id string
state models.TaskSessionState
primary bool
}
type sameStepMoveFixture struct {
svc *service.Service
repo *sqliterepo.Repository
workflowCtrl *workflowcontroller.Controller
workflowRepo *workflowrepo.Repository
db *sqlx.DB
eventBus *bus.MemoryEventBus
taskID string
workspaceID string
workflowID string
stepID string
}
func newSameStepMoveFixture(t *testing.T, sessions ...sameStepMoveSession) *sameStepMoveFixture {
t.Helper()
svc, repo, workflowCtrl, workflowRepo, db, eventBus := newTestTaskServiceWithWorkflowDBAndEventBus(t)
fixture := &sameStepMoveFixture{
svc: svc,
repo: repo,
workflowCtrl: workflowCtrl,
workflowRepo: workflowRepo,
db: db,
eventBus: eventBus,
taskID: "task-same-step",
workspaceID: "ws-same-step",
workflowID: "wf-same-step",
stepID: "step-work",
}
ctx := context.Background()
now := time.Now().UTC()
require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{
ID: fixture.workspaceID, Name: "Same step", OwnerID: "task-owner", CreatedAt: now, UpdatedAt: now,
}))
require.NoError(t, repo.CreateWorkflow(ctx, &models.Workflow{
ID: fixture.workflowID, WorkspaceID: fixture.workspaceID, Name: "Board", CreatedAt: now, UpdatedAt: now,
}))
require.NoError(t, workflowRepo.CreateStep(ctx, &workflowmodels.WorkflowStep{
ID: fixture.stepID, WorkflowID: fixture.workflowID, Name: "Work", Position: 0,
CreatedAt: now, UpdatedAt: now,
}))
require.NoError(t, repo.CreateTask(ctx, &models.Task{
ID: fixture.taskID, WorkspaceID: fixture.workspaceID, WorkflowID: fixture.workflowID,
WorkflowStepID: fixture.stepID, Position: 7, Title: "Task", State: v1.TaskStateInProgress,
Metadata: map[string]interface{}{"same_step_test": "preserve"}, CreatedAt: now, UpdatedAt: now,
}))
_, err := db.Exec(db.Rebind(`UPDATE tasks SET position = ? WHERE id = ?`), 7, fixture.taskID)
require.NoError(t, err)
for _, session := range sessions {
require.NoError(t, repo.CreateTaskSession(ctx, &models.TaskSession{
ID: session.id, TaskID: fixture.taskID, State: session.state, IsPrimary: session.primary,
Metadata: map[string]interface{}{"session_test": session.id}, StartedAt: now, UpdatedAt: now,
}))
}
return fixture
}
func (f *sameStepMoveFixture) handler(t *testing.T, queue MessageQueuer) *Handlers {
return &Handlers{
taskSvc: f.svc,
workflowCtrl: f.workflowCtrl,
messageQueue: queue,
logger: testLogger(t),
}
}
func (f *sameStepMoveFixture) message(t *testing.T, fields map[string]interface{}) *ws.Message {
t.Helper()
payload := map[string]interface{}{
"task_id": f.taskID, "workflow_id": f.workflowID, "workflow_step_id": f.stepID,
}
for key, value := range fields {
payload[key] = value
}
return makeWSMessage(t, ws.ActionMCPMoveTask, payload)
}
func decodeMoveTaskResponse(t *testing.T, response *ws.Message) dto.MoveTaskResponse {
t.Helper()
require.Equal(t, ws.MessageTypeResponse, response.Type, "payload: %s", string(response.Payload))
var result dto.MoveTaskResponse
require.NoError(t, json.Unmarshal(response.Payload, &result))
return result
}
func transitionCount(t *testing.T, db *sqlx.DB, taskID string) int {
t.Helper()
var count int
require.NoError(t, db.Get(&count, db.Rebind(`SELECT COUNT(*) FROM task_step_transitions WHERE task_id = ?`), taskID))
return count
}
// @covers AC-TASKS-MCP-MOVE-RESULTS-001.1
func TestHandleMoveTask_SameStepApplied(t *testing.T) {
cases := []struct {
name string
sessions []sameStepMoveSession
}{
{name: "running primary", sessions: []sameStepMoveSession{{id: "session-running", state: models.TaskSessionStateRunning, primary: true}}},
{name: "starting primary", sessions: []sameStepMoveSession{{id: "session-starting", state: models.TaskSessionStateStarting, primary: true}}},
{name: "idle primary", sessions: []sameStepMoveSession{{id: "session-idle", state: models.TaskSessionStateIdle, primary: true}}},
{name: "no session"},
{name: "mixed sibling sessions", sessions: []sameStepMoveSession{
{id: "session-primary-idle", state: models.TaskSessionStateIdle, primary: true},
{id: "session-sibling-running", state: models.TaskSessionStateRunning},
}},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
fixture := newSameStepMoveFixture(t, tc.sessions...)
response, err := fixture.handler(t, nil).handleMoveTask(context.Background(), fixture.message(t, nil))
require.NoError(t, err)
result := decodeMoveTaskResponse(t, response)
assert.Equal(t, moveDispositionApplied, result.Disposition)
assert.Equal(t, fixture.taskID, result.Task.ID)
assert.Equal(t, fixture.stepID, result.Task.WorkflowStepID)
assert.Equal(t, 7, result.Task.Position)
assert.Empty(t, result.MoveID)
assert.Nil(t, result.EntryOptions)
assert.Empty(t, result.WorkflowEntryIdentity)
})
}
}
// @covers AC-TASKS-MCP-MOVE-RESULTS-001.2
func TestHandleMoveTask_SameStepReturnsStoredTask(t *testing.T) {
fixture := newSameStepMoveFixture(t)
for _, tc := range []struct {
name string
fields map[string]interface{}
}{
{name: "position omitted"},
{name: "requested position ignored", fields: map[string]interface{}{"position": 99}},
{name: "empty entry options normalized", fields: map[string]interface{}{"entry_options": map[string]interface{}{}}},
} {
t.Run(tc.name, func(t *testing.T) {
response, err := fixture.handler(t, nil).handleMoveTask(context.Background(), fixture.message(t, tc.fields))
require.NoError(t, err)
result := decodeMoveTaskResponse(t, response)
assert.Equal(t, 7, result.Task.Position)
assert.Equal(t, map[string]interface{}{"same_step_test": "preserve"}, result.Task.Metadata)
assert.Empty(t, result.MoveID)
assert.Nil(t, result.EntryOptions)
})
}
}
// @covers AC-TASKS-MCP-MOVE-RESULTS-001.3
func TestHandleMoveTask_SameStepPreservesEffects(t *testing.T) {
fixture := newSameStepMoveFixture(t, sameStepMoveSession{
id: "session-running", state: models.TaskSessionStateRunning, primary: true,
})
ctx := context.Background()
queue := &pendingMoveRecordingQueuer{
recordingMessageQueuer: recordingMessageQueuer{calls: []messagequeue.QueuedMessage{{
ID: "existing-prompt", SessionID: "other-session", TaskID: "other-task", Content: "keep this prompt",
}}},
pendingSessionID: "other-session",
pendingMoves: []messagequeue.PendingMove{{
MoveID: "existing-move", TaskID: "other-task", WorkflowID: "other-workflow", WorkflowStepID: "other-step", Position: 4,
}},
}
beforeMessages := append([]messagequeue.QueuedMessage(nil), queue.calls...)
beforeMoves := append([]messagequeue.PendingMove(nil), queue.pendingMoves...)
beforeTask, err := fixture.svc.GetTask(ctx, fixture.taskID)
require.NoError(t, err)
beforeSession, err := fixture.repo.GetTaskSession(ctx, "session-running")
require.NoError(t, err)
beforeTransitions := transitionCount(t, fixture.db, fixture.taskID)
eventCount := 0
subscription, err := fixture.eventBus.Subscribe(">", func(context.Context, *bus.Event) error {
eventCount++
return nil
})
require.NoError(t, err)
t.Cleanup(func() { _ = subscription.Unsubscribe() })
handler := fixture.handler(t, queue)
for range 47 {
response, err := handler.handleMoveTask(ctx, fixture.message(t, map[string]interface{}{
"entry_options": map[string]interface{}{},
}))
require.NoError(t, err)
result := decodeMoveTaskResponse(t, response)
assert.Equal(t, moveDispositionApplied, result.Disposition)
}
afterTask, err := fixture.svc.GetTask(ctx, fixture.taskID)
require.NoError(t, err)
afterSession, err := fixture.repo.GetTaskSession(ctx, "session-running")
require.NoError(t, err)
assert.Equal(t, beforeTask, afterTask)
assert.Equal(t, beforeSession, afterSession)
assert.Equal(t, beforeMessages, queue.calls)
assert.Equal(t, beforeMoves, queue.pendingMoves)
assert.Equal(t, beforeTransitions, transitionCount(t, fixture.db, fixture.taskID))
assert.Zero(t, eventCount)
}
// @covers AC-TASKS-MCP-MOVE-RESULTS-001.3
func TestHandleMoveTask_SameStepPersistentQueue(t *testing.T) {
fixture := newSameStepMoveFixture(t, sameStepMoveSession{
id: "session-running", state: models.TaskSessionStateRunning, primary: true,
})
ctx := context.Background()
queueRepo, err := messagequeue.NewSQLiteRepository(fixture.db, fixture.db)
require.NoError(t, err)
queue := messagequeue.NewService(queueRepo, 0, testLogger(t))
now := time.Now().UTC()
require.NoError(t, fixture.repo.CreateTask(ctx, &models.Task{
ID: "task-unrelated", WorkspaceID: fixture.workspaceID, WorkflowID: fixture.workflowID,
WorkflowStepID: fixture.stepID, Position: 2, Title: "Unrelated task", State: v1.TaskStateInProgress,
CreatedAt: now, UpdatedAt: now,
}))
require.NoError(t, fixture.repo.CreateTaskSession(ctx, &models.TaskSession{
ID: "session-unrelated", TaskID: "task-unrelated", IsPrimary: true,
State: models.TaskSessionStateRunning, StartedAt: now, UpdatedAt: now,
}))
require.NoError(t, queueRepo.SetPendingMove(ctx, "session-unrelated", &messagequeue.PendingMove{
MoveID: "unrelated-move", TaskID: "task-unrelated", WorkflowID: fixture.workflowID,
WorkflowStepID: "step-next", Position: 3, Actor: "agent", SenderSessionID: "sender-session",
EntryOptions: &workflowmove.EntryOptions{Instructions: "retain this override"},
}))
beforeMoves, err := queueRepo.ListPendingMoves(ctx)
require.NoError(t, err)
require.Len(t, beforeMoves, 1)
beforeTransitions := transitionCount(t, fixture.db, fixture.taskID)
dispatcher := ws.NewDispatcher()
fixture.handler(t, queue).RegisterHandlers(dispatcher)
for range 47 {
response, err := dispatcher.Dispatch(ctx, fixture.message(t, nil))
require.NoError(t, err)
result := decodeMoveTaskResponse(t, response)
assert.Equal(t, moveDispositionApplied, result.Disposition)
}
afterMoves, err := queueRepo.ListPendingMoves(ctx)
require.NoError(t, err)
assert.Equal(t, beforeMoves, afterMoves)
currentMove, err := queueRepo.GetPendingMove(ctx, "session-running")
require.NoError(t, err)
assert.Nil(t, currentMove)
task, err := fixture.svc.GetTask(ctx, fixture.taskID)
require.NoError(t, err)
assert.Equal(t, fixture.stepID, task.WorkflowStepID)
assert.Equal(t, 7, task.Position)
assert.Equal(t, beforeTransitions, transitionCount(t, fixture.db, fixture.taskID))
}
// @covers AC-TASKS-MCP-MOVE-RESULTS-001.4
func TestHandleMoveTask_SameStepValidation(t *testing.T) {
type testCase struct {
name string
fields map[string]interface{}
prepare func(*testing.T, *sameStepMoveFixture) context.Context
withoutTask bool
withoutTarget bool
wantCode string
}
cases := []testCase{
{name: "task read failure", fields: map[string]interface{}{"task_id": "missing-task"}, wantCode: ws.ErrorCodeInternalError},
{name: "task write denied", prepare: func(t *testing.T, f *sameStepMoveFixture) context.Context {
require.NoError(t, f.repo.UpsertWorkspaceMember(context.Background(), &models.WorkspaceMember{
WorkspaceID: f.workspaceID, UserID: "viewer", Role: string(authz.WorkspaceRoleViewer),
}))
return authn.WithIdentity(context.Background(), authn.Identity{UserID: "viewer", Role: authn.RoleMember})
}, wantCode: ws.ErrorCodeForbidden},
{name: "task outside caller scope", prepare: func(*testing.T, *sameStepMoveFixture) context.Context {
return authn.WithIdentity(context.Background(), authn.Identity{UserID: "outsider", Role: authn.RoleMember})
}, wantCode: ws.ErrorCodeInternalError},
{name: "archived task", prepare: func(t *testing.T, f *sameStepMoveFixture) context.Context {
require.NoError(t, f.repo.ArchiveTask(context.Background(), f.taskID))
return context.Background()
}, wantCode: ws.ErrorCodeConflict},
{name: "missing step", prepare: func(t *testing.T, f *sameStepMoveFixture) context.Context {
require.NoError(t, f.workflowRepo.DeleteStep(context.Background(), f.stepID))
return context.Background()
}, wantCode: ws.ErrorCodeValidation},
{name: "step belongs to another workflow", prepare: func(t *testing.T, f *sameStepMoveFixture) context.Context {
ctx := context.Background()
require.NoError(t, f.repo.CreateWorkflow(ctx, &models.Workflow{
ID: "wf-other", WorkspaceID: f.workspaceID, Name: "Other", CreatedAt: time.Now().UTC(), UpdatedAt: time.Now().UTC(),
}))
require.NoError(t, f.workflowRepo.DeleteStep(ctx, f.stepID))
require.NoError(t, f.workflowRepo.CreateStep(ctx, &workflowmodels.WorkflowStep{
ID: f.stepID, WorkflowID: "wf-other", Name: "Foreign step", Position: 0,
CreatedAt: time.Now().UTC(), UpdatedAt: time.Now().UTC(),
}))
return ctx
}, wantCode: ws.ErrorCodeValidation},
{name: "missing workflow", fields: map[string]interface{}{"workflow_id": "missing-workflow"}, prepare: func(t *testing.T, f *sameStepMoveFixture) context.Context {
_, err := f.db.Exec(f.db.Rebind(`UPDATE tasks SET workflow_id = ? WHERE id = ?`), "missing-workflow", f.taskID)
require.NoError(t, err)
return context.Background()
}, wantCode: ws.ErrorCodeValidation},
{name: "workflow lookup database failure", prepare: func(t *testing.T, f *sameStepMoveFixture) context.Context {
_, err := f.db.Exec(`ALTER TABLE workflows RENAME TO workflows_unavailable`)
require.NoError(t, err)
return context.Background()
}, wantCode: ws.ErrorCodeInternalError},
{name: "workflow belongs to another workspace", prepare: func(t *testing.T, f *sameStepMoveFixture) context.Context {
ctx := context.Background()
require.NoError(t, f.repo.CreateWorkspace(ctx, &models.Workspace{
ID: "ws-other", Name: "Other", CreatedAt: time.Now().UTC(), UpdatedAt: time.Now().UTC(),
}))
_, err := f.db.Exec(f.db.Rebind(`UPDATE workflows SET workspace_id = ? WHERE id = ?`), "ws-other", f.workflowID)
require.NoError(t, err)
return ctx
}, wantCode: ws.ErrorCodeValidation},
{name: "task service missing", withoutTask: true, wantCode: ws.ErrorCodeInternalError},
{name: "workflow validation missing", withoutTarget: true, wantCode: ws.ErrorCodeInternalError},
{name: "instructions require a step change", fields: map[string]interface{}{"entry_options": map[string]interface{}{"instructions": "handoff"}}, wantCode: ws.ErrorCodeValidation},
{name: "reset requires a step change", fields: map[string]interface{}{"entry_options": map[string]interface{}{"reset_context": true}}, wantCode: ws.ErrorCodeValidation},
{name: "skip prompt requires a step change", fields: map[string]interface{}{"entry_options": map[string]interface{}{"skip_step_prompt": true}}, wantCode: ws.ErrorCodeValidation},
{name: "legacy prompt requires a step change", fields: map[string]interface{}{"prompt": "handoff"}, wantCode: ws.ErrorCodeValidation},
{name: "conflicting prompt aliases", fields: map[string]interface{}{
"prompt": "legacy", "entry_options": map[string]interface{}{"instructions": "nested"},
}, wantCode: ws.ErrorCodeValidation},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
fixture := newSameStepMoveFixture(t)
ctx := context.Background()
if tc.prepare != nil {
ctx = tc.prepare(t, fixture)
}
handler := fixture.handler(t, nil)
if tc.withoutTask {
handler.taskSvc = nil
}
if tc.withoutTarget {
handler.workflowCtrl = nil
}
response, err := handler.handleMoveTask(ctx, fixture.message(t, tc.fields))
require.NoError(t, err)
assertWSError(t, response, tc.wantCode)
if tc.wantCode == ws.ErrorCodeInternalError {
assert.NotContains(t, string(response.Payload), "no such table")
}
})
}
}
func TestHandleMoveTask_SameStepStepLookupDatabaseFailure(t *testing.T) {
fixture := newSameStepMoveFixture(t)
_, _, failedStepController, _, failedDB := newTestTaskServiceWithWorkflowDB(t)
_, err := failedDB.Exec(`ALTER TABLE workflow_steps RENAME TO workflow_steps_unavailable`)
require.NoError(t, err)
handler := fixture.handler(t, nil)
handler.workflowCtrl = failedStepController
response, err := handler.handleMoveTask(context.Background(), fixture.message(t, nil))
require.NoError(t, err)
assertWSError(t, response, ws.ErrorCodeInternalError)
assert.NotContains(t, string(response.Payload), "no such table")
}
@@ -124,6 +124,18 @@ func newTestTaskServiceWithWorkflow(t *testing.T) (*service.Service, *sqliterepo
// without disturbing the rest of the fixture's tables.
func newTestTaskServiceWithWorkflowDB(t *testing.T) (
*service.Service, *sqliterepo.Repository, *workflowcontroller.Controller, *workflowrepo.Repository, *sqlx.DB,
) {
svc, repo, workflowCtrl, workflowRepo, sqlxDB, _ := newTestTaskServiceWithWorkflowDBAndEventBus(t)
return svc, repo, workflowCtrl, workflowRepo, sqlxDB
}
func newTestTaskServiceWithWorkflowDBAndEventBus(t *testing.T) (
*service.Service,
*sqliterepo.Repository,
*workflowcontroller.Controller,
*workflowrepo.Repository,
*sqlx.DB,
*bus.MemoryEventBus,
) {
t.Helper()
dbConn, err := db.OpenSQLite(filepath.Join(t.TempDir(), "test.db"))
@@ -163,7 +175,7 @@ func newTestTaskServiceWithWorkflowDB(t *testing.T) (
svc.SetWorkspacePolicyAttacher(testWorkspacePolicyAttacher{})
workflowSvc := workflowservice.NewService(workflowRepo, log)
t.Cleanup(func() { _ = workflowSvc.Close() })
return svc, repo, workflowcontroller.NewController(workflowSvc), workflowRepo, sqlxDB
return svc, repo, workflowcontroller.NewController(workflowSvc), workflowRepo, sqlxDB, eventBus
}
func TestHandleListWorkspacesAutomationIsScopedToPrincipalWorkspace(t *testing.T) {
@@ -362,12 +362,12 @@ 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. 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.WithDescription(`Move a task to a different workflow step. A request naming the task's current workflow and step, without entry options, returns "applied" with the stored task because the destination already holds; no retry is needed. Only an actual step change is deferred to turn-end while the source session is RUNNING or STARTING. Idle-session and admin step changes apply immediately. Returns a move-result envelope: "disposition" is "applied" for a completed move or current-step no-op, or "deferred" when a step change was recorded for turn-end; "task" is the stored or moved task. An optioned step change 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("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.")),
mcp.WithNumber("position", mcp.Description("Requested position (0-based); the server determines arrival order, and a current-step request preserves the stored position.")),
mcp.WithString("prompt", mcp.Description("Legacy alias for entry_options.instructions. Optional hand-off message applied once for a new step. Supplying both this and entry_options.instructions is rejected; non-empty prompts are invalid for a current-step request.")),
moveTaskEntryOptionsToolOption(),
),
s.wrapHandler("move_task_kandev", s.moveTaskHandler()),
@@ -97,6 +97,49 @@ func TestMoveTaskToolSchemasExposeEntryOptions(t *testing.T) {
}
}
func TestMoveTaskSameStepDescriptionsAndResponseForwarding(t *testing.T) {
for name, backend := range map[string]*testBackend{
"task": {response: map[string]interface{}{"disposition": "applied", "task": map[string]interface{}{"id": "task-1", "workflow_step_id": "step-work", "position": 7}}},
"config": {response: map[string]interface{}{"disposition": "applied", "task": map[string]interface{}{"id": "task-1", "workflow_step_id": "step-work", "position": 7}}},
} {
var server *Server
if name == "task" {
server = newTaskModeServer(t, backend, "task-current")
} else {
server = newTestServer(t, backend)
}
tool := server.mcpServer.ListTools()["move_task_kandev"]
assert.Contains(t, tool.Tool.Description, "current workflow and step")
assert.Contains(t, tool.Tool.Description, "no retry is needed")
assert.Contains(t, tool.Tool.Description, "Only an actual step change")
position, ok := toolInputProperties(t, server, "move_task_kandev")["position"].(map[string]interface{})
require.True(t, ok)
assert.Contains(t, position["description"], "server determines arrival order")
assert.Contains(t, position["description"], "preserves the stored position")
result := callTool(t, server, "move_task_kandev", map[string]interface{}{
"task_id": "task-1", "workflow_id": "wf-1", "workflow_step_id": "step-work", "position": 99,
})
require.False(t, result.IsError)
require.Len(t, result.Content, 1)
content, ok := result.Content[0].(mcplib.TextContent)
require.True(t, ok)
var response map[string]interface{}
require.NoError(t, json.Unmarshal([]byte(content.Text), &response))
assert.Equal(t, "applied", response["disposition"])
task, ok := response["task"].(map[string]interface{})
require.True(t, ok)
assert.Equal(t, float64(7), task["position"])
assert.Equal(t, ws.ActionMCPMoveTask, backend.lastAction)
payload, ok := backend.lastPayload.(map[string]interface{})
require.True(t, ok)
assert.Equal(t, "task-1", payload["task_id"])
assert.Equal(t, "wf-1", payload["workflow_id"])
assert.Equal(t, "step-work", payload["workflow_step_id"])
}
}
// --- Action constant tests ---
func TestActionConstants_MatchWebSocketActions(t *testing.T) {
+3 -3
View File
@@ -1269,12 +1269,12 @@ 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. 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.WithDescription(`Move a task to a different workflow step. A request naming the task's current workflow and step, without entry options, returns "applied" with the stored task because the destination already holds; no retry is needed. Only an actual step change is deferred to turn-end while the source session is RUNNING or STARTING. Idle-session and admin step changes apply immediately. Returns a move-result envelope: "disposition" is "applied" for a completed move or current-step no-op, or "deferred" when a step change was recorded for turn-end; "task" is the stored or moved task. An optioned step change 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.WithNumber("position", mcp.Description("Requested position (0-based); the server determines arrival order, and a current-step request preserves the stored position.")),
mcp.WithString("prompt", mcp.Description("Optional hand-off message for the receiving agent at a new step. Only actual step changes are deferred during an active turn; non-empty prompts are invalid for a current-step request.")),
moveTaskEntryOptionsToolOption(),
),
s.wrapHandler("move_task_kandev", s.moveTaskHandler()),
@@ -9,6 +9,9 @@ var ErrWorkspaceNameMismatch = errors.New("workspace name mismatch")
// ErrWorkspaceNotFound reports that no workspace row matched the supplied id.
var ErrWorkspaceNotFound = errors.New("workspace not found")
// ErrWorkflowNotFound reports that no workflow row matched the supplied id.
var ErrWorkflowNotFound = errors.New("workflow not found")
// ErrTaskNotFound reports that no task row matched the supplied id.
var ErrTaskNotFound = errors.New("task not found")
@@ -12,6 +12,7 @@ import (
"github.com/kandev/kandev/internal/db/dialect"
"github.com/kandev/kandev/internal/steptelemetry"
"github.com/kandev/kandev/internal/task/models"
"github.com/kandev/kandev/internal/task/repository/repoerrors"
)
// AddTaskToWorkflow adds a task to a workflow with placement. Wrapped in a
@@ -242,7 +243,7 @@ func (r *Repository) GetWorkflow(ctx context.Context, id string) (*models.Workfl
FROM workflows WHERE id = ?
`, workflowSelectColumns)), id))
if err == sql.ErrNoRows {
return nil, fmt.Errorf("workflow not found: %s", id)
return nil, fmt.Errorf("%w: %s", repoerrors.ErrWorkflowNotFound, id)
}
if err != nil {
return nil, err
@@ -2,9 +2,11 @@ package repository
import (
"context"
"errors"
"testing"
"github.com/kandev/kandev/internal/task/models"
"github.com/kandev/kandev/internal/task/repository/repoerrors"
)
// Workflow CRUD tests
@@ -142,6 +144,8 @@ func TestSQLiteRepository_WorkflowNotFound(t *testing.T) {
_, err := repo.GetWorkflow(ctx, "nonexistent")
if err == nil {
t.Error("expected error for nonexistent workflow")
} else if !errors.Is(err, repoerrors.ErrWorkflowNotFound) {
t.Errorf("GetWorkflow error = %v, want ErrWorkflowNotFound", err)
}
err = repo.UpdateWorkflow(ctx, &models.Workflow{ID: "nonexistent", Name: "Test"})
@@ -8,6 +8,9 @@ import (
// ErrWIPLimitExceeded identifies a workflow-step capacity rejection.
var ErrWIPLimitExceeded = errors.New("workflow step WIP limit exceeded")
// ErrWorkflowStepNotFound reports that no workflow step row matched the supplied id.
var ErrWorkflowStepNotFound = errors.New("workflow step not found")
// WIPLimitError carries workflow-step capacity details while preserving
// errors.Is(err, ErrWIPLimitExceeded) classification at service boundaries.
type WIPLimitError struct {
@@ -770,7 +770,7 @@ func (r *Repository) GetStep(ctx context.Context, id string) (*models.WorkflowSt
step, err := r.scanStep(row)
if err == sql.ErrNoRows {
return nil, fmt.Errorf("workflow step not found: %s", id)
return nil, fmt.Errorf("%w: %s", models.ErrWorkflowStepNotFound, id)
}
if err != nil {
return nil, err
@@ -3,6 +3,7 @@ package repository
import (
"context"
"database/sql"
"errors"
"reflect"
"testing"
@@ -12,6 +13,14 @@ import (
"github.com/kandev/kandev/internal/workflow/models"
)
func TestGetMissingWorkflowStepReturnsTypedNotFound(t *testing.T) {
repo := setupTestRepo(t)
_, err := repo.GetStep(context.Background(), "missing-step")
if !errors.Is(err, models.ErrWorkflowStepNotFound) {
t.Fatalf("GetStep error = %v, want ErrWorkflowStepNotFound", err)
}
}
func setupTestRepo(t *testing.T) *Repository {
repo, _ := setupTestRepoWithDB(t)
return repo
@@ -9,6 +9,7 @@ import (
"github.com/kandev/kandev/internal/auth/authn"
taskmodels "github.com/kandev/kandev/internal/task/models"
"github.com/kandev/kandev/internal/task/repository/repoerrors"
workflowmodels "github.com/kandev/kandev/internal/workflow/models"
)
// Per-user scoping for the workflow-step surface (opt-in authentication).
@@ -205,9 +206,11 @@ func isNotFoundError(err error) bool {
return false
}
if errors.Is(err, ErrNotVisible) ||
errors.Is(err, repoerrors.ErrWorkflowNotFound) ||
errors.Is(err, repoerrors.ErrWorkspaceNotFound) ||
errors.Is(err, repoerrors.ErrTaskNotFound) ||
errors.Is(err, taskmodels.ErrTaskSessionNotFound) {
errors.Is(err, taskmodels.ErrTaskSessionNotFound) ||
errors.Is(err, workflowmodels.ErrWorkflowStepNotFound) {
return true
}
return strings.Contains(strings.ToLower(err.Error()), "not found")
@@ -262,10 +262,12 @@ func (o opaqueWrap) Unwrap() error { return o.err }
func TestIsNotFoundMatchesSentinelsWithoutRelyingOnWording(t *testing.T) {
for name, sentinel := range map[string]error{
"not visible": ErrNotVisible,
"workspace": repoerrors.ErrWorkspaceNotFound,
"task": repoerrors.ErrTaskNotFound,
"task session": taskmodels.ErrTaskSessionNotFound,
"not visible": ErrNotVisible,
"workflow": repoerrors.ErrWorkflowNotFound,
"workspace": repoerrors.ErrWorkspaceNotFound,
"task": repoerrors.ErrTaskNotFound,
"task session": taskmodels.ErrTaskSessionNotFound,
"workflow step": models.ErrWorkflowStepNotFound,
} {
t.Run(name, func(t *testing.T) {
require.True(t, IsNotFound(sentinel))
+142
View File
@@ -0,0 +1,142 @@
---
created: 2026-09-22
status: implemented
requirements:
- REQ-TASKS-MCP-MOVE-RESULTS-001
system_design:
- ../../specs/tasks/system-design/mcp-task-move-results.md
legacy_specs:
- ../../specs/workflow-step-move-overrides/spec.md
---
# Implementation plan: Same-step MCP move results
## Overview
[Issue #3872](https://github.com/kdlbs/kandev/issues/3872) reports repeated deferred moves to a task's current step.
One sequential work order adds a validated no-op result, regression coverage, and matching tool documentation.
Implementation and targeted verification are complete.
## Evidence and root cause
Investigation used commit `c4262c644d4` and the canonical issue. The issue contains no image attachments or comments.
The authenticated GitHub user is `carlosflorencio`; issue assignment succeeded on 2026-09-22.
`handleMoveTask` routes every RUNNING or STARTING primary session to `deferMoveTask`.
`deferMoveTask` validates entry options against step equality but does not complete an option-less equal-step request.
It calls `SetPendingMove`, creates a move identifier, and returns `deferred`.
At turn end, `applyPendingMove` recognizes the equal target and skips the transition.
A temporary `TestIssue3872SameStepRepro` used the existing envelope fixture with a SQLite task service and recording queue.
The task and requested destination both used `wf-envd` / `step-work`; the primary session was RUNNING.
No entry options were supplied. The expected result was `applied` with no pending move.
```text
go test ./internal/mcp/handlers -run '^TestIssue3872SameStepRepro$' -count=1
FAIL: expected "applied", actual "deferred"
FAIL: expected empty pendingMoves, actual one record targeting step-work
```
The reproduction ran from `apps/backend` and failed for both expected assertions.
The temporary test and generator were removed. No production or permanent test files changed.
Current orchestrator code includes equal-target record consumption and stale-move recovery.
The historical nine-row leak reported for v0.95.1 was not reproduced against a live database.
This package prevents new unnecessary records. It does not claim to repair every historical row.
## Requirements and assumptions
The task system owns this contract because it owns task placement and pending transitions.
Adjacent agent and plugin specifications do not own this MCP task result.
The existing AC-TASKS-KANBAN-TASK-REORDERING-001.28 already forbids position changes for a move to the current step.
The new [result requirement](../../specs/tasks/requirements/mcp-task-move-results.md) supplies the missing observable completion semantics.
The legacy move-overrides specification continues to own normalization and one-time entry behavior.
The selected result is `applied`, one of the issue's proposed solutions and an existing protocol value.
This avoids adding a new disposition. No material product question blocks this package.
Existing queued work is preserved because a destination no-op is not a cancellation request.
## Scope
### In scope
- Validated equal-workflow, equal-step completion before session routing.
- Stored task response, unchanged position, and no new queue or transition effects.
- Regression tests, both MCP tool descriptions, and public protocol documentation.
### Out of scope
- Historical database cleanup, cancellation semantics, and global queue redesign.
- Changes to real cross-step moves, HTTP moves, WIP admission, or rendered UI.
- New dispositions, migrations, runtime flags, and prompt-only retry workarounds.
## Technical approach
Follow [the system design](../../specs/tasks/system-design/mcp-task-move-results.md).
Add a small validated completion helper in `internal/mcp/handlers` before the session lookup.
Preserve existing write authorization through `AuthorizeWorkspaceScope` rather than relying on read permission alone.
Return the existing move envelope without calling either mutation path.
Update both tool registrations and `docs/public/agent-communication.md` in the implementation work order.
Public docs remain unchanged during this design turn because the runtime fix is not implemented.
Existing move-overrides plans remain historical records; their implementation results are not rerun or rewritten by this package.
## Tests
All new named tests below belong to `config_task_handlers_same_step_test.go` unless another path is given.
| Acceptance | Planned regression |
| --- | --- |
| AC-TASKS-MCP-MOVE-RESULTS-001.1 | `TestHandleMoveTask_SameStepApplied`: RUNNING, STARTING, idle, absent, and mixed sibling sessions |
| AC-TASKS-MCP-MOVE-RESULTS-001.2 | `TestHandleMoveTask_SameStepReturnsStoredTask`: omitted and differing requested positions |
| AC-TASKS-MCP-MOVE-RESULTS-001.3 | `TestHandleMoveTask_SameStepPreservesEffects`: 47 repeats, pending moves, prompts, metadata, session state, and event counts |
| AC-TASKS-MCP-MOVE-RESULTS-001.4 | `TestHandleMoveTask_SameStepValidation`: archive, access scope, missing targets, operational lookup failures, dependency failures, and entry options |
| AC-TASKS-MCP-MOVE-RESULTS-001.5 | Existing `TestHandleMoveTask` / `TestDeferMoveTask` cases and server `TestMoveTaskToolSchemasExposeEntryOptions` |
The first regression must fail on the current code before implementation.
Empty options normalize to no options. Non-empty instructions, reset, skip, and legacy prompt remain invalid for equal-step requests.
The existing `TestPendingMove_EqualTargetRecordsAppliedMoveID` protects downstream defense behavior.
## E2E tests
The changed interface is MCP, with no rendered UI change. No Playwright test is required.
Add `TestHandleMoveTask_SameStepPersistentQueue` with the real task service and queue repository.
Send 47 requests through the registered WebSocket MCP action and inspect responses, task state, and persisted pending rows.
For a seeded unrelated move, compare its identity and payload before and after the requests.
Add `TestMoveTaskSameStepResponseForwarding` in `internal/mcp/server/config_handlers_test.go` for both MCP modes.
Together these cover the public tool adapter, backend dispatch, and persistence boundaries.
## Work orders
- [x] [Task 01: Complete same-step MCP requests](task-01-complete-same-step-moves.md)
## Verification results
Design validation on 2026-09-22:
- `python3 scripts/list-docs.py validate`: passed (299 decisions, 1114 specifications).
- `python3 scripts/lint-spec-files.test.py`: passed (36 tests).
- `python3 scripts/lint-spec-files.py --all`: passed.
- `git diff --check -- docs/specs docs/plans/mcp-same-step-move`: passed.
- Catalog discovery includes both new specifications. All four package files were inspected before staging.
Implementation validation on 2026-09-23:
- Handler regressions, including the same-step matrix, passed. The new regression failed before the fix with `deferred` and one pending move, as expected.
- MCP handler, MCP server, workflow move, and orchestrator equal-target regression commands in the work order passed.
- `golangci-lint run ./internal/mcp/handlers ./internal/mcp/server`: passed with 0 issues.
- `go build -o /tmp/kandev-issue3872-final ./cmd/kandev` from `apps/backend`: passed.
- Public docs validation tests passed (62 tests); public docs validation passed (47 pages).
- `python3 scripts/list-docs.py validate`: passed (299 decisions, 1114 specifications).
- `python3 scripts/lint-spec-files.py --all`: passed.
- `git diff --check`: passed.
- PR review follow-up: missing and inaccessible targets retain validation errors; operational workflow or step lookup failures now return `internal_error` without exposing repository details.
- Handler, task repository, workflow repository, workflow service, MCP server, workflow move, and orchestrator regressions passed after the follow-up. Lint passed for the four changed Go packages, the backend binary built, and catalog/specification validation passed.
## Risks
- An early return can bypass authorization, target checks, or entry-option validation unless their tests cover it.
- Reusing the requested position in the response can misrepresent stored order.
- Removing pending work can cancel an unrelated accepted transition.
- A concurrent independent move can change the task after the no-op snapshot. This response does not reserve the destination.
@@ -0,0 +1,122 @@
---
id: "01-complete-same-step-moves"
title: "Complete same-step MCP requests"
status: done
wave: 1
depends_on: []
plan: "plan.md"
requirements:
- REQ-TASKS-MCP-MOVE-RESULTS-001
acceptance_criteria:
- AC-TASKS-MCP-MOVE-RESULTS-001.1
- AC-TASKS-MCP-MOVE-RESULTS-001.2
- AC-TASKS-MCP-MOVE-RESULTS-001.3
- AC-TASKS-MCP-MOVE-RESULTS-001.4
- AC-TASKS-MCP-MOVE-RESULTS-001.5
system_design:
- ../../specs/tasks/system-design/mcp-task-move-results.md
---
# Task 01: Complete same-step MCP requests
## Summary
Implement the validated no-op branch before session routing.
Prove that repeated calls return completion without changing task state or queued work.
## In scope
- The handler helper and regression matrix in the plan.
- Both tool registrations, response forwarding tests, and public reference wording.
- A persistent-queue integration case with repeated MCP backend dispatch.
## Out of scope
- Production database cleanup, queue cancellation, and orchestrator rewrites.
- HTTP, plugin, frontend, or position-reorder behavior changes.
## Acceptance
1. The named handler regressions fail before the correction and pass after it, including authorization, mixed-session, and operational target lookup cases.
2. A valid no-op preserves the complete stored task, existing queue state, and all transition effects across 47 requests.
3. Both tool modes expose the same result semantics, and all commands below pass.
## Implementation sequence
1. Read the requirement, design, scoped backend guidance, and existing envelope and validation fixtures.
2. Add the same-step regression first. Record its expected failure.
3. Implement the small handler helper described in the design.
4. Add validation, preservation, and persistent-queue coverage from the plan's test matrix.
5. Update tool descriptions and public reference text. Preserve the existing result envelope.
6. Run the commands below. Record results here and synchronize the plan.
The validation matrix includes nonexistent tasks, unauthorized tasks, read-only access, archived tasks, missing steps, and mismatched workflows.
It also includes cross-workspace targets, task-read failures, missing validation dependencies, and option normalization errors.
The positive cases include absent or unavailable queue support because a no-op does not use the queue.
## Verification
Run from the repository root:
```bash
(cd apps/backend && go test ./internal/mcp/handlers -run 'TestHandleMoveTask|TestDeferMoveTask|TestMoveTaskErrorMessage' -count=1)
(cd apps/backend && go test ./internal/mcp/server -run 'TestMoveTask' -count=1)
(cd apps/backend && go test ./internal/workflow/move -count=1)
(cd apps/backend && go test ./internal/orchestrator -run '^TestPendingMove_EqualTargetRecordsAppliedMoveID$' -count=1)
(cd apps/backend && go test ./internal/task/repository ./internal/workflow/repository ./internal/workflow/service -count=1)
(cd apps/backend && golangci-lint run ./internal/mcp/handlers ./internal/task/repository ./internal/workflow/repository ./internal/workflow/service)
(cd apps/backend && go build -o /tmp/kandev-issue3872-fixup ./cmd/kandev)
node --test scripts/validate-public-docs.test.mjs
node scripts/validate-public-docs.mjs
python3 scripts/list-docs.py validate
python3 scripts/lint-spec-files.py --all
git diff --check
```
## Files likely touched
- `apps/backend/internal/mcp/handlers/config_task_handlers.go`
- `apps/backend/internal/mcp/handlers/config_task_handlers_same_step_test.go` (new)
- `apps/backend/internal/mcp/handlers/handlers_test.go` (shared fixture)
- `apps/backend/internal/task/repository/repoerrors/errors.go`
- `apps/backend/internal/task/repository/sqlite/workflow.go`
- `apps/backend/internal/task/repository/workflow_repository_test.go`
- `apps/backend/internal/workflow/models/errors.go`
- `apps/backend/internal/workflow/repository/sqlite.go`
- `apps/backend/internal/workflow/repository/sqlite_test.go`
- `apps/backend/internal/workflow/service/access.go`
- `apps/backend/internal/workflow/service/access_test.go`
- `apps/backend/internal/mcp/server/server.go`
- `apps/backend/internal/mcp/server/config_handlers.go`
- `apps/backend/internal/mcp/server/config_handlers_test.go`
- `docs/public/agent-communication.md`
## Dependencies
None.
## Risks
The current fixtures sometimes omit workflow controllers. Do not preserve that omission as a production validation bypass.
Existing task-write authorization must run even though this branch performs no write.
Existing queued work must survive the no-op unchanged.
## Parallelism
`sequential`
## Inputs
- [Requirement](../../specs/tasks/requirements/mcp-task-move-results.md)
- [System design](../../specs/tasks/system-design/mcp-task-move-results.md)
- [Plan and diagnostic evidence](plan.md)
- Existing `pendingMoveRecordingQueuer`, `newTestTaskService`, and `seedRunningTask` fixtures.
- Existing `TestHandleMoveTask_DeferredReturnsMoveResultEnvelope` and `TestPendingMove_EqualTargetRecordsAppliedMoveID`.
## Results
Implemented and verified on 2026-09-23. The regression was first confirmed red: the same-step request returned `deferred`, included a move ID, and recorded one pending move. After the fix, the handler matrix, 47-request preservation cases, persisted queue dispatch, response forwarding, and tool-description checks passed. The validated no-op returns the stored task and position without changing task, queue, session, prompt, metadata, event, or transition state.
All verification commands above passed. Additional checks passed: `golangci-lint run ./internal/mcp/handlers ./internal/mcp/server` (0 issues), backend binary build, public documentation validation (62 tests and 47 pages), catalog validation (299 decisions and 1114 specifications), specification lint, and `git diff --check`.
PR review follow-up: typed workflow and step not-found errors now preserve validation behavior, while operational repository/controller failures are logged and return `internal_error` without leaking database details. The new workflow and step failure regressions failed before the correction with `VALIDATION_ERROR` and pass with `INTERNAL_ERROR`. Handler, MCP server, task repository, workflow repository/service, workflow move, and orchestrator tests passed. Changed-package lint reported 0 issues; the backend binary build, catalog validation, specification lint, and diff check passed.
+3 -1
View File
@@ -237,7 +237,9 @@ Task B processes each incoming message as a normal turn. When it receives Agent
}
```
The server normalizes this object once and omits empty optional strings. Reset is additive with the destination reset policy, and instructions are appended once after the normal destination prompt. When `skip_step_prompt` is set, the destination step's configured prompt and its task-description fallback are suppressed for this entry: with instructions the agent starts a turn carrying only those instructions, and without instructions no turn starts and the task lands idle. The destination and options are validated together. Every call returns a move-result envelope: `disposition` is `"deferred"` when the current agent is running (the options persist through the turn boundary, WIP queue promotion, and backend restart before they are applied) or `"applied"` when an idle move committed immediately, `task` is the moved (or target-step) task, and an optioned move also returns a `move_id` plus the accepted `entry_options` so you can correlate the retained one-shot override with the eventual step entry. The legacy top-level `prompt` argument remains accepted as an alias for `entry_options.instructions`; when both are non-empty, validation fails. Pull-request draft/readiness is not a generic move option.
The server normalizes this object once and omits empty optional strings. Reset is additive with the destination reset policy, and instructions are appended once after the normal destination prompt. When `skip_step_prompt` is set, the destination step's configured prompt and its task-description fallback are suppressed for this entry: with instructions the agent starts a turn carrying only those instructions, and without instructions no turn starts and the task lands idle. The destination and options are validated together.
Every call returns a move-result envelope. `disposition` is `"deferred"` when the source session is RUNNING or STARTING and an actual step change is recorded for turn-end. It is `"applied"` when a step change commits immediately or when the task already has the requested workflow and step and no entry options were supplied. In the same-step case, `task` is the stored task and no retry is needed. An optioned step change also returns a `move_id` plus the accepted `entry_options` so you can correlate the retained one-shot override with the eventual step entry. The legacy top-level `prompt` argument remains accepted as an alias for `entry_options.instructions`; when both are non-empty, validation fails. The server determines task position when it enters a step, and a same-step request preserves its stored position. Pull-request draft/readiness is not a generic move option.
**No secrets or large dumps.** Messages are coordination, not a code-delivery channel. Do not send credentials, private keys, or large file contents through cross-task messages. Reference files by path; share access via the repository, not the message.
+1 -1
View File
@@ -226,7 +226,7 @@ Creation and immediate launch are not one rollback boundary. If the task is crea
Task states on the wire are `TODO`, `CREATED`, `SCHEDULING`, `IN_PROGRESS`, `REVIEW`, `BLOCKED`, `WAITING_FOR_INPUT`, `COMPLETED`, `FAILED`, and `CANCELLED`. Use `task.move` to change the workflow step and `task.state` to change runtime state; these are separate operations.
`task.move` accepts an optional one-shot `entry_options` object. Its normalized fields are `reset_context`, `instructions`, and `skip_step_prompt`; empty optional strings are omitted. Reset is additive, and instructions are appended after the destination step's normal prompt. When `skip_step_prompt` is set, the destination step's prompt and its task-description fallback are suppressed for this entry: with instructions the agent starts a turn carrying only those instructions, and without instructions no turn starts and the task lands idle. The values do not mutate workflow defaults. A successful response includes, when supplied, the normalized `entry_options`, plus a `move_id` correlating options retained for a deferred move. The same options are accepted by `move_task_kandev`; its legacy top-level `prompt` is an alias for `entry_options.instructions`, and conflicting non-empty values are rejected. Moves requested by an active agent use the deferred MCP path and persist the complete options through turn completion, WIP promotion, and backend restart before the retained move is applied.
`task.move` accepts an optional one-shot `entry_options` object. Its normalized fields are `reset_context`, `instructions`, and `skip_step_prompt`; empty optional strings are omitted. Reset is additive, and instructions are appended after the destination step's normal prompt. When `skip_step_prompt` is set, the destination step's prompt and its task-description fallback are suppressed for this entry: with instructions the agent starts a turn carrying only those instructions, and without instructions no turn starts and the task lands idle. The values do not mutate workflow defaults. A successful response includes, when supplied, the normalized `entry_options`, plus a `move_id` correlating options retained for a deferred move. The same options are accepted by `move_task_kandev`; its legacy top-level `prompt` is an alias for `entry_options.instructions`, and conflicting non-empty values are rejected. Actual workflow-step changes requested through `move_task_kandev` from a RUNNING or STARTING session use the deferred MCP path and persist their complete options through turn completion, WIP promotion, and backend restart. A valid same-step MCP request without entry options returns `disposition: "applied"` with the stored task, so the caller does not need to retry.
```json
{
@@ -0,0 +1,52 @@
---
status: draft
system: tasks
created: 2026-09-22
owners:
- kandev
---
# MCP task move results
## Overview
Agents need to distinguish a completed destination request from a move that waits for turn end.
The task system owns this result because it owns workflow placement and deferred moves.
The existing [ordering requirement](kanban-task-reordering.md),
AC-TASKS-KANBAN-TASK-REORDERING-001.28, already preserves position for moves to the current step.
This document defines the MCP result for that case. It does not replace the ordering contract.
## Requirements
### REQ-TASKS-MCP-MOVE-RESULTS-001: Completed destination requests
**Intent:** An agent can recognize success without submitting the same destination request again.
#### Acceptance criteria
- **AC-TASKS-MCP-MOVE-RESULTS-001.1:** When a valid request names the current workflow and step without entry options, the system shall return `disposition: "applied"`.
This result shall apply with no session, an idle session, or any running or starting session.
- **AC-TASKS-MCP-MOVE-RESULTS-001.2:** The result shall contain the stored task, including its actual position.
The system shall not create a move identifier or report a new workflow entry for this no-op.
- **AC-TASKS-MCP-MOVE-RESULTS-001.3:** The no-op shall leave task state, position, metadata, session state, prompts, and pending moves unchanged.
It shall not run step actions or create transition history. Repeated requests shall have the same behavior.
- **AC-TASKS-MCP-MOVE-RESULTS-001.4:** The no-op shall retain task-write authorization, archive restrictions, and workflow and step validation.
Missing or inaccessible targets shall remain validation failures; repository and controller failures shall return an internal error without exposing implementation details.
Invalid requests shall fail without side effects. Non-empty normalized entry options shall remain invalid without a step change.
- **AC-TASKS-MCP-MOVE-RESULTS-001.5:** A valid request to another step shall retain the existing immediate or deferred behavior.
Both task-mode and configuration-mode tool descriptions shall explain that `applied` also means the task already occupies the requested destination.
The descriptions shall state that callers need no retry for that result.
## Compatibility and exclusions
The result reuses the existing `applied` value and response envelope. It adds no disposition value or request field.
Empty entry options retain their existing normalization. The legacy `prompt` remains an alias for entry instructions.
Existing queued work remains independently eligible. Success describes the destination at the time of the task read, not a reservation against later moves.
Historical pending-row repair, move cancellation, position reordering, HTTP behavior, and workflow recovery are outside this capability.
## Implementation plans
- [Same-step MCP move fix](../../../plans/mcp-same-step-move/plan.md)
@@ -0,0 +1,86 @@
---
status: draft
system: tasks
requirements:
- REQ-TASKS-MCP-MOVE-RESULTS-001
---
# MCP task move results
## Purpose and boundaries
The MCP backend handler owns destination-result classification before it selects immediate or deferred execution.
The task service remains the source of task data and authorization.
The queue and orchestrator retain ownership of real deferred transitions.
| Requirement | Design section |
| --- | --- |
| REQ-TASKS-MCP-MOVE-RESULTS-001 | Classification and response |
## Classification and response
`Handlers.handleMoveTask` in `internal/mcp/handlers/config_task_handlers.go` first parses required fields and normalizes entry options.
A small helper then reads the task through `taskSvc.GetTask` and compares both workflow and step identifiers.
For different destinations, normal routing continues. No active-session permission is broadened.
For an equal destination, the helper validates the candidate before returning success:
1. Enforce `authz.ScopeTaskWrite` through the existing `AuthorizeWorkspaceScope` service method.
2. Reject archived tasks.
3. Resolve the workflow and validate workspace membership through existing service accessors.
4. Resolve the step through `workflowCtrl.GetStep` and validate workflow membership.
5. Use `workflowmove.ValidateEntryOptions` with `MoveChangePositionOnly`.
Target lookups preserve typed not-found errors. A missing or inaccessible workflow or step remains a validation error; operational repository and controller failures are logged and return `internal_error` without exposing the underlying error to the MCP caller.
Production requires the task and workflow dependencies. A missing validation dependency must fail closed, without a fabricated success.
Handler fixtures for this branch must supply real workflow steps or the existing workflow controller fixture.
The helper returns `dto.MoveTaskResponse` with `dto.FromTask(task)` and `moveDispositionApplied`.
It leaves `MoveID`, `EntryOptions`, and `WorkflowEntryIdentity` empty.
It does not use `synthesizeMovedTaskDTO`, because that function can substitute requested values.
It completes before `lookupSession`, `deferMoveTask`, and `applyMoveTaskImmediate`.
Thus a valid no-op does not depend on queue availability or session liveness.
Task-mode registration in `internal/mcp/server/server.go` and configuration-mode registration in `config_handlers.go` describe the same result.
Their move and prompt descriptions must limit deferral language to actual step changes.
Their position descriptions must agree with the existing server-owned ordering contract.
`moveTaskHandler` continues forwarding the existing response envelope.
## Concurrency, persistence, and recovery
The no-op performs no writes and requires no schema change or queue admission.
Its success refers to the authorized task snapshot. A later independent transition can still change that task.
The handler must not call the generic move service for this branch: that path checks active sessions and performs writes.
The no-op neither replaces nor consumes an existing pending move, including one for another destination.
It does not cancel previously accepted work or erase entry markers.
`orchestrator.applyPendingMove` retains its equal-target guard for older records and destinations reached after admission.
That guard already records applied move identity and consumes the matching record through the applicable queue path.
This change does not add a database sweep or infer why an older deployment retained a particular row.
## Failure and security
Malformed payloads and conflicting prompt aliases fail during existing normalization.
Task read failures, target lookup failures, authorization failures, and archive restrictions must not become `applied` responses.
Missing or inaccessible targets retain validation errors. Operational lookup failures return `internal_error`, and response messages must not expose database or controller details.
Responses must not reveal inaccessible task data.
Both workflow and step equality are required. Matching only the step identifier is insufficient.
## Validation and observability
Handler tests cover SQLite task state and a recording queue, with repeated requests and mixed session states.
A handler integration case uses the real message queue repository to prove that pending rows do not grow.
Existing deferred-move and immediate-move tests prove that actual transitions retain their behavior.
Tool tests cover both registrations and MCP response forwarding.
The no-op emits neither `pending move recorded` nor workflow transition events.
No new metric or logging stream is necessary.
## Related contracts
- [Kanban task ordering](kanban-task-reordering.md)
- [Workflow move overrides](../../workflow-step-move-overrides/spec.md)
- [Move override decision](../../../decisions/2026-08-13-workflow-move-overrides.md)
- [Implementation plan](../../../plans/mcp-same-step-move/plan.md)