refactor(lunatask): genericize task API client

Amolith created

- Add typed errors (ErrNotFound, ErrUnauthorized, etc.) with APIError
  wrapper
- Use pointer types for nullable Task fields; add Name/Note for future
  E2EE support
- Add source_id to CreateTaskRequest for duplicate detection
- Create separate UpdateTaskRequest with all-optional fields
- Unify response types to return full Task object
- Add ListTasks and GetTask methods
- Move MCP-specific validation (name length, RFC3339, enums) to tools
  layer
- Remove validator dependency from client package

Assisted-by: Claude Sonnet 4 via Crush

Change summary

lunatask/tasks.go | 399 ++++++++++++++++++++++++++----------------------
tools/tasks.go    | 198 +++++++++++++++---------
2 files changed, 339 insertions(+), 258 deletions(-)

Detailed changes

lunatask/tasks.go 🔗

@@ -12,252 +12,309 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"strings"
+	"net/url"
+)
 
-	"github.com/go-playground/validator/v10"
+// API error types for typed error handling
+var (
+	// ErrBadRequest indicates invalid, malformed, or missing parameters (400)
+	ErrBadRequest = errors.New("bad request")
+	// ErrUnauthorized indicates missing, wrong, or revoked access token (401)
+	ErrUnauthorized = errors.New("unauthorized")
+	// ErrPaymentRequired indicates a subscription is required (402)
+	ErrPaymentRequired = errors.New("subscription required")
+	// ErrNotFound indicates the specified entity could not be found (404)
+	ErrNotFound = errors.New("not found")
+	// ErrUnprocessableEntity indicates the provided entity is not valid (422)
+	ErrUnprocessableEntity = errors.New("unprocessable entity")
+	// ErrServerError indicates an internal server error (500)
+	ErrServerError = errors.New("server error")
+	// ErrServiceUnavailable indicates temporary maintenance (503)
+	ErrServiceUnavailable = errors.New("service unavailable")
+	// ErrTimeout indicates request timed out (524)
+	ErrTimeout = errors.New("request timed out")
 )
 
+// APIError wraps an API error with status code and response body
+type APIError struct {
+	StatusCode int
+	Body       string
+	Err        error
+}
+
+func (e *APIError) Error() string {
+	if e.Body != "" {
+		return fmt.Sprintf("%s (status %d): %s", e.Err.Error(), e.StatusCode, e.Body)
+	}
+	return fmt.Sprintf("%s (status %d)", e.Err.Error(), e.StatusCode)
+}
+
+func (e *APIError) Unwrap() error {
+	return e.Err
+}
+
+// newAPIError creates an APIError from an HTTP status code
+func newAPIError(statusCode int, body string) *APIError {
+	var err error
+	switch statusCode {
+	case http.StatusBadRequest:
+		err = ErrBadRequest
+	case http.StatusUnauthorized:
+		err = ErrUnauthorized
+	case http.StatusPaymentRequired:
+		err = ErrPaymentRequired
+	case http.StatusNotFound:
+		err = ErrNotFound
+	case http.StatusUnprocessableEntity:
+		err = ErrUnprocessableEntity
+	case http.StatusInternalServerError:
+		err = ErrServerError
+	case http.StatusServiceUnavailable:
+		err = ErrServiceUnavailable
+	case 524:
+		err = ErrTimeout
+	default:
+		err = fmt.Errorf("unexpected status %d", statusCode)
+	}
+	return &APIError{StatusCode: statusCode, Body: body, Err: err}
+}
+
 // Source represents a task source like GitHub or other integrations
 type Source struct {
 	Source   string `json:"source"`
 	SourceID string `json:"source_id"`
 }
 
