From b304a80cade7f8208addf036b320f8796987425c Mon Sep 17 00:00:00 2001 From: tester Date: Fri, 15 May 2026 02:25:40 -0700 Subject: [PATCH] feat(events): normalize event taxonomy at emit and consume sites Route the SSE autosync push/pull paths through the events taxonomy normalizer at the wire boundary, normalize admin_events action_type filter, and add an EmitEvent helper that normalizes both inputs and validates against ValidEntityActionCombinations. Also fill in explicit legacy verb mappings for issue state transitions (close, approve, reject, block, unblock, reopen, review_*, start, board_move_issue, close_after_review) so the contract is locked by tests rather than relying on the catch-all default. Tests: exhaustive legacy verb table, round-trip canonical check, unknown-entity negatives, and EmitEvent valid/invalid combinations. Nightshift-Task: event-taxonomy Nightshift-Ref: https://github.com/marcus/nightshift --- internal/api/admin_events.go | 9 +- internal/events/taxonomy.go | 40 +++++++- internal/events/taxonomy_test.go | 161 +++++++++++++++++++++++++++++++ internal/serve/sse.go | 35 +++++-- 4 files changed, 235 insertions(+), 10 deletions(-) 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()