refactor: deduplicate engine internals, replace bubble sorts, remove dead code

Extract shared code that was duplicated across functions:
- taskJSON struct (MarshalJSON/UnmarshalJSON) to package-level type
- scanTask(scanner) helper for GetTask/GetTasks (~70 identical lines)
- monthNames map for parseMonthName/parseDayAndMonth
- applyNonDateAttribute helper for Apply/ApplyToNew
- resolveDisplayID calls replace inline loops in FormatTaskListWithFormat

Replace O(n²) bubble sorts with sort.Slice in all four report sort
functions (sortByUrgency, NewestReport, NextReport, OldestReport).

Remove dead code: formatTimeWithColor (unused, also used time.Now()
instead of timeNow()), getCurrentTimestamp (unnecessary wrapper).

Remove ~20 comments that restated the next line of code.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

opal-task/cmd/sync.go

@@ -224,8 +224,8 @@ var syncUpCmd = &cobra.Command{
 		client := sync.NewClient(cfg.SyncURL, cfg.SyncAPIKey, cfg.SyncClientID)
 
 		// Get local changes
-		lastSync := getLastSyncTime(cfg.SyncClientID)
-		localChanges, err := getLocalChanges(lastSync)
+		lastSync := sync.GetLastSyncTime(cfg.SyncClientID)
+		localChanges, err := sync.GetLocalChanges(lastSync)
 		if err != nil {
 			fmt.Fprintf(os.Stderr, "Error getting local changes: %v\n", err)
 			os.Exit(1)
@@ -264,7 +264,7 @@ var syncDownCmd = &cobra.Command{
 		}
 
 		client := sync.NewClient(cfg.SyncURL, cfg.SyncAPIKey, cfg.SyncClientID)
-		lastSync := getLastSyncTime(cfg.SyncClientID)
+		lastSync := sync.GetLastSyncTime(cfg.SyncClientID)
 
 		changes, err := client.PullChanges(lastSync)
 		if err != nil {
@@ -277,7 +277,55 @@ var syncDownCmd = &cobra.Command{
 			return
 		}
 
-		fmt.Printf("✓ Pulled %d changes from server\n", len(changes))
+		// Parse changes into tasks
+		tasks, err := client.ParseChanges(changes)
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "Error parsing changes: %v\n", err)
+			os.Exit(1)
+		}
+
+		// Apply each task locally
+		var applied int
+		for _, task := range tasks {
+			if err := task.Save(); err != nil {
+				fmt.Fprintf(os.Stderr, "Warning: failed to save task %s: %v\n", task.UUID, err)
+				continue
+			}
+
+			// Mark as sync-originated to prevent feedback loop
+			_ = engine.MarkChangeLogAsSync(task.UUID.String())
+
+			// Sync tags
+			savedTask, err := engine.GetTask(task.UUID)
+			if err != nil {
+				fmt.Fprintf(os.Stderr, "Warning: failed to reload task %s: %v\n", task.UUID, err)
+				continue
+			}
+
+			currentTags, _ := savedTask.GetTags()
+			currentSet := make(map[string]bool)
+			for _, tag := range currentTags {
+				currentSet[tag] = true
+			}
+			desiredSet := make(map[string]bool)
+			for _, tag := range task.Tags {
+				desiredSet[tag] = true
+			}
+			for tag := range currentSet {
+				if !desiredSet[tag] {
+					savedTask.RemoveTag(tag)
+				}
+			}
+			for tag := range desiredSet {
+				if !currentSet[tag] {
+					savedTask.AddTag(tag)
+				}
+			}
+
+			applied++
+		}
+
+		fmt.Printf("✓ Pulled %d changes, applied %d tasks from server\n", len(changes), applied)
 	},
 }
 
@@ -414,63 +462,6 @@ func init() {
 	syncCmd.PersistentFlags().BoolVarP(&quietFlag, "quiet", "q", false, "Suppress progress output")
 }
 