-// Task is only ever used in responses
+// Task represents a task returned from the Lunatask API
 type Task struct {
-	ID             string   `json:"id,omitempty"`
-	AreaID         string   `json:"area_id,omitempty"`
-	GoalID         string   `json:"goal_id,omitempty"`
-	Status         string   `json:"status,omitempty"`
-	PreviousStatus string   `json:"previous_status,omitempty"`
-	Estimate       int      `json:"estimate,omitempty"`
-	Priority       int      `json:"priority,omitempty"`
-	Progress       int      `json:"progress,omitempty"`
-	Motivation     string   `json:"motivation,omitempty"`
-	Eisenhower     int      `json:"eisenhower,omitempty"`
-	Sources        []Source `json:"sources,omitempty"`
-	ScheduledOn    string   `json:"scheduled_on,omitempty"`
-	CompletedAt    string   `json:"completed_at,omitempty"`
-	CreatedAt      string   `json:"created_at,omitempty"`
-	UpdatedAt      string   `json:"updated_at,omitempty"`
+	ID             string   `json:"id"`
+	AreaID         *string  `json:"area_id"`
+	GoalID         *string  `json:"goal_id"`
+	Name           *string  `json:"name"`
+	Note           *string  `json:"note"`
+	Status         *string  `json:"status"`
+	PreviousStatus *string  `json:"previous_status"`
+	Estimate       *int     `json:"estimate"`
+	Priority       *int     `json:"priority"`
+	Progress       *int     `json:"progress"`
+	Motivation     *string  `json:"motivation"`
+	Eisenhower     *int     `json:"eisenhower"`
+	Sources        []Source `json:"sources"`
+	ScheduledOn    *string  `json:"scheduled_on"`
+	CompletedAt    *string  `json:"completed_at"`
+	CreatedAt      string   `json:"created_at"`
+	UpdatedAt      string   `json:"updated_at"`
 }
 
 // CreateTaskRequest represents the request to create a task in Lunatask
 type CreateTaskRequest struct {
-	AreaID      string `json:"area_id,omitempty" validate:"omitempty,uuid4"`
-	GoalID      string `json:"goal_id,omitempty" validate:"omitempty,uuid4"` // Assuming GoalID remains optional for tasks
-	Name        string `json:"name" validate:"required,max=100"`
-	Note        string `json:"note,omitempty" validate:"omitempty"`
-	Status      string `json:"status,omitempty" validate:"omitempty,oneof=later next started waiting completed"`
-	Motivation  string `json:"motivation,omitempty" validate:"omitempty,oneof=must should want unknown"`
-	Estimate    int    `json:"estimate,omitempty" validate:"omitempty,min=0,max=720"`
-	Priority    int    `json:"priority,omitempty" validate:"omitempty,min=-2,max=2"`
-	Eisenhower  int    `json:"eisenhower,omitempty" validate:"omitempty,min=0,max=4"`
-	ScheduledOn string `json:"scheduled_on,omitempty" validate:"omitempty"`
-	CompletedAt string `json:"completed_at,omitempty" validate:"omitempty"`
-	Source      string `json:"source,omitempty" validate:"omitempty"`
+	Name        string  `json:"name"`
+	AreaID      *string `json:"area_id,omitempty"`
+	GoalID      *string `json:"goal_id,omitempty"`
+	Note        *string `json:"note,omitempty"`
+	Status      *string `json:"status,omitempty"`
+	Motivation  *string `json:"motivation,omitempty"`
+	Estimate    *int    `json:"estimate,omitempty"`
+	Priority    *int    `json:"priority,omitempty"`
+	Eisenhower  *int    `json:"eisenhower,omitempty"`
+	ScheduledOn *string `json:"scheduled_on,omitempty"`
+	CompletedAt *string `json:"completed_at,omitempty"`
+	Source      *string `json:"source,omitempty"`
+	SourceID    *string `json:"source_id,omitempty"`
 }
 
