diff --git a/internal/api/admin_events.go b/internal/api/admin_events.go index fd8f0b9c..21007aa4 100644 --- a/internal/api/admin_events.go +++ b/internal/api/admin_events.go @@ -99,8 +99,15 @@ func (s *Server) handleAdminProjectEvents(w http.ResponseWriter, r *http.Request args = append(args, string(normalized)) } if v := q.Get("action_type"); v != "" { + // Normalize legacy action verbs (e.g. "handoff" → "create") so + // callers can filter using either legacy or canonical strings. + // Already-canonical strings round-trip unchanged. + normalizedAction := v + if !tdevents.IsValidActionType(v) { + normalizedAction = string(tdevents.NormalizeActionType(v)) + } query += " AND action_type = ?" - args = append(args, v) + args = append(args, normalizedAction) } if v := q.Get("from"); v != "" { query += " AND server_timestamp >= ?" diff --git a/internal/events/taxonomy.go b/internal/events/taxonomy.go index 5bd50faf..4e43c2b0 100644 --- a/internal/events/taxonomy.go +++ b/internal/events/taxonomy.go @@ -25,13 +25,19 @@ // - 'remove_dependency', 'unlink_file', 'board_delete', 'work_session_untag' → 'delete' // - 'delete', 'board_unposition', 'board_remove_issue', 'soft_delete' → 'soft_delete' // - 'restore' → 'restore' +// - Issue state transitions ('start', 'review', 'review_approve', +// 'review_changes_requested', 'close_after_review', 'approve', 'reject', +// 'block', 'unblock', 'close', 'reopen', 'board_move_issue') → 'update' // - Others default to 'update' // // This mapping ensures existing events in the events table with old action/entity types // can be queried and processed correctly by the sync engine. package events -import "strings" +import ( + "fmt" + "strings" +) // EntityType represents the canonical entity types in the sync system. type EntityType string @@ -163,11 +169,43 @@ func NormalizeActionType(tdAction string) ActionType { return ActionSoftDelete case "restore": return ActionRestore + case "update", "start", "review", "review_approve", "review_changes_requested", + "close_after_review", "approve", "reject", "block", "unblock", "close", + "reopen", "board_move_issue": + return ActionUpdate default: return ActionUpdate } } +// EmitEvent normalizes the given entity type and action type strings to their +// canonical forms and validates that the combination is allowed by +// ValidEntityActionCombinations. +// +// It is intentionally side-effect free: callers remain responsible for any +// DB writes / network sends. The helper exists so emit sites can route raw +// strings through a single chokepoint and surface invalid pairings (e.g. +// 'logs' + 'restore', or unknown entity types) at the boundary rather than +// silently dropping them downstream. +// +// Returns the canonical EntityType, the canonical ActionType, and a non-nil +// error when the entity is unknown or the entity+action pair is not in +// ValidEntityActionCombinations. +func EmitEvent(entityType, actionType string) (EntityType, ActionType, error) { + canonicalEntity, ok := NormalizeEntityType(entityType) + if !ok { + return "", "", fmt.Errorf("events: unknown entity type %q", entityType) + } + canonicalAction := NormalizeActionType(actionType) + if !IsValidEntityActionCombination(canonicalEntity, canonicalAction) { + return canonicalEntity, canonicalAction, fmt.Errorf( + "events: invalid combination entity=%q action=%q", + canonicalEntity, canonicalAction, + ) + } + return canonicalEntity, canonicalAction, nil +} + // ValidEntityActionCombinations defines which entity types can have which action types. // Used for validation and semantic checking. func ValidEntityActionCombinations() map[EntityType]map[ActionType]bool { diff --git a/internal/events/taxonomy_test.go b/internal/events/taxonomy_test.go index ccee8eb7..829a6937 100644 --- a/internal/events/taxonomy_test.go +++ b/internal/events/taxonomy_test.go @@ -280,6 +280,167 @@ func TestAllActionTypes(t *testing.T) { } } +func TestNormalizeActionType_LegacyVerbsExhaustive(t *testing.T) { + // Every legacy action_log verb listed in the package doc plus the + // issue state transitions discovered during the audit. This test exists + // so future edits to NormalizeActionType cannot silently change the + // legacy → canonical mapping. + tests := []struct { + legacy string + expected ActionType + }{ + // Documented create-aliases + {"create", ActionCreate}, + {"handoff", ActionCreate}, + {"add_dependency", ActionCreate}, + {"link_file", ActionCreate}, + {"board_create", ActionCreate}, + {"board_update", ActionCreate}, + {"board_add_issue", ActionCreate}, + {"board_set_position", ActionCreate}, + {"work_session_tag", ActionCreate}, + + // Documented delete-aliases + {"remove_dependency", ActionDelete}, + {"unlink_file", ActionDelete}, + {"board_delete", ActionDelete}, + {"work_session_untag", ActionDelete}, + + // Documented soft_delete-aliases + {"delete", ActionSoftDelete}, + {"board_unposition", ActionSoftDelete}, + {"board_remove_issue", ActionSoftDelete}, + {"soft_delete", ActionSoftDelete}, + + // Restore + {"restore", ActionRestore}, + + // Issue state transitions (explicit update-aliases) + {"update", ActionUpdate}, + {"start", ActionUpdate}, + {"review", ActionUpdate}, + {"review_approve", ActionUpdate}, + {"review_changes_requested", ActionUpdate}, + {"close_after_review", ActionUpdate}, + {"approve", ActionUpdate}, + {"reject", ActionUpdate}, + {"block", ActionUpdate}, + {"unblock", ActionUpdate}, + {"close", ActionUpdate}, + {"reopen", ActionUpdate}, + {"board_move_issue", ActionUpdate}, + + // Case-insensitive + {"HANDOFF", ActionCreate}, + {"Board_Delete", ActionDelete}, + + // Unknown defaults to update (preserve forward-compat) + {"totally_unknown_verb", ActionUpdate}, + } + + for _, test := range tests { + got := NormalizeActionType(test.legacy) + if got != test.expected { + t.Errorf("NormalizeActionType(%q) = %q; want %q", test.legacy, got, test.expected) + } + } +} + +func TestNormalizeEntityType_RoundTripCanonical(t *testing.T) { + // Every normalized output must itself be in AllEntityTypes() — i.e. + // the normalizer cannot emit a string that the validator rejects. + canon := AllEntityTypes() + for raw := range canon { + got, ok := NormalizeEntityType(string(raw)) + if !ok { + t.Errorf("NormalizeEntityType(%q): canonical form rejected by normalizer", raw) + continue + } + if !canon[got] { + t.Errorf("NormalizeEntityType(%q) = %q; not in AllEntityTypes()", raw, got) + } + } +} + +func TestNormalizeEntityType_UnknownNegative(t *testing.T) { + negatives := []string{"users", "foo", "issue_review_v2", "boards_", " "} + for _, n := range negatives { + if _, ok := NormalizeEntityType(n); ok { + t.Errorf("NormalizeEntityType(%q): expected invalid, got valid", n) + } + } +} + +func TestEmitEvent(t *testing.T) { + tests := []struct { + name string + entity string + action string + wantEntity EntityType + wantAction ActionType + wantErr bool + }{ + // Happy path: canonical inputs + {"canonical_issue_create", "issues", "create", EntityIssues, ActionCreate, false}, + {"canonical_issue_update", "issues", "update", EntityIssues, ActionUpdate, false}, + + // Singular legacy entity + legacy action + {"legacy_handoff_verb", "handoffs", "handoff", EntityHandoffs, ActionCreate, false}, + {"legacy_dep_create", "dependency", "add_dependency", EntityIssueDependencies, ActionCreate, false}, + {"legacy_dep_remove", "dependency", "remove_dependency", EntityIssueDependencies, ActionDelete, false}, + {"legacy_file_link", "file_link", "link_file", EntityIssueFiles, ActionCreate, false}, + {"legacy_board_create", "board", "board_create", EntityBoards, ActionCreate, false}, + + // Issue state transitions on issues + {"issue_close", "issue", "close", EntityIssues, ActionUpdate, false}, + {"issue_review_approve", "issue", "review_approve", EntityIssues, ActionUpdate, false}, + + // Restore on issues is valid + {"issue_restore", "issues", "restore", EntityIssues, ActionRestore, false}, + + // Invalid combination: logs cannot be restored + {"logs_restore_invalid", "logs", "restore", EntityLogs, ActionRestore, true}, + // Invalid combination: git_snapshots cannot be updated + {"git_snapshots_update_invalid", "git_snapshots", "update", EntityGitSnapshots, ActionUpdate, true}, + // Invalid combination: work_session_issues cannot be updated + {"wsi_update_invalid", "work_session_issues", "update", EntityWorkSessionIssues, ActionUpdate, true}, + + // Unknown entity is rejected outright + {"unknown_entity", "users", "create", "", "", true}, + {"empty_entity", "", "create", "", "", true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + gotEntity, gotAction, err := EmitEvent(tc.entity, tc.action) + if tc.wantErr { + if err == nil { + t.Fatalf("EmitEvent(%q, %q): expected error, got nil (entity=%q action=%q)", + tc.entity, tc.action, gotEntity, gotAction) + } + if gotEntity != tc.wantEntity { + t.Errorf("EmitEvent(%q, %q): entity = %q; want %q", + tc.entity, tc.action, gotEntity, tc.wantEntity) + } + if gotAction != tc.wantAction { + t.Errorf("EmitEvent(%q, %q): action = %q; want %q", + tc.entity, tc.action, gotAction, tc.wantAction) + } + return + } + if err != nil { + t.Fatalf("EmitEvent(%q, %q): unexpected error: %v", tc.entity, tc.action, err) + } + if gotEntity != tc.wantEntity { + t.Errorf("EmitEvent(%q, %q): entity = %q; want %q", tc.entity, tc.action, gotEntity, tc.wantEntity) + } + if gotAction != tc.wantAction { + t.Errorf("EmitEvent(%q, %q): action = %q; want %q", tc.entity, tc.action, gotAction, tc.wantAction) + } + }) + } +} + func TestValidEntityActionCombinations(t *testing.T) { combinations := ValidEntityActionCombinations() diff --git a/internal/serve/sse.go b/internal/serve/sse.go index 6d0aa399..f6b4a23f 100644 --- a/internal/serve/sse.go +++ b/internal/serve/sse.go @@ -11,6 +11,7 @@ import ( "time" "github.com/marcus/td/internal/db" + tdevents "github.com/marcus/td/internal/events" "github.com/marcus/td/internal/session" tdsync "github.com/marcus/td/internal/sync" "github.com/marcus/td/internal/syncclient" @@ -412,10 +413,19 @@ func serveAutoSyncPush(database *db.DB, client *syncclient.Client, state *db.Syn SessionID: sessionID, } for _, ev := range events { + // GetPendingEvents already routes through the taxonomy normalizer, + // but re-normalize at the wire boundary so any future emit-site + // that bypasses GetPendingEvents still produces canonical strings. + canonicalEntity, action, err := tdevents.EmitEvent(ev.EntityType, ev.ActionType) + if err != nil { + slog.Debug("serve autosync: drop event with invalid taxonomy", + "entity", ev.EntityType, "action", ev.ActionType, "err", err) + continue + } pushReq.Events = append(pushReq.Events, syncclient.EventInput{ ClientActionID: ev.ClientActionID, - ActionType: ev.ActionType, - EntityType: ev.EntityType, + ActionType: string(action), + EntityType: string(canonicalEntity), EntityID: ev.EntityID, Payload: ev.Payload, ClientTimestamp: ev.ClientTimestamp.Format(time.RFC3339), @@ -474,23 +484,32 @@ func serveAutoSyncPull(database *db.DB, client *syncclient.Client, state *db.Syn break } - events := make([]tdsync.Event, len(pullResp.Events)) - for i, pe := range pullResp.Events { + events := make([]tdsync.Event, 0, len(pullResp.Events)) + for _, pe := range pullResp.Events { clientTS, err := time.Parse(time.RFC3339Nano, pe.ClientTimestamp) if err != nil { clientTS, _ = time.Parse(time.RFC3339, pe.ClientTimestamp) } - events[i] = tdsync.Event{ + // Normalize on the consume side so downstream comparisons + // (entity_type / action_type) are always against canonical + // strings even if the server somehow returned legacy forms. + canonicalEntity, action, err := tdevents.EmitEvent(pe.EntityType, pe.ActionType) + if err != nil { + slog.Debug("serve autosync: drop pulled event with invalid taxonomy", + "entity", pe.EntityType, "action", pe.ActionType, "err", err) + continue + } + events = append(events, tdsync.Event{ ServerSeq: pe.ServerSeq, DeviceID: pe.DeviceID, SessionID: pe.SessionID, ClientActionID: pe.ClientActionID, - ActionType: pe.ActionType, - EntityType: pe.EntityType, + ActionType: string(action), + EntityType: string(canonicalEntity), EntityID: pe.EntityID, Payload: pe.Payload, ClientTimestamp: clientTS, - } + }) } conn := database.Conn()