-// Helper functions
-
-func getLastSyncTime(clientID string) int64 {
-	db := engine.GetDB()
-	if db == nil {
-		return 0
-	}
-
-	var lastSync int64
-	err := db.QueryRow("SELECT last_sync FROM sync_state WHERE client_id = ?", clientID).Scan(&lastSync)
-	if err != nil {
-		return 0
-	}
-
-	return lastSync
-}
-
-func getLocalChanges(since int64) ([]*engine.Task, error) {
-	db := engine.GetDB()
-	if db == nil {
-		return nil, fmt.Errorf("database not initialized")
-	}
-
-	rows, err := db.Query(`
-		SELECT DISTINCT task_uuid
-		FROM change_log
-		WHERE changed_at > ?
-		ORDER BY changed_at ASC
-	`, since)
-	if err != nil {
-		return nil, err
-	}
-	defer rows.Close()
-
-	var tasks []*engine.Task
-	for rows.Next() {
-		var uuidStr string
-		if err := rows.Scan(&uuidStr); err != nil {
-			continue
-		}
-
-		taskUUID, err := uuid.Parse(uuidStr)
-		if err != nil {
-			continue
-		}
-
-		task, err := engine.GetTask(taskUUID)
-		if err != nil {
-			continue
-		}
-
-		tasks = append(tasks, task)
-	}
-
-	return tasks, nil
-}
-
 func formatTimestamp(ts int64) string {
 	t := time.Unix(ts, 0)
 	now := time.Now()

opal-task/internal/api/handlers/sync.go

@@ -104,6 +104,8 @@ func PushChanges(w http.ResponseWriter, r *http.Request) {
 			if err := task.Save(); err != nil {
 				continue
 			}
+			// Mark as sync-originated to prevent feedback loop
+			_ = engine.MarkChangeLogAsSync(task.UUID.String())
 			// Add tags
 			for _, tag := range task.Tags {
 				_ = task.AddTag(tag)
@@ -114,15 +116,18 @@ func PushChanges(w http.ResponseWriter, r *http.Request) {
 
 		// Task exists - check timestamps for conflicts
 		if existing.Modified.Unix() > task.Modified.Unix() {
-			// Server version is newer - conflict (but we'll apply last-write-wins)
+			// Server version is newer - skip this push
 			conflicts++
+			continue
 		}
 
-		// Apply changes (last-write-wins)
+		// Apply changes (client is newer or equal)
 		task.ID = existing.ID // Preserve database ID
 		if err := task.Save(); err != nil {
 			continue
 		}
+		// Mark as sync-originated to prevent feedback loop
+		_ = engine.MarkChangeLogAsSync(task.UUID.String())
 
 		// Sync tags
 		existingTags := make(map[string]bool)

opal-task/internal/engine/database.go

@@ -307,6 +307,13 @@ func runMigrations() error {
 				END;
 			`,
 		},
+		{
+			version: 2,
+			sql: `
+				ALTER TABLE change_log ADD COLUMN source TEXT NOT NULL DEFAULT 'local';
+				CREATE INDEX idx_change_log_source ON change_log(source);
+			`,
+		},
 	}
 
 	// Apply pending migrations
@@ -327,7 +334,7 @@ func runMigrations() error {
 			if _, err := tx.Exec(
 				"INSERT INTO schema_version (version, applied_at) VALUES (?, ?)",
 				migration.version,
-				getCurrentTimestamp(),
+				GetCurrentTimestamp(),
 			); err != nil {
 				tx.Rollback()
 				return fmt.Errorf("failed to record migration %d: %w", migration.version, err)
@@ -342,14 +349,9 @@ func runMigrations() error {
 	return nil
 }
 
-// getCurrentTimestamp returns the current Unix timestamp
-func getCurrentTimestamp() int64 {
-	return timeNow().Unix()
-}
-
 // GetCurrentTimestamp returns the current Unix timestamp (exported for API use)
 func GetCurrentTimestamp() int64 {
-	return getCurrentTimestamp()
+	return timeNow().Unix()
 }
 
 // CleanupChangeLog removes old change log entries based on retention policy
@@ -409,3 +411,24 @@ func SetChangeLogRetentionDays(days int) error {
 	_, err := db.Exec("INSERT OR REPLACE INTO sync_config (key, value) VALUES ('change_log_retention_days', ?)", days)
 	return err
 }
+
+// MarkChangeLogAsSync marks the most recent change_log entry for a task UUID
+// as originating from sync (not local), preventing the feedback loop where
+// synced changes get re-pushed as local changes.
+func MarkChangeLogAsSync(taskUUID string) error {
+	db := GetDB()
+	if db == nil {
+		return fmt.Errorf("database not initialized")
+	}
+
+	_, err := db.Exec(`
+		UPDATE change_log SET source = 'sync'
+		WHERE id = (
+			SELECT id FROM change_log
+			WHERE task_uuid = ?
+			ORDER BY id DESC
+			LIMIT 1
+		)
+	`, taskUUID)
+	return err
+}

opal-task/internal/engine/dateparse.go

@@ -7,6 +7,21 @@ import (
 	"time"
 )
 
+var monthNames = map[string]time.Month{
+	"jan": time.January, "january": time.January,
+	"feb": time.February, "february": time.February,
+	"mar": time.March, "march": time.March,
+	"apr": time.April, "april": time.April,
+	"may": time.May,
+	"jun": time.June, "june": time.June,
+	"jul": time.July, "july": time.July,
+	"aug": time.August, "august": time.August,
+	"sep": time.September, "september": time.September,
+	"oct": time.October, "october": time.October,
+	"nov": time.November, "november": time.November,
+	"dec": time.December, "december": time.December,
+}
+
 // DateParser handles all date/time/duration parsing with configurable options
 type DateParser struct {
 	base      time.Time
@@ -238,22 +253,7 @@ func (p *DateParser) parseWeekday(s string) (time.Time, bool) {
 
 // parseMonthName handles month names (jan, january, feb, february, etc.)
 func (p *DateParser) parseMonthName(s string) (time.Time, bool) {
-	months := map[string]time.Month{
-		"jan": time.January, "january": time.January,
-		"feb": time.February, "february": time.February,
-		"mar": time.March, "march": time.March,
-		"apr": time.April, "april": time.April,
-		"may": time.May,
-		"jun": time.June, "june": time.June,
-		"jul": time.July, "july": time.July,
-		"aug": time.August, "august": time.August,
-		"sep": time.September, "september": time.September,
-		"oct": time.October, "october": time.October,
-		"nov": time.November, "november": time.November,
-		"dec": time.December, "december": time.December,
-	}
-
-	month, ok := months[s]
+	month, ok := monthNames[s]
 	if !ok {
 		return time.Time{}, false
 	}
@@ -316,22 +316,7 @@ func (p *DateParser) parseDayAndMonth(dayStr, monthStr string) (int, time.Month,
 		return 0, 0, false
 	}
 
-	months := map[string]time.Month{
-		"jan": time.January, "january": time.January,
-		"feb": time.February, "february": time.February,
-		"mar": time.March, "march": time.March,
-		"apr": time.April, "april": time.April,
-		"may": time.May,
-		"jun": time.June, "june": time.June,
-		"jul": time.July, "july": time.July,
-		"aug": time.August, "august": time.August,
-		"sep": time.September, "september": time.September,
-		"oct": time.October, "october": time.October,
-		"nov": time.November, "november": time.November,
-		"dec": time.December, "december": time.December,
-	}
-
-	month, ok := months[monthStr]
+	month, ok := monthNames[monthStr]
 	if !ok {
 		return 0, 0, false
 	}

opal-task/internal/engine/display.go

@@ -29,15 +29,9 @@ func FormatTaskListWithFormat(tasks []*Task, ws *WorkingSet, format string) stri
 	if format == "minimal" {
 		result := ""
 		for i, task := range tasks {
-			displayID := i + 1
-			if ws != nil {
-				// Use working set display ID if available
-				for id, uuid := range ws.byID {
-					if uuid == task.UUID {
-						displayID = id
-						break
-					}
-				}
+			displayID := resolveDisplayID(task, ws)
+			if displayID == 0 {
+				displayID = i + 1
 			}
 			urgency := task.CalculateUrgency(coeffs)
 			urgencyColor := getUrgencyColor(urgency)
@@ -71,15 +65,9 @@ func FormatTaskListWithFormat(tasks []*Task, ws *WorkingSet, format string) stri
 
 	// Add rows
 	for i, task := range tasks {
-		displayID := i + 1
-		if ws != nil {
-			// Use working set display ID if available
-			for id, uuid := range ws.byID {
-				if uuid == task.UUID {
-					displayID = id
-					break
-				}
-			}
+		displayID := resolveDisplayID(task, ws)
+		if displayID == 0 {
+			displayID = i + 1
 		}
 
 		urgency := task.CalculateUrgency(coeffs)
@@ -270,8 +258,6 @@ func FormatTagCounts(tagCounts map[string]int) string {
 	return t.Render()
 }
 
-// Helper functions
-
 func formatStatus(status Status) string {
 	switch status {
 	case StatusPending:
@@ -322,7 +308,6 @@ func formatUrgency(urgency float64) string {
 }
 
 func getUrgencyColor(urgency float64) *color.Color {
-	// Returns color for minimal format
 	if urgency >= 10.0 {
 		return color.New(color.FgHiRed, color.Bold)
 	} else if urgency >= 5.0 {
@@ -362,14 +347,6 @@ func formatDue(due *time.Time) string {
 	return rel
 }
 
-func formatTimeWithColor(t time.Time) string {
-	now := time.Now()
-	if t.Before(now) {
-		return color.RedString(t.Format("2006-01-02 15:04"))
-	}
-	return t.Format("2006-01-02 15:04")
-}
-
 func formatTime(t time.Time) string {
 	return t.Format("2006-01-02 15:04")
 }

opal-task/internal/engine/filter.go

@@ -71,10 +71,8 @@ func (f *Filter) ToSQL() (string, []interface{}) {
 	conditions := []string{}
 	args := []interface{}{}
 
-	// Track if we have an explicit status filter
 	hasStatusFilter := false
 
-	// Status filter
 	if status, ok := f.Attributes["status"]; ok {
 		hasStatusFilter = true
 
@@ -104,13 +102,11 @@ func (f *Filter) ToSQL() (string, []interface{}) {
 		}
 	}
 
-	// Project filter
 	if project, ok := f.Attributes["project"]; ok {
 		conditions = append(conditions, "project = ?")
 		args = append(args, project)
 	}
 
-	// Priority filter
 	if priority, ok := f.Attributes["priority"]; ok {
 		priorityInt := priorityStringToInt(priority)
 		conditions = append(conditions, "priority = ?")
@@ -138,7 +134,6 @@ func (f *Filter) ToSQL() (string, []interface{}) {
 		args = append(args, tag)
 	}
 
-	// UUID filter
 	if len(f.UUIDs) > 0 {
 		placeholders := strings.Repeat("?,", len(f.UUIDs))
 		placeholders = placeholders[:len(placeholders)-1]

opal-task/internal/engine/modifier.go

@@ -92,7 +92,6 @@ func (m *Modifier) Apply(task *Task) error {
 	for _, key := range m.AttributeOrder {
 		valuePtr := m.SetAttributes[key]
 
-		// Handle date attributes with relative expression support
 		if dateKeys[key] {
 			if err := applyDateAttribute(key, valuePtr, task, resolvedDates); err != nil {
 				return err
@@ -100,30 +99,11 @@ func (m *Modifier) Apply(task *Task) error {
 			continue
 		}
 
-		// Handle non-date attributes
-		switch key {
-		case "priority":
-			if valuePtr == nil {
-				task.Priority = PriorityDefault
-			} else {
-				task.Priority = Priority(priorityStringToInt(*valuePtr))
-			}
-		case "project":
-			task.Project = valuePtr
-		case "recur":
-			if valuePtr == nil {
-				task.RecurrenceDuration = nil
-			} else {
-				duration, err := ParseRecurrencePattern(*valuePtr)
-				if err != nil {
-					return fmt.Errorf("invalid recurrence: %w", err)
-				}
-				task.RecurrenceDuration = &duration
-			}
+		if err := applyNonDateAttribute(key, valuePtr, task); err != nil {
+			return err
 		}
 	}
 
-	// Apply tag changes
 	for _, tag := range m.AddTags {
 		if err := task.AddTag(tag); err != nil {
 			return err
@@ -160,7 +140,6 @@ func (m *Modifier) ApplyToNew(task *Task) error {
 	for _, key := range m.AttributeOrder {
 		valuePtr := m.SetAttributes[key]
 
-		// Handle date attributes with relative expression support
 		if dateKeys[key] {
 			if err := applyDateAttribute(key, valuePtr, task, resolvedDates); err != nil {
 				return err
@@ -168,26 +147,8 @@ func (m *Modifier) ApplyToNew(task *Task) error {
 			continue
 		}
 
-		// Handle non-date attributes
-		switch key {
-		case "priority":
-			if valuePtr == nil {
-				task.Priority = PriorityDefault
-			} else {
-				task.Priority = Priority(priorityStringToInt(*valuePtr))
-			}
-		case "project":
-			task.Project = valuePtr
-		case "recur":
-			if valuePtr == nil {
-				task.RecurrenceDuration = nil
-			} else {
-				duration, err := ParseRecurrencePattern(*valuePtr)
-				if err != nil {
-					return fmt.Errorf("invalid recurrence: %w", err)
-				}
-				task.RecurrenceDuration = &duration
-			}
+		if err := applyNonDateAttribute(key, valuePtr, task); err != nil {
+			return err
 		}
 	}
 
@@ -195,6 +156,31 @@ func (m *Modifier) ApplyToNew(task *Task) error {
 	return nil
 }
 
+// applyNonDateAttribute applies a non-date attribute (priority, project, recur) to a task.
+func applyNonDateAttribute(key string, valuePtr *string, task *Task) error {
+	switch key {
+	case "priority":
+		if valuePtr == nil {
+			task.Priority = PriorityDefault
+		} else {
+			task.Priority = Priority(priorityStringToInt(*valuePtr))
+		}
+	case "project":
+		task.Project = valuePtr
+	case "recur":
+		if valuePtr == nil {
+			task.RecurrenceDuration = nil
+		} else {
+			duration, err := ParseRecurrencePattern(*valuePtr)
+			if err != nil {
+				return fmt.Errorf("invalid recurrence: %w", err)
+			}
+			task.RecurrenceDuration = &duration
+		}
+	}
+	return nil
+}
+
 // parseRelativeExpression checks if a string is a relative date expression
 // Returns: baseAttr, operator, offset, isRelative
 // Example: "due-1d" -> "due", "-", "1d", true

opal-task/internal/engine/parser.go

@@ -14,20 +14,16 @@ func ParseKeyValueFormat(data string, skipComments bool) (map[string]string, err
 	lines := strings.Split(data, "\n")
 
 	for i, line := range lines {
-		// Trim whitespace
 		line = strings.TrimSpace(line)
 
-		// Skip empty lines
 		if line == "" {
 			continue
 		}
 
-		// Skip comments if requested
 		if skipComments && strings.HasPrefix(line, "#") {
 			continue
 		}
 
-		// Split on first ':'
 		parts := strings.SplitN(line, ":", 2)
 		if len(parts) != 2 {
 			return nil, fmt.Errorf("line %d: invalid format (expected 'key:value')", i+1)

opal-task/internal/engine/report.go

@@ -2,6 +2,7 @@ package engine
 
 import (
 	"fmt"
+	"sort"
 )
 
 // DisplayFormat defines how tasks should be displayed
@@ -131,16 +132,11 @@ func NewestReport() *Report {
 		BaseFilter:    filter,
 		DisplayFormat: DisplayFormatTable,
 		SortFunc: func(tasks []*Task) []*Task {
-			// Sort by created descending
 			sorted := make([]*Task, len(tasks))
 			copy(sorted, tasks)
-			for i := 0; i < len(sorted)-1; i++ {
-				for j := i + 1; j < len(sorted); j++ {
-					if sorted[i].Created.Before(sorted[j].Created) {
-						sorted[i], sorted[j] = sorted[j], sorted[i]
-					}
-				}
-			}
+			sort.Slice(sorted, func(i, j int) bool {
+				return sorted[i].Created.After(sorted[j].Created)
+			})
 			return sorted
 		},
 		LimitFunc: func(tasks []*Task) []*Task {
@@ -164,23 +160,14 @@ func NextReport() *Report {
 		BaseFilter:    filter,
 		DisplayFormat: DisplayFormatTable,
 		SortFunc: func(tasks []*Task) []*Task {
-			// Sort by urgency descending
 			cfg, _ := GetConfig()
 			coeffs := BuildUrgencyCoefficients(cfg)
 
 			sorted := make([]*Task, len(tasks))
 			copy(sorted, tasks)
-
-			for i := 0; i < len(sorted)-1; i++ {
-				for j := i + 1; j < len(sorted); j++ {
-					urgI := sorted[i].CalculateUrgency(coeffs)
-					urgJ := sorted[j].CalculateUrgency(coeffs)
-					if urgI < urgJ {
-						sorted[i], sorted[j] = sorted[j], sorted[i]
-					}
-				}
-			}
-
+			sort.Slice(sorted, func(i, j int) bool {
+				return sorted[i].CalculateUrgency(coeffs) > sorted[j].CalculateUrgency(coeffs)
+			})
 			return sorted
 		},
 		LimitFunc: func(tasks []*Task) []*Task {
@@ -208,16 +195,11 @@ func OldestReport() *Report {
 		BaseFilter:    filter,
 		DisplayFormat: DisplayFormatTable,
 		SortFunc: func(tasks []*Task) []*Task {
-			// Sort by created ascending (already default, but explicit)
 			sorted := make([]*Task, len(tasks))
 			copy(sorted, tasks)
-			for i := 0; i < len(sorted)-1; i++ {
-				for j := i + 1; j < len(sorted); j++ {
-					if sorted[i].Created.After(sorted[j].Created) {
-						sorted[i], sorted[j] = sorted[j], sorted[i]
-					}
-				}
-			}
+			sort.Slice(sorted, func(i, j int) bool {
+				return sorted[i].Created.Before(sorted[j].Created)
+			})
 			return sorted
 		},
 	}
@@ -429,18 +411,13 @@ func sortByUrgency(tasks []*Task) []*Task {
 	sorted := make([]*Task, len(tasks))
 	copy(sorted, tasks)
 
-	// Calculate and store urgency on each task
 	for _, t := range sorted {
 		t.Urgency = t.CalculateUrgency(coeffs)
 	}
 
-	for i := 0; i < len(sorted)-1; i++ {
-		for j := i + 1; j < len(sorted); j++ {
-			if sorted[i].Urgency < sorted[j].Urgency {
-				sorted[i], sorted[j] = sorted[j], sorted[i]
-			}
-		}
-	}
+	sort.Slice(sorted, func(i, j int) bool {
+		return sorted[i].Urgency > sorted[j].Urgency
+	})
 
 	return sorted
 }

opal-task/internal/engine/task.go

@@ -83,30 +83,31 @@ type Task struct {
 	Urgency float64  `json:"urgency"`
 }
 
+// taskJSON is the wire format for Task, using unix timestamps instead of time.Time.
+type taskJSON struct {
+	UUID               uuid.UUID    `json:"uuid"`
+	ID                 int          `json:"id"`
+	Status             Status       `json:"status"`
+	Description        string       `json:"description"`
+	Project            *string      `json:"project"`
+	Priority           Priority     `json:"priority"`
+	Created            int64        `json:"created"`
+	Modified           int64        `json:"modified"`
+	Start              *int64       `json:"start,omitempty"`
+	End                *int64       `json:"end,omitempty"`
+	Due                *int64       `json:"due,omitempty"`
+	Scheduled          *int64       `json:"scheduled,omitempty"`
+	Wait               *int64       `json:"wait,omitempty"`
+	Until              *int64       `json:"until,omitempty"`
+	RecurrenceDuration *int64       `json:"recurrence_duration,omitempty"`
+	ParentUUID         *uuid.UUID   `json:"parent_uuid,omitempty"`
+	Annotations        []Annotation `json:"annotations,omitempty"`
+	Tags               []string     `json:"tags"`
+	Urgency            float64      `json:"urgency"`
+}
+
 // MarshalJSON emits Task with unix timestamps (int64) instead of RFC3339 strings.
 func (t Task) MarshalJSON() ([]byte, error) {
-	type taskJSON struct {
-		UUID               uuid.UUID    `json:"uuid"`
-		ID                 int          `json:"id"`
-		Status             Status       `json:"status"`
-		Description        string       `json:"description"`
-		Project            *string      `json:"project"`
-		Priority           Priority     `json:"priority"`
-		Created            int64        `json:"created"`
-		Modified           int64        `json:"modified"`
-		Start              *int64       `json:"start,omitempty"`
-		End                *int64       `json:"end,omitempty"`
-		Due                *int64       `json:"due,omitempty"`
-		Scheduled          *int64       `json:"scheduled,omitempty"`
-		Wait               *int64       `json:"wait,omitempty"`
-		Until              *int64       `json:"until,omitempty"`
-		RecurrenceDuration *int64       `json:"recurrence_duration,omitempty"`
-		ParentUUID         *uuid.UUID   `json:"parent_uuid,omitempty"`
-		Annotations        []Annotation `json:"annotations,omitempty"`
-		Tags               []string     `json:"tags"`
-		Urgency            float64      `json:"urgency"`
-	}
-
 	toUnix := func(tp *time.Time) *int64 {
 		if tp == nil {
 			return nil
@@ -146,28 +147,6 @@ func (t Task) MarshalJSON() ([]byte, error) {
 
 // UnmarshalJSON parses Task from JSON with unix timestamps (int64) and duration in seconds.
 func (t *Task) UnmarshalJSON(data []byte) error {
-	type taskJSON struct {
-		UUID               uuid.UUID    `json:"uuid"`
-		ID                 int          `json:"id"`
-		Status             Status       `json:"status"`
-		Description        string       `json:"description"`
-		Project            *string      `json:"project"`
-		Priority           Priority     `json:"priority"`
-		Created            int64        `json:"created"`
-		Modified           int64        `json:"modified"`
-		Start              *int64       `json:"start,omitempty"`
-		End                *int64       `json:"end,omitempty"`
-		Due                *int64       `json:"due,omitempty"`
-		Scheduled          *int64       `json:"scheduled,omitempty"`
-		Wait               *int64       `json:"wait,omitempty"`
-		Until              *int64       `json:"until,omitempty"`
-		RecurrenceDuration *int64       `json:"recurrence_duration,omitempty"`
-		ParentUUID         *uuid.UUID   `json:"parent_uuid,omitempty"`
-		Annotations        []Annotation `json:"annotations,omitempty"`
-		Tags               []string     `json:"tags"`
-		Urgency            float64      `json:"urgency"`
-	}
-
 	var raw taskJSON
 	if err := json.Unmarshal(data, &raw); err != nil {
 		return err
@@ -366,21 +345,13 @@ func CreateTaskWithModifier(description string, mod *Modifier) (*Task, error) {
 	return task, nil
 }
 
-// GetTask retrieves a task by UUID
-func GetTask(taskUUID uuid.UUID) (*Task, error) {
-	db := GetDB()
-	if db == nil {
-		return nil, fmt.Errorf("database not initialized")
-	}
-
-	query := `
-		SELECT id, uuid, status, description, project, priority,
-		       created, modified, start, end, due, scheduled, wait, until_date,
-		       recurrence_duration, parent_uuid, annotations
-		FROM tasks
-		WHERE uuid = ?
-	`
+// scanner is satisfied by both *sql.Row and *sql.Rows.
+type scanner interface {
+	Scan(dest ...interface{}) error
+}
 
+// scanTask reads a single task row from a scanner and populates all fields including tags.
+func scanTask(s scanner) (*Task, error) {
 	task := &Task{}
 	var (
 		uuidStr        string
@@ -398,7 +369,7 @@ func GetTask(taskUUID uuid.UUID) (*Task, error) {
 		annotationsStr interface{}
 	)
 
-	err := db.QueryRow(query, taskUUID.String()).Scan(
+	err := s.Scan(
 		&task.ID,
 		&uuidStr,
 		&task.Status,
@@ -417,22 +388,18 @@ func GetTask(taskUUID uuid.UUID) (*Task, error) {
 		&parentUUIDStr,
 		&annotationsStr,
 	)
-
 	if err != nil {
-		return nil, fmt.Errorf("failed to get task: %w", err)
+		return nil, err
 	}
 
-	// Parse UUID
 	task.UUID, err = uuid.Parse(uuidStr)
 	if err != nil {
 		return nil, fmt.Errorf("failed to parse UUID: %w", err)
 	}
 
-	// Convert timestamps
 	task.Created = time.Unix(created, 0)
 	task.Modified = time.Unix(modified, 0)
 
-	// Convert nullable fields
 	task.Project = sqlToStringPtr(project)
 	task.Start = sqlToTime(start)
 	task.End = sqlToTime(end)
@@ -444,7 +411,6 @@ func GetTask(taskUUID uuid.UUID) (*Task, error) {
 	task.ParentUUID = sqlToUUIDPtr(parentUUIDStr)
 	task.Annotations = sqlToAnnotations(annotationsStr)
 
-	// Load tags
 	tags, err := task.GetTags()
 	if err != nil {
 		return nil, fmt.Errorf("failed to load tags: %w", err)
@@ -454,6 +420,29 @@ func GetTask(taskUUID uuid.UUID) (*Task, error) {
 	return task, nil
 }
 
+// GetTask retrieves a task by UUID
+func GetTask(taskUUID uuid.UUID) (*Task, error) {
+	db := GetDB()
+	if db == nil {
+		return nil, fmt.Errorf("database not initialized")
+	}
+
+	query := `
+		SELECT id, uuid, status, description, project, priority,
+		       created, modified, start, end, due, scheduled, wait, until_date,
+		       recurrence_duration, parent_uuid, annotations
+		FROM tasks
+		WHERE uuid = ?
+	`
+
+	task, err := scanTask(db.QueryRow(query, taskUUID.String()))
+	if err != nil {
+		return nil, fmt.Errorf("failed to get task: %w", err)
+	}
+
+	return task, nil
+}
+
 // GetTasks retrieves all tasks with optional filtering
 func GetTasks(filter *Filter) ([]*Task, error) {
 	db := GetDB()
@@ -489,76 +478,10 @@ func GetTasks(filter *Filter) ([]*Task, error) {
 	tasks := []*Task{}
 
 	for rows.Next() {
-		task := &Task{}
-		var (
-			uuidStr        string
-			project        interface{}
-			created        int64
-			modified       int64
-			start          interface{}
-			end            interface{}
-			due            interface{}
-			scheduled      interface{}
-			wait           interface{}
-			until          interface{}
-			recurDuration  interface{}
-			parentUUIDStr  interface{}
-			annotationsStr interface{}
-		)
-
-		err := rows.Scan(
-			&task.ID,
-			&uuidStr,
-			&task.Status,
-			&task.Description,
-			&project,
-			&task.Priority,
-			&created,
-			&modified,
-			&start,
-			&end,
-			&due,
-			&scheduled,
-			&wait,
-			&until,
-			&recurDuration,
-			&parentUUIDStr,
-			&annotationsStr,
-		)
-
+		task, err := scanTask(rows)
 		if err != nil {
 			return nil, fmt.Errorf("failed to scan task: %w", err)
 		}
-
-		// Parse UUID
-		task.UUID, err = uuid.Parse(uuidStr)
-		if err != nil {
-			return nil, fmt.Errorf("failed to parse UUID: %w", err)
-		}
-
-		// Convert timestamps
-		task.Created = time.Unix(created, 0)
-		task.Modified = time.Unix(modified, 0)
-
-		// Convert nullable fields
-		task.Project = sqlToStringPtr(project)
-		task.Start = sqlToTime(start)
-		task.End = sqlToTime(end)
-		task.Due = sqlToTime(due)
-		task.Scheduled = sqlToTime(scheduled)
-		task.Wait = sqlToTime(wait)
-		task.Until = sqlToTime(until)
-		task.RecurrenceDuration = sqlToDuration(recurDuration)
-		task.ParentUUID = sqlToUUIDPtr(parentUUIDStr)
-		task.Annotations = sqlToAnnotations(annotationsStr)
-
-		// Load tags
-		tags, err := task.GetTags()
-		if err != nil {
-			return nil, fmt.Errorf("failed to load tags: %w", err)
-		}
-		task.Tags = tags
-
 		tasks = append(tasks, task)
 	}
 

opal-task/internal/sync/client.go

@@ -102,14 +102,20 @@ func (c *Client) PullChanges(since int64) ([]ChangeLogEntry, error) {
 func (c *Client) PushChanges(tasks []*engine.Task) error {
 	// Convert tasks to JSON
 	var taskData []json.RawMessage
+	var marshalErrors []string
 	for _, task := range tasks {
 		data, err := json.Marshal(task)
 		if err != nil {
+			marshalErrors = append(marshalErrors, fmt.Sprintf("task %s: %v", task.UUID, err))
 			continue
 		}
 		taskData = append(taskData, data)
 	}
 
+	if len(taskData) == 0 && len(marshalErrors) > 0 {
+		return fmt.Errorf("all tasks failed to marshal: %s", strings.Join(marshalErrors, "; "))
+	}
+
 	reqBody := map[string]interface{}{
 		"tasks":     taskData,
 		"client_id": c.clientID,
@@ -139,6 +145,11 @@ func (c *Client) PushChanges(tasks []*engine.Task) error {
 		return fmt.Errorf("server returned %d: %s", resp.StatusCode, string(body))
 	}
 
+	if len(marshalErrors) > 0 {
+		return fmt.Errorf("pushed %d tasks but %d failed to marshal: %s",
+			len(taskData), len(marshalErrors), strings.Join(marshalErrors, "; "))
+	}
+
 	return nil
 }
 
@@ -219,7 +230,7 @@ func (c *Client) Sync(strategy ConflictResolution, reporter ProgressReporter) (*
 	}
 
 	// Convert changes to tasks
-	remoteTasks, err := c.parseChanges(changes)
+	remoteTasks, err := c.ParseChanges(changes)
 	if err != nil {
 		if len(changes) > 0 {
 			reporter.CompletePhase()
@@ -283,6 +294,11 @@ func (c *Client) Sync(strategy ConflictResolution, reporter ProgressReporter) (*
 			continue
 		}
 
+		// Mark change_log entry as sync-originated to prevent feedback loop
+		if err := engine.MarkChangeLogAsSync(task.UUID.String()); err != nil {
+			result.Errors = append(result.Errors, fmt.Sprintf("failed to mark change as sync for %s: %v", task.UUID, err))
+		}
+
 		// Reload task to ensure we have the database ID
 		savedTask, err := engine.GetTask(task.UUID)
 		if err != nil {
@@ -347,18 +363,7 @@ func (c *Client) Sync(strategy ConflictResolution, reporter ProgressReporter) (*
 
 // getLastSyncTime retrieves the last sync timestamp from database
 func (c *Client) getLastSyncTime() int64 {
-	db := engine.GetDB()
-	if db == nil {
-		return 0
-	}
-
-	var lastSync int64
-	err := db.QueryRow("SELECT last_sync FROM sync_state WHERE client_id = ?", c.clientID).Scan(&lastSync)
-	if err != nil {
-		return 0
-	}
-
-	return lastSync
+	return GetLastSyncTime(c.clientID)
 }
 
 // updateLastSyncTime updates the last sync timestamp
@@ -376,6 +381,27 @@ func (c *Client) updateLastSyncTime(timestamp int64) {
 
 // getLocalChanges retrieves local changes since a timestamp
 func (c *Client) getLocalChanges(since int64) ([]*engine.Task, error) {
+	return GetLocalChanges(since)
+}
+
+// GetLastSyncTime retrieves the last sync timestamp for a client ID from the database.
+func GetLastSyncTime(clientID string) int64 {
+	db := engine.GetDB()
+	if db == nil {
+		return 0
+	}
+
+	var lastSync int64
+	err := db.QueryRow("SELECT last_sync FROM sync_state WHERE client_id = ?", clientID).Scan(&lastSync)
+	if err != nil {
+		return 0
+	}
+
+	return lastSync
+}
+
+// GetLocalChanges retrieves local (non-sync-originated) changes since a timestamp.
+func GetLocalChanges(since int64) ([]*engine.Task, error) {
 	db := engine.GetDB()
 	if db == nil {
 		return nil, fmt.Errorf("database not initialized")
@@ -384,7 +410,7 @@ func (c *Client) getLocalChanges(since int64) ([]*engine.Task, error) {
 	rows, err := db.Query(`
 		SELECT DISTINCT task_uuid
 		FROM change_log
-		WHERE changed_at > ?
+		WHERE changed_at > ? AND source = 'local'
 		ORDER BY changed_at ASC
 	`, since)
 	if err != nil {
@@ -415,8 +441,8 @@ func (c *Client) getLocalChanges(since int64) ([]*engine.Task, error) {
 	return tasks, nil
 }
 
-// parseChanges converts change log entries to tasks
-func (c *Client) parseChanges(changes []ChangeLogEntry) ([]*engine.Task, error) {
+// ParseChanges converts change log entries to tasks
+func (c *Client) ParseChanges(changes []ChangeLogEntry) ([]*engine.Task, error) {
 	// Sort changes by timestamp (primary) and ID (secondary) to ensure correct order
 	// This handles same-second updates (e.g., CREATE followed by UPDATE with tags)
 	sort.Slice(changes, func(i, j int) bool {
@@ -666,16 +692,31 @@ func parseTagsFromChangeLog(s string) []string {
 // pushQueuedChanges sends queued changes to server
 func (c *Client) pushQueuedChanges(changes []QueuedChange) error {
 	var tasks []*engine.Task
+	var unmarshalErrors []string
 
 	for _, change := range changes {
 		var task engine.Task
 		if err := json.Unmarshal(change.Data, &task); err != nil {
+			unmarshalErrors = append(unmarshalErrors, fmt.Sprintf("queued change: %v", err))
 			continue
 		}
 		tasks = append(tasks, &task)
 	}
 
-	return c.PushChanges(tasks)
+	if len(tasks) == 0 && len(unmarshalErrors) > 0 {
+		return fmt.Errorf("all queued changes failed to unmarshal: %s", strings.Join(unmarshalErrors, "; "))
+	}
+
+	if err := c.PushChanges(tasks); err != nil {
+		return err
+	}
+
+	if len(unmarshalErrors) > 0 {
+		return fmt.Errorf("pushed %d tasks but %d queued changes failed to unmarshal: %s",
+			len(tasks), len(unmarshalErrors), strings.Join(unmarshalErrors, "; "))
+	}
+
+	return nil
 }
 
 // SyncResult represents the result of a sync operation

opal-task/internal/sync/strategy.go

@@ -57,7 +57,11 @@ func MergeTasks(local, remote []*engine.Task, strategy ConflictResolution) ([]*e
 		if DetectConflict(task, remoteTask) {
 			conflicts++
 			winner := resolveConflict(task, remoteTask, strategy)
-			logConflict(task, remoteTask, winner)
+			winnerLabel := "local"
+			if winner == remoteTask {
+				winnerLabel = "remote"
+			}
+			logConflict(task, remoteTask, winnerLabel)
 			result = append(result, winner)
 		} else {
 			// No conflict - use either (same content)
@@ -110,17 +114,12 @@ func resolveConflict(local, remote *engine.Task, strategy ConflictResolution) *e
 }
 
 // logConflict writes conflict information to log file
-func logConflict(local, remote *engine.Task, winner *engine.Task) {
+func logConflict(local, remote *engine.Task, winnerLabel string) {
 	logPath, err := engine.GetSyncConflictLogPath()
 	if err != nil {
 		return
 	}
 
-	winnerLabel := "local"
-	if winner.UUID == remote.UUID && winner.Modified.Equal(remote.Modified) {
-		winnerLabel = "remote"
-	}
-
 	entry := fmt.Sprintf(
 		"[%s] Conflict on task %s\n"+
 			"  Local:  modified %s - %s\n"+