-// CreateTaskResponse represents the response from Lunatask API when creating a task
-type CreateTaskResponse struct {
-	Task struct {
-		ID string `json:"id"`
-	} `json:"task"`
+// UpdateTaskRequest represents the request to update a task in Lunatask.
+// All fields are optional; only provided fields will be updated.
+type UpdateTaskRequest struct {
+	Name        *string `json:"name,omitempty"`
+	AreaID      *string `json:"area_id,omitempty"`
+	GoalID      *string `json:"goal_id,omitempty"`
+	Note        *string `json:"note,omitempty"`
+	Status      *string `json:"status,omitempty"`
+	Motivation  *string `json:"motivation,omitempty"`
+	Estimate    *int    `json:"estimate,omitempty"`
+	Priority    *int    `json:"priority,omitempty"`
+	Eisenhower  *int    `json:"eisenhower,omitempty"`
+	ScheduledOn *string `json:"scheduled_on,omitempty"`
+	CompletedAt *string `json:"completed_at,omitempty"`
 }
 
-// UpdateTaskResponse represents the response from Lunatask API when updating a task
-type UpdateTaskResponse struct {
+// TaskResponse represents a single task response from the API
+type TaskResponse struct {
 	Task Task `json:"task"`
 }
 
-// DeleteTaskResponse represents the response from Lunatask API when deleting a task
-type DeleteTaskResponse struct {
-	Task Task `json:"task"`
+// TasksResponse represents a list of tasks response from the API
+type TasksResponse struct {
+	Tasks []Task `json:"tasks"`
 }
 
-// ValidationError represents errors returned by the validator
-type ValidationError struct {
-	Field   string
-	Tag     string
-	Message string
-}
+// doRequest performs an HTTP request and handles common response processing
+func (c *Client) doRequest(req *http.Request) ([]byte, error) {
+	req.Header.Set("Authorization", "bearer "+c.AccessToken)
+
+	resp, err := c.HTTPClient.Do(req)
+	if err != nil {
+		return nil, fmt.Errorf("failed to send HTTP request: %w", err)
+	}
+	defer func() { _ = resp.Body.Close() }()
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read response body: %w", err)
+	}
+
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		return nil, newAPIError(resp.StatusCode, string(body))
+	}
 
-// Error implements the error interface for ValidationError
-func (e ValidationError) Error() string {
-	return e.Message
+	return body, nil
 }
 
-// ValidateTask validates the create task request
-func ValidateTask(task *CreateTaskRequest) error {
-	validate := validator.New()
-	if err := validate.Struct(task); err != nil {
-		var invalidValidationError *validator.InvalidValidationError
-		if errors.As(err, &invalidValidationError) {
-			return fmt.Errorf("invalid validation error: %w", err)
-		}
+// ListTasks retrieves all tasks, optionally filtered by source
+func (c *Client) ListTasks(ctx context.Context, source *string) ([]Task, error) {
+	u := fmt.Sprintf("%s/tasks", c.BaseURL)
+	if source != nil && *source != "" {
+		u = fmt.Sprintf("%s?source=%s", u, url.QueryEscape(*source))
+	}
 
-		var validationErrs validator.ValidationErrors
-		if errors.As(err, &validationErrs) {
-			var msgBuilder strings.Builder
-			msgBuilder.WriteString("task validation failed:")
-			for _, e := range validationErrs {
-				fmt.Fprintf(&msgBuilder, " field '%s' failed '%s' validation (was: %v);", e.Field(), e.Tag(), e.Value())
-			}
-			return errors.New(msgBuilder.String())
-		}
-		return fmt.Errorf("validation error: %w", err)
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create HTTP request: %w", err)
 	}
-	return nil
+
+	body, err := c.doRequest(req)
+	if err != nil {
+		return nil, err
+	}
+
+	var response TasksResponse
+	if err := json.Unmarshal(body, &response); err != nil {
+		return nil, fmt.Errorf("failed to parse response: %w", err)
+	}
+
+	return response.Tasks, nil
 }
 
-// CreateTask creates a new task in Lunatask
-func (c *Client) CreateTask(ctx context.Context, task *CreateTaskRequest) (*CreateTaskResponse, error) {
-	// Validate the task
-	if err := ValidateTask(task); err != nil {
+// GetTask retrieves a specific task by ID
+func (c *Client) GetTask(ctx context.Context, taskID string) (*Task, error) {
+	if taskID == "" {
+		return nil, fmt.Errorf("%w: task ID cannot be empty", ErrBadRequest)
+	}
+
+	req, err := http.NewRequestWithContext(
+		ctx,
+		http.MethodGet,
+		fmt.Sprintf("%s/tasks/%s", c.BaseURL, taskID),
+		nil,
+	)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create HTTP request: %w", err)
+	}
+
+	body, err := c.doRequest(req)
+	if err != nil {
 		return nil, err
 	}
 
-	// Marshal the task to JSON
+	var response TaskResponse
+	if err := json.Unmarshal(body, &response); err != nil {
+		return nil, fmt.Errorf("failed to parse response: %w", err)
+	}
+
+	return &response.Task, nil
+}
+
+// CreateTask creates a new task in Lunatask.
+// Returns nil, nil if a matching task already exists (HTTP 204).
+func (c *Client) CreateTask(ctx context.Context, task *CreateTaskRequest) (*Task, error) {
+	if task.Name == "" {
+		return nil, fmt.Errorf("%w: name is required", ErrBadRequest)
+	}
+
 	payloadBytes, err := json.Marshal(task)
 	if err != nil {
 		return nil, fmt.Errorf("failed to marshal payload: %w", err)
 	}
 
-	// Create the request
 	req, err := http.NewRequestWithContext(
 		ctx,
-		"POST",
+		http.MethodPost,
 		fmt.Sprintf("%s/tasks", c.BaseURL),
 		bytes.NewBuffer(payloadBytes),
 	)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create HTTP request: %w", err)
 	}
-
-	// Set headers
 	req.Header.Set("Content-Type", "application/json")
-	req.Header.Set("Authorization", "bearer "+c.AccessToken)
 
-	// Send the request
-	resp, err := c.HTTPClient.Do(req)
+	body, err := c.doRequest(req)
 	if err != nil {
-		return nil, fmt.Errorf("failed to send HTTP request: %w", err)
-	}
-	defer func() {
-		if resp.Body != nil {
-			if err := resp.Body.Close(); err != nil {
-				// We're in a defer, so we can only log the error
-				fmt.Printf("Error closing response body: %v\n", err)
-			}
+		// Check for 204 No Content (task already exists)
+		var apiErr *APIError
+		if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNoContent {
+			return nil, nil
 		}
-	}()
-
-	// Handle already exists (no content) case
-	if resp.StatusCode == http.StatusNoContent {
-		return nil, nil // Task already exists (not an error)
+		return nil, err
 	}
 
-	// Handle error status codes
-	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-		respBody, _ := io.ReadAll(resp.Body)
-		return nil, fmt.Errorf("API error (status %d): %s", resp.StatusCode, string(respBody))
+	// Handle empty body (204 case that slipped through)
+	if len(body) == 0 {
+		return nil, nil
 	}
 
-	// Read and parse the response
-	respBody, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return nil, fmt.Errorf("failed to read response body: %w", err)
-	}
-
-	var response CreateTaskResponse
-	err = json.Unmarshal(respBody, &response)
-	if err != nil {
+	var response TaskResponse
+	if err := json.Unmarshal(body, &response); err != nil {
 		return nil, fmt.Errorf("failed to parse response: %w", err)
 	}
 
-	return &response, nil
+	return &response.Task, nil
 }
 
 // UpdateTask updates an existing task in Lunatask
-func (c *Client) UpdateTask(ctx context.Context, taskID string, task *CreateTaskRequest) (*UpdateTaskResponse, error) {
+func (c *Client) UpdateTask(ctx context.Context, taskID string, task *UpdateTaskRequest) (*Task, error) {
 	if taskID == "" {
-		return nil, errors.New("task ID cannot be empty")
+		return nil, fmt.Errorf("%w: task ID cannot be empty", ErrBadRequest)
 	}
 
-	// Validate the task payload
-	// Note: ValidateTask checks fields like Name, Priority, Estimate, etc.
-	// It's assumed that the API handles partial updates correctly,
-	// especially for fields like Name or AreaID in CreateTaskRequest that lack `omitempty`.
-	if err := ValidateTask(task); err != nil {
-		return nil, err
-	}
-
-	// Marshal the task to JSON
 	payloadBytes, err := json.Marshal(task)
 	if err != nil {
 		return nil, fmt.Errorf("failed to marshal payload: %w", err)
 	}
 
-	// Create the request
 	req, err := http.NewRequestWithContext(
 		ctx,
-		"PUT",
+		http.MethodPut,
 		fmt.Sprintf("%s/tasks/%s", c.BaseURL, taskID),
 		bytes.NewBuffer(payloadBytes),
 	)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create HTTP request: %w", err)
 	}
-
-	// Set headers
 	req.Header.Set("Content-Type", "application/json")
-	req.Header.Set("Authorization", "bearer "+c.AccessToken)
 
-	// Send the request
-	resp, err := c.HTTPClient.Do(req)
+	body, err := c.doRequest(req)
 	if err != nil {
-		return nil, fmt.Errorf("failed to send HTTP request: %w", err)
-	}
-	defer func() {
-		if resp.Body != nil {
-			if err := resp.Body.Close(); err != nil {
-				fmt.Printf("Error closing response body: %v\n", err)
-			}
-		}
-	}()
-
-	// Handle error status codes
-	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-		respBody, _ := io.ReadAll(resp.Body)
-		// Consider specific handling for 404 Not Found if needed
-		return nil, fmt.Errorf("API error (status %d): %s", resp.StatusCode, string(respBody))
-	}
-
-	// Read and parse the response
-	respBody, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return nil, fmt.Errorf("failed to read response body: %w", err)
+		return nil, err
 	}
 
-	var response UpdateTaskResponse
-	err = json.Unmarshal(respBody, &response)
-	if err != nil {
+	var response TaskResponse
+	if err := json.Unmarshal(body, &response); err != nil {
 		return nil, fmt.Errorf("failed to parse response: %w", err)
 	}
 
-	return &response, nil
+	return &response.Task, nil
 }
 
 // DeleteTask deletes a task in Lunatask
-func (c *Client) DeleteTask(ctx context.Context, taskID string) (*DeleteTaskResponse, error) {
+func (c *Client) DeleteTask(ctx context.Context, taskID string) (*Task, error) {
 	if taskID == "" {
-		return nil, errors.New("task ID cannot be empty")
+		return nil, fmt.Errorf("%w: task ID cannot be empty", ErrBadRequest)
 	}
 
-	// Create the request
 	req, err := http.NewRequestWithContext(
 		ctx,
-		"DELETE",
+		http.MethodDelete,
 		fmt.Sprintf("%s/tasks/%s", c.BaseURL, taskID),
 		nil,
 	)
@@ -265,39 +322,15 @@ func (c *Client) DeleteTask(ctx context.Context, taskID string) (*DeleteTaskResp
 		return nil, fmt.Errorf("failed to create HTTP request: %w", err)
 	}
 
-	// Set headers
-	req.Header.Set("Authorization", "bearer "+c.AccessToken)
-
-	// Send the request
-	resp, err := c.HTTPClient.Do(req)
-	if err != nil {
-		return nil, fmt.Errorf("failed to send HTTP request: %w", err)
-	}
-	defer func() {
-		if resp.Body != nil {
-			if err := resp.Body.Close(); err != nil {
-				fmt.Printf("Error closing response body: %v\n", err)
-			}
-		}
-	}()
-
-	// Handle error status codes
-	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-		respBody, _ := io.ReadAll(resp.Body)
-		return nil, fmt.Errorf("API error (status %d): %s", resp.StatusCode, string(respBody))
-	}
-
-	// Read and parse the response
-	respBody, err := io.ReadAll(resp.Body)
+	body, err := c.doRequest(req)
 	if err != nil {
-		return nil, fmt.Errorf("failed to read response body: %w", err)
+		return nil, err
 	}
 
-	var response DeleteTaskResponse
-	err = json.Unmarshal(respBody, &response)
-	if err != nil {
+	var response TaskResponse
+	if err := json.Unmarshal(body, &response); err != nil {
 		return nil, fmt.Errorf("failed to parse response: %w", err)
 	}
 
-	return &response, nil
+	return &response.Task, nil
 }

tools/tasks.go 🔗

@@ -6,7 +6,6 @@ package tools
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"strings"
 	"time"
@@ -39,10 +38,11 @@ func (h *Handlers) HandleCreateTask(ctx context.Context, request mcp.CallToolReq
 		return reportMCPError("Area not found for given area_id")
 	}
 
-	if goalID, exists := arguments["goal_id"].(string); exists && goalID != "" {
+	var goalID *string
+	if goalIDStr, exists := arguments["goal_id"].(string); exists && goalIDStr != "" {
 		found := false
 		for _, goal := range areaFoundProvider.GetGoals() {
-			if goal.GetID() == goalID {
+			if goal.GetID() == goalIDStr {
 				found = true
 				break
 			}
@@ -50,14 +50,25 @@ func (h *Handlers) HandleCreateTask(ctx context.Context, request mcp.CallToolReq
 		if !found {
 			return reportMCPError("Goal not found in specified area for given goal_id")
 		}
+		goalID = &goalIDStr
 	}
 
-	priorityMap := map[string]int{
-		"lowest":  -2,
-		"low":     -1,
-		"neutral": 0,
-		"high":    1,
-		"highest": 2,
+	name, ok := arguments["name"].(string)
+	if !ok || name == "" {
+		return reportMCPError("Missing or invalid required argument: name")
+	}
+	if len(name) > 100 {
+		return reportMCPError("'name' must be 100 characters or fewer")
+	}
+
+	task := lunatask.CreateTaskRequest{
+		Name:   name,
+		AreaID: &areaID,
+		GoalID: goalID,
+	}
+
+	if noteVal, exists := arguments["note"].(string); exists {
+		task.Note = &noteVal
 	}
 
 	if priorityArg, exists := arguments["priority"]; exists && priorityArg != nil {
@@ -65,19 +76,18 @@ func (h *Handlers) HandleCreateTask(ctx context.Context, request mcp.CallToolReq
 		if !ok {
 			return reportMCPError("Invalid type for 'priority' argument: expected string.")
 		}
+		priorityMap := map[string]int{
+			"lowest":  -2,
+			"low":     -1,
+			"neutral": 0,
+			"high":    1,
+			"highest": 2,
+		}
 		translatedPriority, isValid := priorityMap[strings.ToLower(priorityStr)]
 		if !isValid {
 			return reportMCPError(fmt.Sprintf("Invalid 'priority' value: '%s'. Must be one of 'lowest', 'low', 'neutral', 'high', 'highest'.", priorityStr))
 		}
-		arguments["priority"] = translatedPriority
-	}
-
-	eisenhowerMap := map[string]int{
-		"uncategorised":                0,
-		"both urgent and important":    1,
-		"urgent, but not important":    2,
-		"important, but not urgent":    3,
-		"neither urgent nor important": 4,
+		task.Priority = &translatedPriority
 	}
 
 	if eisenhowerArg, exists := arguments["eisenhower"]; exists && eisenhowerArg != nil {
@@ -85,59 +95,82 @@ func (h *Handlers) HandleCreateTask(ctx context.Context, request mcp.CallToolReq
 		if !ok {
 			return reportMCPError("Invalid type for 'eisenhower' argument: expected string.")
 		}
+		eisenhowerMap := map[string]int{
+			"uncategorised":                0,
+			"both urgent and important":    1,
+			"urgent, but not important":    2,
+			"important, but not urgent":    3,
+			"neither urgent nor important": 4,
+		}
 		translatedEisenhower, isValid := eisenhowerMap[strings.ToLower(eisenhowerStr)]
 		if !isValid {
 			return reportMCPError(fmt.Sprintf("Invalid 'eisenhower' value: '%s'. Must be one of 'uncategorised', 'both urgent and important', 'urgent, but not important', 'important, but not urgent', 'neither urgent nor important'.", eisenhowerStr))
 		}
-		arguments["eisenhower"] = translatedEisenhower
+		task.Eisenhower = &translatedEisenhower
 	}
 
 	if motivationVal, exists := arguments["motivation"]; exists && motivationVal != nil {
-		if motivation, ok := motivationVal.(string); ok && motivation != "" {
+		motivation, ok := motivationVal.(string)
+		if !ok {
+			return reportMCPError("'motivation' must be a string")
+		}
+		if motivation != "" {
 			validMotivations := map[string]bool{"must": true, "should": true, "want": true}
 			if !validMotivations[motivation] {
 				return reportMCPError("'motivation' must be one of 'must', 'should', or 'want'")
 			}
-		} else if ok {
-			// empty string is allowed
-		} else {
-			return reportMCPError("'motivation' must be a string")
+			task.Motivation = &motivation
 		}
 	}
 
 	if statusVal, exists := arguments["status"]; exists && statusVal != nil {
-		if status, ok := statusVal.(string); ok && status != "" {
+		status, ok := statusVal.(string)
+		if !ok {
+			return reportMCPError("'status' must be a string")
+		}
+		if status != "" {
 			validStatus := map[string]bool{"later": true, "next": true, "started": true, "waiting": true, "completed": true}
 			if !validStatus[status] {
 				return reportMCPError("'status' must be one of 'later', 'next', 'started', 'waiting', or 'completed'")
 			}
-		} else if ok {
-			// empty string is allowed
-		} else {
-			return reportMCPError("'status' must be a string")
+			task.Status = &status
+		}
+	}
+
+	if estimateArg, exists := arguments["estimate"]; exists && estimateArg != nil {
+		estimateVal, ok := estimateArg.(float64)
+		if !ok {
+			return reportMCPError("Invalid type for 'estimate' argument: expected number.")
+		}
+		estimate := int(estimateVal)
+		if estimate < 0 || estimate > 720 {
+			return reportMCPError("'estimate' must be between 0 and 720 minutes")
 		}
+		task.Estimate = &estimate
 	}
 
 	if scheduledOnArg, exists := arguments["scheduled_on"]; exists {
-		if scheduledOnStr, ok := scheduledOnArg.(string); ok && scheduledOnStr != "" {
+		scheduledOnStr, ok := scheduledOnArg.(string)
+		if !ok {
+			return reportMCPError("Invalid type for scheduled_on argument: expected string.")
+		}
+		if scheduledOnStr != "" {
 			if _, err := time.Parse(time.RFC3339, scheduledOnStr); err != nil {
 				return reportMCPError(fmt.Sprintf("Invalid format for scheduled_on: '%s'. Must be RFC3339 timestamp (e.g., YYYY-MM-DDTHH:MM:SSZ). Use get_timestamp tool first.", scheduledOnStr))
 			}
-		} else if !ok {
-			return reportMCPError("Invalid type for scheduled_on argument: expected string.")
+			task.ScheduledOn = &scheduledOnStr
 		}
 	}
 
-	client := lunatask.NewClient(h.config.AccessToken)
-	var task lunatask.CreateTaskRequest
-	argBytes, err := json.Marshal(arguments)
-	if err != nil {
-		return reportMCPError(fmt.Sprintf("Failed to process arguments: %v", err))
+	if sourceArg, exists := arguments["source"].(string); exists && sourceArg != "" {
+		task.Source = &sourceArg
 	}
-	if err := json.Unmarshal(argBytes, &task); err != nil {
-		return reportMCPError(fmt.Sprintf("Failed to parse arguments: %v", err))
+
+	if sourceIDArg, exists := arguments["source_id"].(string); exists && sourceIDArg != "" {
+		task.SourceID = &sourceIDArg
 	}
 
+	client := lunatask.NewClient(h.config.AccessToken)
 	response, err := client.CreateTask(ctx, &task)
 	if err != nil {
 		return reportMCPError(fmt.Sprintf("%v", err))
@@ -158,7 +191,7 @@ func (h *Handlers) HandleCreateTask(ctx context.Context, request mcp.CallToolReq
 		Content: []mcp.Content{
 			mcp.TextContent{
 				Type: "text",
-				Text: fmt.Sprintf("Task created successfully with ID: %s", response.Task.ID),
+				Text: fmt.Sprintf("Task created successfully with ID: %s", response.ID),
 			},
 		},
 	}, nil
@@ -177,14 +210,18 @@ func (h *Handlers) HandleUpdateTask(ctx context.Context, request mcp.CallToolReq
 		return reportMCPError(err.Error())
 	}
 
-	updatePayload := lunatask.CreateTaskRequest{}
+	updatePayload := lunatask.UpdateTaskRequest{}
 
 	var specifiedAreaProvider AreaProvider
 	areaIDProvided := false
 
 	if areaIDArg, exists := arguments["area_id"]; exists {
-		if areaIDStr, ok := areaIDArg.(string); ok && areaIDStr != "" {
-			updatePayload.AreaID = areaIDStr
+		areaIDStr, ok := areaIDArg.(string)
+		if !ok && areaIDArg != nil {
+			return reportMCPError("Invalid type for area_id argument: expected string.")
+		}
+		if ok && areaIDStr != "" {
+			updatePayload.AreaID = &areaIDStr
 			areaIDProvided = true
 			found := false
 			for _, ap := range h.config.Areas {
@@ -197,16 +234,16 @@ func (h *Handlers) HandleUpdateTask(ctx context.Context, request mcp.CallToolReq
 			if !found {
 				return reportMCPError(fmt.Sprintf("Area not found for given area_id: %s", areaIDStr))
 			}
-		} else if !ok && areaIDArg != nil {
-			return reportMCPError("Invalid type for area_id argument: expected string.")
 		}
-		// If area_id is not provided or is empty, we don't set it in the updatePayload
-		// This will leave the task in its current area
 	}
 
 	if goalIDArg, exists := arguments["goal_id"]; exists {
-		if goalIDStr, ok := goalIDArg.(string); ok && goalIDStr != "" {
-			updatePayload.GoalID = goalIDStr
+		goalIDStr, ok := goalIDArg.(string)
+		if !ok && goalIDArg != nil {
+			return reportMCPError("Invalid type for goal_id argument: expected string.")
+		}
+		if ok && goalIDStr != "" {
+			updatePayload.GoalID = &goalIDStr
 			if specifiedAreaProvider != nil {
 				foundGoal := false
 				for _, goal := range specifiedAreaProvider.GetGoals() {
@@ -221,34 +258,39 @@ func (h *Handlers) HandleUpdateTask(ctx context.Context, request mcp.CallToolReq
 			} else if areaIDProvided {
 				return reportMCPError("Internal error: area_id provided but area details not loaded for goal validation.")
 			}
-			// If area_id is not provided, we're not moving the task to a different area
-			// In this case, the goal validation should be skipped as we don't know the current area
-		} else if !ok && goalIDArg != nil {
-			return reportMCPError("Invalid type for goal_id argument: expected string.")
 		}
 	}
 
 	nameArg := arguments["name"]
 	if nameStr, ok := nameArg.(string); ok {
-		updatePayload.Name = nameStr
+		if len(nameStr) > 100 {
+			return reportMCPError("'name' must be 100 characters or fewer")
+		}
+		updatePayload.Name = &nameStr
 	} else {
 		return reportMCPError("Invalid type for name argument: expected string.")
 	}
 
 	if noteArg, exists := arguments["note"]; exists {
-		if noteStr, ok := noteArg.(string); ok {
-			updatePayload.Note = noteStr
-		} else if !ok && noteArg != nil {
+		noteStr, ok := noteArg.(string)
+		if !ok && noteArg != nil {
 			return reportMCPError("Invalid type for note argument: expected string.")
 		}
+		if ok {
+			updatePayload.Note = &noteStr
+		}
 	}
 
 	if estimateArg, exists := arguments["estimate"]; exists && estimateArg != nil {
-		if estimateVal, ok := estimateArg.(float64); ok {
-			updatePayload.Estimate = int(estimateVal)
-		} else {
+		estimateVal, ok := estimateArg.(float64)
+		if !ok {
 			return reportMCPError("Invalid type for estimate argument: expected number.")
 		}
+		estimate := int(estimateVal)
+		if estimate < 0 || estimate > 720 {
+			return reportMCPError("'estimate' must be between 0 and 720 minutes")
+		}
+		updatePayload.Estimate = &estimate
 	}
 
 	if priorityArg, exists := arguments["priority"]; exists && priorityArg != nil {
@@ -267,7 +309,7 @@ func (h *Handlers) HandleUpdateTask(ctx context.Context, request mcp.CallToolReq
 		if !isValid {
 			return reportMCPError(fmt.Sprintf("Invalid 'priority' value: '%s'. Must be one of 'lowest', 'low', 'neutral', 'high', 'highest'.", priorityStr))
 		}
-		updatePayload.Priority = translatedPriority
+		updatePayload.Priority = &translatedPriority
 	}
 
 	if eisenhowerArg, exists := arguments["eisenhower"]; exists && eisenhowerArg != nil {
@@ -286,47 +328,53 @@ func (h *Handlers) HandleUpdateTask(ctx context.Context, request mcp.CallToolReq
 		if !isValid {
 			return reportMCPError(fmt.Sprintf("Invalid 'eisenhower' value: '%s'. Must be one of 'uncategorised', 'both urgent and important', 'urgent, but not important', 'important, but not urgent', 'neither urgent nor important'.", eisenhowerStr))
 		}
-		updatePayload.Eisenhower = translatedEisenhower
+		updatePayload.Eisenhower = &translatedEisenhower
 	}
 
 	if motivationArg, exists := arguments["motivation"]; exists {
-		if motivationStr, ok := motivationArg.(string); ok {
+		motivationStr, ok := motivationArg.(string)
+		if !ok && motivationArg != nil {
+			return reportMCPError("Invalid type for motivation argument: expected string.")
+		}
+		if ok {
 			if motivationStr != "" {
 				validMotivations := map[string]bool{"must": true, "should": true, "want": true}
 				if !validMotivations[motivationStr] {
 					return reportMCPError("'motivation' must be one of 'must', 'should', or 'want', or empty to clear.")
 				}
 			}
-			updatePayload.Motivation = motivationStr
-		} else if !ok && motivationArg != nil {
-			return reportMCPError("Invalid type for motivation argument: expected string.")
+			updatePayload.Motivation = &motivationStr
 		}
 	}
 
 	if statusArg, exists := arguments["status"]; exists {
-		if statusStr, ok := statusArg.(string); ok {
+		statusStr, ok := statusArg.(string)
+		if !ok && statusArg != nil {
+			return reportMCPError("Invalid type for status argument: expected string.")
+		}
+		if ok {
 			if statusStr != "" {
 				validStatus := map[string]bool{"later": true, "next": true, "started": true, "waiting": true, "completed": true}
 				if !validStatus[statusStr] {
 					return reportMCPError("'status' must be one of 'later', 'next', 'started', 'waiting', 'completed', or empty.")
 				}
 			}
-			updatePayload.Status = statusStr
-		} else if !ok && statusArg != nil {
-			return reportMCPError("Invalid type for status argument: expected string.")
+			updatePayload.Status = &statusStr
 		}
 	}
 
 	if scheduledOnArg, exists := arguments["scheduled_on"]; exists {
-		if scheduledOnStr, ok := scheduledOnArg.(string); ok {
+		scheduledOnStr, ok := scheduledOnArg.(string)
+		if !ok && scheduledOnArg != nil {
+			return reportMCPError("Invalid type for scheduled_on argument: expected string.")
+		}
+		if ok {
 			if scheduledOnStr != "" {
 				if _, err := time.Parse(time.RFC3339, scheduledOnStr); err != nil {
 					return reportMCPError(fmt.Sprintf("Invalid format for scheduled_on: '%s'. Must be RFC3339. Use get_timestamp tool.", scheduledOnStr))
 				}
 			}
-			updatePayload.ScheduledOn = scheduledOnStr
-		} else if !ok && scheduledOnArg != nil {
-			return reportMCPError("Invalid type for scheduled_on argument: expected string.")
+			updatePayload.ScheduledOn = &scheduledOnStr
 		}
 	}
 
@@ -340,7 +388,7 @@ func (h *Handlers) HandleUpdateTask(ctx context.Context, request mcp.CallToolReq
 		Content: []mcp.Content{
 			mcp.TextContent{
 				Type: "text",
-				Text: fmt.Sprintf("Task updated successfully. ID: %s", response.Task.ID),
+				Text: fmt.Sprintf("Task updated successfully. ID: %s", response.ID),
 			},
 		},
 	}, nil