fix(mcp): make archive_task_kandev idempotent when already archived (#2405)
Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -515,6 +515,45 @@ func TestHandleArchiveTask_InvalidPayload(t *testing.T) {
|
||||
assertWSError(t, resp, ws.ErrorCodeBadRequest)
|
||||
}
|
||||
|
||||
// TestHandleArchiveTask_AlreadyArchived_IsIdempotent pins that re-archiving a
|
||||
// task the caller already archived reports success rather than surfacing the
|
||||
// ErrTaskAlreadyArchived sentinel as an opaque INTERNAL ERROR. Archiving is a
|
||||
// goal-state operation, so the requested state already holding is not a
|
||||
// failure — but the response flags it so the caller can tell the two apart.
|
||||
func TestHandleArchiveTask_AlreadyArchived_IsIdempotent(t *testing.T) {
|
||||
svc, repo := newTestTaskService(t)
|
||||
ctx := context.Background()
|
||||
require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Test"}))
|
||||
require.NoError(t, repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-1", WorkspaceID: "ws-1", Name: "Board"}))
|
||||
task, err := svc.CreateTask(ctx, &service.CreateTaskRequest{
|
||||
WorkspaceID: "ws-1",
|
||||
WorkflowID: "wf-1",
|
||||
Title: "Archive me",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
h := &Handlers{taskSvc: svc, logger: testLogger(t).WithFields()}
|
||||
msg := makeWSMessage(t, ws.ActionMCPArchiveTask, map[string]string{"task_id": task.ID})
|
||||
|
||||
resp, err := h.handleArchiveTask(ctx, msg)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, resp)
|
||||
require.Equal(t, ws.MessageTypeResponse, resp.Type)
|
||||
var first map[string]interface{}
|
||||
require.NoError(t, json.Unmarshal(resp.Payload, &first))
|
||||
assert.Equal(t, true, first["success"])
|
||||
assert.NotContains(t, first, "already_archived", "first archive is a real state change")
|
||||
|
||||
resp2, err := h.handleArchiveTask(ctx, msg)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, resp2)
|
||||
require.Equal(t, ws.MessageTypeResponse, resp2.Type, "re-archiving must not surface as an error")
|
||||
var second map[string]interface{}
|
||||
require.NoError(t, json.Unmarshal(resp2.Payload, &second))
|
||||
assert.Equal(t, true, second["success"])
|
||||
assert.Equal(t, true, second["already_archived"])
|
||||
}
|
||||
|
||||
func TestHandleUpdateTaskState_MissingTaskID(t *testing.T) {
|
||||
h := &Handlers{}
|
||||
msg := makeWSMessage(t, ws.ActionMCPUpdateTaskState, map[string]interface{}{
|
||||
|
||||
@@ -11,6 +11,7 @@ import (
|
||||
"github.com/kandev/kandev/internal/task/dto"
|
||||
"github.com/kandev/kandev/internal/task/models"
|
||||
taskrepo "github.com/kandev/kandev/internal/task/repository/sqlite"
|
||||
"github.com/kandev/kandev/internal/task/service"
|
||||
v1 "github.com/kandev/kandev/pkg/api/v1"
|
||||
ws "github.com/kandev/kandev/pkg/websocket"
|
||||
"go.uber.org/zap"
|
||||
@@ -335,6 +336,16 @@ func (h *Handlers) handleArchiveTask(ctx context.Context, msg *ws.Message) (*ws.
|
||||
}
|
||||
|
||||
if err := h.taskSvc.ArchiveTask(ctx, taskID); err != nil {
|
||||
// Archiving is a goal-state operation: a task that is already archived
|
||||
// is in the requested state, so report success instead of an opaque
|
||||
// internal error. The flag lets the caller tell a no-op from a real
|
||||
// state change.
|
||||
if errors.Is(err, service.ErrTaskAlreadyArchived) {
|
||||
return ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{
|
||||
"success": true,
|
||||
"already_archived": true,
|
||||
})
|
||||
}
|
||||
h.logger.Error("failed to archive task", zap.Error(err))
|
||||
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "Failed to archive task", nil)
|
||||
}
|
||||
|
||||
@@ -694,7 +694,7 @@ func (s *Server) registerKanbanTools() {
|
||||
)
|
||||
s.mcpServer.AddTool(
|
||||
mcp.NewTool("archive_task_kandev",
|
||||
mcp.WithDescription("Archive a task. The task is hidden from active board views but kept in the database. Use to tidy up finished or abandoned tasks. Unarchiving is a user action done from the UI, not via MCP."),
|
||||
mcp.WithDescription("Archive a task. The task is hidden from active board views but kept in the database. Use to tidy up finished or abandoned tasks. Archiving an already-archived task is a no-op that succeeds with already_archived: true. Unarchiving is a user action done from the UI, not via MCP."),
|
||||
mcp.WithString("task_id", mcp.Required(), mcp.Description("The task ID to archive")),
|
||||
),
|
||||
s.wrapHandler("archive_task_kandev", s.archiveTaskHandler()),
|
||||
|
||||
Reference in New Issue
Block a user