From 1eb3e4d19b7eb81c8246bf88c380683481a7fe8a Mon Sep 17 00:00:00 2001 From: Amolith Date: Thu, 8 Jan 2026 19:15:15 -0700 Subject: [PATCH] feat: add workflow-aware status validation When creating or updating tasks, status is now validated against the area's workflow. Error messages include valid options for that workflow: invalid status for workflow: 'foo' for Kanban; valid: later, next, ... MCP tools (create/update) validate when area context is available. The query tool's hardcoded status list is replaced with a dynamic one. CLI (task add/update) passes resolved area to status validation. When no area is specified, falls back to listing all valid statuses. Assisted-by: Claude Opus 4.5 via Crush --- cmd/task/add.go | 39 ++++++++++++++-------- cmd/task/update.go | 39 ++++++++++++++-------- internal/mcp/shared/errors.go | 54 +++++++++++++++++++++++++++++++ internal/mcp/tools/crud/create.go | 7 +++- internal/mcp/tools/crud/query.go | 2 +- internal/mcp/tools/crud/update.go | 27 ++++++++++++---- internal/validate/validate.go | 37 +++++++++++++++++++++ 7 files changed, 171 insertions(+), 34 deletions(-) diff --git a/cmd/task/add.go b/cmd/task/add.go index f0a9108069182c44a40e68645178924ea23668e8..f1c609e692ab5adbd98f443b1a47158c719a9dd4 100644 --- a/cmd/task/add.go +++ b/cmd/task/add.go @@ -73,11 +73,12 @@ func runAdd(cmd *cobra.Command, args []string) error { builder := apiClient.NewTask(name) - if err := applyAreaAndGoal(cmd, builder); err != nil { + area, err := applyAreaAndGoal(cmd, builder) + if err != nil { return err } - if err := applyOptionalFlags(cmd, builder); err != nil { + if err := applyOptionalFlags(cmd, builder, area); err != nil { return err } @@ -110,54 +111,66 @@ func resolveName(arg string) (string, error) { return "", ErrNoInput } -func applyAreaAndGoal(cmd *cobra.Command, builder *lunatask.TaskBuilder) error { +//nolint:nilnil // nil area with no error is valid when area flag not provided +func applyAreaAndGoal(cmd *cobra.Command, builder *lunatask.TaskBuilder) (*config.Area, error) { areaKey, _ := cmd.Flags().GetString("area") goalKey, _ := cmd.Flags().GetString("goal") if areaKey == "" && goalKey == "" { - return nil + return nil, nil } if areaKey == "" && goalKey != "" { fmt.Fprintln(cmd.ErrOrStderr(), ui.Warning.Render("Goal specified without area; ignoring")) - return nil + return nil, nil } cfg, err := config.Load() if err != nil { - return err + return nil, err } area := cfg.AreaByKey(areaKey) if area == nil { - return fmt.Errorf("%w: %s", ErrUnknownArea, areaKey) + return nil, fmt.Errorf("%w: %s", ErrUnknownArea, areaKey) } builder.InArea(area.ID) if goalKey == "" { - return nil + return area, nil } goal := area.GoalByKey(goalKey) if goal == nil { - return fmt.Errorf("%w: %s", ErrUnknownGoal, goalKey) + return nil, fmt.Errorf("%w: %s", ErrUnknownGoal, goalKey) } builder.InGoal(goal.ID) - return nil + return area, nil } -func applyOptionalFlags(cmd *cobra.Command, builder *lunatask.TaskBuilder) error { +//nolint:cyclop // flag handling inherently has many branches +func applyOptionalFlags(cmd *cobra.Command, builder *lunatask.TaskBuilder, area *config.Area) error { if status, _ := cmd.Flags().GetString("status"); status != "" { - s, err := validate.TaskStatus(status) + var ( + parsedStatus lunatask.TaskStatus + err error + ) + + if area != nil { + parsedStatus, err = validate.TaskStatusForWorkflow(status, area.Workflow) + } else { + parsedStatus, err = validate.TaskStatus(status) + } + if err != nil { return err } - builder.WithStatus(s) + builder.WithStatus(parsedStatus) } if note, _ := cmd.Flags().GetString("note"); note != "" { diff --git a/cmd/task/update.go b/cmd/task/update.go index 43e00d78ca5a0d825607673a82c926c50aa3f407..f9c767de35e780be653d995337249810476ed03e 100644 --- a/cmd/task/update.go +++ b/cmd/task/update.go @@ -68,11 +68,12 @@ func runUpdate(cmd *cobra.Command, args []string) error { return err } - if err := applyUpdateAreaAndGoal(cmd, builder); err != nil { + area, err := applyUpdateAreaAndGoal(cmd, builder) + if err != nil { return err } - if err := applyUpdateFlags(cmd, builder); err != nil { + if err := applyUpdateFlags(cmd, builder, area); err != nil { return err } @@ -104,54 +105,66 @@ func applyUpdateName(cmd *cobra.Command, builder *lunatask.TaskUpdateBuilder) er return nil } -func applyUpdateAreaAndGoal(cmd *cobra.Command, builder *lunatask.TaskUpdateBuilder) error { +//nolint:nilnil // nil area with no error is valid when area flag not provided +func applyUpdateAreaAndGoal(cmd *cobra.Command, builder *lunatask.TaskUpdateBuilder) (*config.Area, error) { areaKey, _ := cmd.Flags().GetString("area") goalKey, _ := cmd.Flags().GetString("goal") if areaKey == "" && goalKey == "" { - return nil + return nil, nil } if areaKey == "" && goalKey != "" { fmt.Fprintln(cmd.ErrOrStderr(), ui.Warning.Render("Goal specified without area; ignoring")) - return nil + return nil, nil } cfg, err := config.Load() if err != nil { - return err + return nil, err } area := cfg.AreaByKey(areaKey) if area == nil { - return fmt.Errorf("%w: %s", ErrUnknownArea, areaKey) + return nil, fmt.Errorf("%w: %s", ErrUnknownArea, areaKey) } builder.InArea(area.ID) if goalKey == "" { - return nil + return area, nil } goal := area.GoalByKey(goalKey) if goal == nil { - return fmt.Errorf("%w: %s", ErrUnknownGoal, goalKey) + return nil, fmt.Errorf("%w: %s", ErrUnknownGoal, goalKey) } builder.InGoal(goal.ID) - return nil + return area, nil } -func applyUpdateFlags(cmd *cobra.Command, builder *lunatask.TaskUpdateBuilder) error { +//nolint:cyclop // flag handling inherently has many branches +func applyUpdateFlags(cmd *cobra.Command, builder *lunatask.TaskUpdateBuilder, area *config.Area) error { if status, _ := cmd.Flags().GetString("status"); status != "" { - s, err := validate.TaskStatus(status) + var ( + parsedStatus lunatask.TaskStatus + err error + ) + + if area != nil { + parsedStatus, err = validate.TaskStatusForWorkflow(status, area.Workflow) + } else { + parsedStatus, err = validate.TaskStatus(status) + } + if err != nil { return err } - builder.WithStatus(s) + builder.WithStatus(parsedStatus) } if note, _ := cmd.Flags().GetString("note"); note != "" { diff --git a/internal/mcp/shared/errors.go b/internal/mcp/shared/errors.go index 211e13f64a41ffdd0523055889e13bb8b0a59af1..a87b82ea663fe024b2d19b73e3ccb8181acde666 100644 --- a/internal/mcp/shared/errors.go +++ b/internal/mcp/shared/errors.go @@ -7,7 +7,10 @@ package shared import ( "errors" "fmt" + "slices" + "strings" + "git.secluded.site/go-lunatask" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -32,6 +35,9 @@ const ( // ErrInvalidEstimate indicates the estimate is out of range. var ErrInvalidEstimate = errors.New("estimate must be between 0 and 720 minutes") +// ErrInvalidStatusForWorkflow indicates the status is not valid for the area's workflow. +var ErrInvalidStatusForWorkflow = errors.New("invalid status for workflow") + // ValidateEstimate checks that an estimate is within the valid range (0-720 minutes). func ValidateEstimate(estimate int) error { if estimate < MinEstimate || estimate > MaxEstimate { @@ -40,3 +46,51 @@ func ValidateEstimate(estimate int) error { return nil } + +// ValidateStatusForWorkflow parses a status string and validates it's allowed for +// the given workflow. Returns the parsed status and nil on success, or an error +// with valid options on failure. +func ValidateStatusForWorkflow( + input string, + workflow lunatask.Workflow, +) (lunatask.TaskStatus, error) { + status, err := lunatask.ParseTaskStatus(input) + if err != nil { + return "", fmt.Errorf( + "%w: '%s' for %s; valid: %s", + ErrInvalidStatusForWorkflow, input, workflow.Description(), formatValidStatuses(workflow), + ) + } + + if !slices.Contains(workflow.ValidStatuses(), status) { + return "", fmt.Errorf( + "%w: '%s' not valid for %s; valid: %s", + ErrInvalidStatusForWorkflow, input, workflow.Description(), formatValidStatuses(workflow), + ) + } + + return status, nil +} + +func formatValidStatuses(workflow lunatask.Workflow) string { + statuses := workflow.ValidStatuses() + strs := make([]string, len(statuses)) + + for i, s := range statuses { + strs[i] = string(s) + } + + return strings.Join(strs, ", ") +} + +// FormatAllStatuses returns a comma-separated list of all valid task statuses. +func FormatAllStatuses() string { + statuses := lunatask.AllTaskStatuses() + strs := make([]string, len(statuses)) + + for i, s := range statuses { + strs[i] = string(s) + } + + return strings.Join(strs, ", ") +} diff --git a/internal/mcp/tools/crud/create.go b/internal/mcp/tools/crud/create.go index b3540e723b64ec2d21333bb40fa00133b9fc9368..70c487c5119642acb9381411ad31e9880df07614 100644 --- a/internal/mcp/tools/crud/create.go +++ b/internal/mcp/tools/crud/create.go @@ -232,7 +232,12 @@ func (h *Handler) parseTaskCreateInput(input CreateInput) (*parsedTaskCreateInpu } if input.Status != nil { - status, err := lunatask.ParseTaskStatus(*input.Status) + area := h.cfg.AreaByID(parsed.AreaID) + if area == nil { + return nil, shared.ErrorResult("internal error: area not found after validation") + } + + status, err := shared.ValidateStatusForWorkflow(*input.Status, area.Workflow) if err != nil { return nil, shared.ErrorResult(err.Error()) } diff --git a/internal/mcp/tools/crud/query.go b/internal/mcp/tools/crud/query.go index 9a00c064283d71ed6bdf0649a430f9b7eb7daa77..fcc50a47a3f1158a36fecab0ab581df23da7f73c 100644 --- a/internal/mcp/tools/crud/query.go +++ b/internal/mcp/tools/crud/query.go @@ -215,7 +215,7 @@ func (h *Handler) listTasks( if input.Status != nil { if _, err := lunatask.ParseTaskStatus(*input.Status); err != nil { - return shared.ErrorResult("invalid status: must be later, next, in-progress, waiting, or completed"), + return shared.ErrorResult(fmt.Sprintf("invalid status '%s'; valid: %s", *input.Status, shared.FormatAllStatuses())), QueryOutput{Entity: EntityTask}, nil } } diff --git a/internal/mcp/tools/crud/update.go b/internal/mcp/tools/crud/update.go index c54f439c85cb22018915a17d4dd0a6e1a996ec10..574725c8395a8b28d4b181206f8494ff58e97fd7 100644 --- a/internal/mcp/tools/crud/update.go +++ b/internal/mcp/tools/crud/update.go @@ -6,6 +6,7 @@ package crud import ( "context" + "fmt" "git.secluded.site/go-lunatask" "git.secluded.site/lune/internal/dateutil" @@ -143,7 +144,7 @@ func (h *Handler) updateTask( }, UpdateOutput{Entity: input.Entity, DeepLink: deepLink}, nil } -//nolint:cyclop,funlen +//nolint:cyclop,funlen,gocognit,nestif func (h *Handler) parseTaskUpdateInput(input UpdateInput) (*parsedTaskUpdateInput, *mcp.CallToolResult) { _, id, err := lunatask.ParseReference(input.ID) if err != nil { @@ -189,12 +190,26 @@ func (h *Handler) parseTaskUpdateInput(input UpdateInput) (*parsedTaskUpdateInpu } if input.Status != nil { - status, err := lunatask.ParseTaskStatus(*input.Status) - if err != nil { - return nil, shared.ErrorResult(err.Error()) + if parsed.AreaID != nil { + area := h.cfg.AreaByID(*parsed.AreaID) + if area != nil { + status, err := shared.ValidateStatusForWorkflow(*input.Status, area.Workflow) + if err != nil { + return nil, shared.ErrorResult(err.Error()) + } + + parsed.Status = &status + } + } else { + status, err := lunatask.ParseTaskStatus(*input.Status) + if err != nil { + return nil, shared.ErrorResult( + fmt.Sprintf("invalid status '%s'; valid: %s", *input.Status, shared.FormatAllStatuses()), + ) + } + + parsed.Status = &status } - - parsed.Status = &status } if input.Priority != nil { diff --git a/internal/validate/validate.go b/internal/validate/validate.go index f06ebac1fd6e3fcc691f7077a2df47988e4e84ed..debc4acc3dd792d3c52c2c9aba6986e2f48bd706 100644 --- a/internal/validate/validate.go +++ b/internal/validate/validate.go @@ -8,6 +8,8 @@ package validate import ( "errors" "fmt" + "slices" + "strings" "git.secluded.site/go-lunatask" "git.secluded.site/lune/internal/config" @@ -32,6 +34,9 @@ func Reference(input string) (string, error) { // ErrInvalidStatus indicates the status value is not recognized. var ErrInvalidStatus = errors.New("invalid status") +// ErrInvalidStatusForWorkflow indicates the status is not valid for the area's workflow. +var ErrInvalidStatusForWorkflow = errors.New("invalid status for workflow") + // TaskStatus validates and normalizes a task status string. // Returns the corresponding lunatask.TaskStatus or an error if invalid. func TaskStatus(input string) (lunatask.TaskStatus, error) { @@ -43,6 +48,38 @@ func TaskStatus(input string) (lunatask.TaskStatus, error) { return status, nil } +// TaskStatusForWorkflow validates a status string against a specific workflow. +// Returns the status if valid for the workflow, or an error listing valid options. +func TaskStatusForWorkflow(input string, workflow lunatask.Workflow) (lunatask.TaskStatus, error) { + status, err := lunatask.ParseTaskStatus(input) + if err != nil { + return "", fmt.Errorf( + "%w: '%s' for %s; valid: %s", + ErrInvalidStatusForWorkflow, input, workflow.Description(), formatValidStatuses(workflow), + ) + } + + if !slices.Contains(workflow.ValidStatuses(), status) { + return "", fmt.Errorf( + "%w: '%s' not valid for %s; valid: %s", + ErrInvalidStatusForWorkflow, input, workflow.Description(), formatValidStatuses(workflow), + ) + } + + return status, nil +} + +func formatValidStatuses(workflow lunatask.Workflow) string { + statuses := workflow.ValidStatuses() + strs := make([]string, len(statuses)) + + for i, s := range statuses { + strs[i] = string(s) + } + + return strings.Join(strs, ", ") +} + // ErrInvalidMotivation indicates the motivation value is not recognized. var ErrInvalidMotivation = errors.New("invalid motivation")