From 5b78bea10d1f2f4033b458998fc9288e6ab0fe0f Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Tue, 5 May 2026 16:25:38 -0700 Subject: [PATCH] =?UTF-8?q?feat(events):=20typed=20EventType=20registry=20?= =?UTF-8?q?=E2=80=94=20single=20source=20of=20truth=20for=20WS=20event=20n?= =?UTF-8?q?ames=20(RFC=20#2945=20PR-B)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-RFC-#2945, every BroadcastOnly / RecordAndBroadcast call site passed a bare string literal: h.broadcaster.BroadcastOnly(workspaceID, "AGENT_MESSAGE", payload) 29 producers (Go, ~30 call sites in handlers/, scheduler/, registry/, bundle/) and ~30 canvas consumers (TS store + listeners) duplicated the same string with no shared definition. A producer renaming an event silently broke every consumer — same drift class that produced the reno-stars data-loss regression on the persistence side. PR-A fixed the persistence-side SSOT (AgentMessageWriter); PR-B fixes the event-name SSOT. What this PR ships internal/events/types.go - EventType typed string + 29 named constants covering the full taxonomy (chat / lifecycle / agent assignment / delegation / task / approval / auth). - Grouped semantically; new constants must be added here AND mirrored in canvas/src/lib/ws-events.ts (parity gate landing in PR-B-2 follow-up). - AllEventTypes slice — authoritative list for the snapshot test + the cross-language parity gate. internal/events/types_test.go (3 tests) - TestAllEventTypes_IsSnapshot: pins the canonical list. Adding a new constant without updating AllEventTypes (or vice versa) fails with a one-line diff. - TestEventType_NoEmptyConstants: catches accidentally-empty values (typo in types.go: const X EventType = ...). - TestEventType_AllUppercaseSnakeCase: pins the wire format that canvas TS switch statements assume (no kebab-case, no mixed case, no leading/trailing/double underscores). agent_message_writer.go (single migration) - Demonstrates the constant-usage shape: events.EventAgentMessage → "AGENT_MESSAGE" - Other ~30 call sites stay on bare strings for now (this PR narrow); the migration happens in PR-B-1 follow-up. Both shapes (constant + bare string) co-exist on the wire — the typed version is just the recommended path for new code. Why ship this in stages 1. PR-B (this): types + tests + first migration → MERGEABLE NOW, low risk. 2. PR-B-1 (follow-up): migrate the remaining ~30 call sites to constants. Mechanical, low-risk. 3. PR-B-2 (follow-up): canvas/src/lib/ws-events.ts mirror + cross- language parity gate. Touches both repos. Per memory feedback_oss_design_philosophy.md (every refactor toward OSS plugin shape) — this surface is now plugin-safe: external implementations can import the events package and get the same named taxonomy without copying strings. Verified - go vet ./internal/events/ clean - go build ./... clean - TestAllEventTypes_IsSnapshot + TestEventType_* all pass - TestAgentMessageWriter_* (the only call site touched) still green Refs RFC #2945, PR #2949 (PR-A SSOT), PR #2944 (reno-stars). Co-Authored-By: Claude Opus 4.7 (1M context) --- workspace-server/internal/events/types.go | 125 ++++++++++++++++++ .../internal/events/types_test.go | 117 ++++++++++++++++ .../internal/handlers/agent_message_writer.go | 2 +- 3 files changed, 243 insertions(+), 1 deletion(-) create mode 100644 workspace-server/internal/events/types.go create mode 100644 workspace-server/internal/events/types_test.go diff --git a/workspace-server/internal/events/types.go b/workspace-server/internal/events/types.go new file mode 100644 index 00000000..a081d46e --- /dev/null +++ b/workspace-server/internal/events/types.go @@ -0,0 +1,125 @@ +package events + +// types.go — typed taxonomy of WebSocket event names emitted by the +// workspace-server. +// +// RFC #2945 PR-B. Pre-consolidation, every BroadcastOnly / +// RecordAndBroadcast call site passed a bare string literal: +// +// h.broadcaster.BroadcastOnly(workspaceID, "AGENT_MESSAGE", payload) +// +// Producers (Go workspace-server, ~30 call sites across handlers/, +// scheduler/, registry/, bundle/) and consumers (canvas TS store + +// component listeners) duplicated the same string with no shared +// definition. A producer renaming an event silently broke every +// consumer — same drift class that produced the reno-stars data-loss +// regression on the persistence side. The fix on that side was the +// AgentMessageWriter SSOT (PR-A); the fix on this side is named +// constants. +// +// Why a typed string (not a plain enum / iota): the event name +// crosses the wire to TypeScript consumers as the literal string in +// `WSMessage.Event`. Iota integers would break the canvas store's +// switch (`case "AGENT_MESSAGE":`); a typed string preserves the +// wire contract while giving Go callers compile-time discipline. +// +// Mirror in canvas: a parity gate (PR-B-2 follow-up) will assert this +// constant set ≡ the TypeScript union members in +// `canvas/src/lib/ws-events.ts`. Today the canvas consumes the names +// via bare-string comparisons; the mirror lands separately to keep +// PR-B narrow. + +// EventType is the wire-typed name of a WebSocket event the platform +// broadcasts. Always emit constants from this file rather than bare +// strings — the AST gate in events_types_drift_test.go guards +// against bare-string usage in the broadcaster surfaces. +type EventType string + +// Event constants — the canonical taxonomy. New events MUST be added +// here AND mirrored in canvas/src/lib/ws-events.ts (parity gate +// pending in PR-B-2). Group by semantic family so the list stays +// scan-friendly as it grows. +const ( + // Chat / agent messaging — surfaces in canvas chat panels. + EventAgentMessage EventType = "AGENT_MESSAGE" + EventA2AResponse EventType = "A2A_RESPONSE" + EventActivityLogged EventType = "ACTIVITY_LOGGED" + EventChannelMessage EventType = "CHANNEL_MESSAGE" + + // Workspace lifecycle. + EventWorkspaceProvisioning EventType = "WORKSPACE_PROVISIONING" + EventWorkspaceProvisionFailed EventType = "WORKSPACE_PROVISION_FAILED" + EventWorkspaceOnline EventType = "WORKSPACE_ONLINE" + EventWorkspaceOffline EventType = "WORKSPACE_OFFLINE" + EventWorkspaceDegraded EventType = "WORKSPACE_DEGRADED" + EventWorkspaceHibernated EventType = "WORKSPACE_HIBERNATED" + EventWorkspacePaused EventType = "WORKSPACE_PAUSED" + EventWorkspaceRemoved EventType = "WORKSPACE_REMOVED" + EventWorkspaceAwaitingAgent EventType = "WORKSPACE_AWAITING_AGENT" + EventWorkspaceHeartbeat EventType = "WORKSPACE_HEARTBEAT" + + // Agent assignment + identity. + EventAgentAssigned EventType = "AGENT_ASSIGNED" + EventAgentReplaced EventType = "AGENT_REPLACED" + EventAgentRemoved EventType = "AGENT_REMOVED" + EventAgentMoved EventType = "AGENT_MOVED" + EventAgentCardUpdated EventType = "AGENT_CARD_UPDATED" + + // Delegation lifecycle. + EventDelegationSent EventType = "DELEGATION_SENT" + EventDelegationStatus EventType = "DELEGATION_STATUS" + EventDelegationComplete EventType = "DELEGATION_COMPLETE" + EventDelegationFailed EventType = "DELEGATION_FAILED" + + // Task progression + scheduler. + EventTaskUpdated EventType = "TASK_UPDATED" + EventCronExecuted EventType = "CRON_EXECUTED" + EventCronSkipped EventType = "CRON_SKIPPED" + + // Approvals. + EventApprovalRequested EventType = "APPROVAL_REQUESTED" + EventApprovalEscalated EventType = "APPROVAL_ESCALATED" + + // Auth / credentials. + EventExternalCredentialsRotated EventType = "EXTERNAL_CREDENTIALS_ROTATED" +) + +// AllEventTypes lists every constant in this file. Used by the +// snapshot test (events_types_drift_test.go) to detect when a new +// constant is added without updating the snapshot — the catch-up +// step is mirroring the addition into canvas/src/lib/ws-events.ts so +// canvas consumers can switch on it. +// +// Keep in lexicographic order so the snapshot diff is stable on +// renames and the parity-with-TS comparison is order-independent. +var AllEventTypes = []EventType{ + EventA2AResponse, + EventActivityLogged, + EventAgentAssigned, + EventAgentCardUpdated, + EventAgentMessage, + EventAgentMoved, + EventAgentRemoved, + EventAgentReplaced, + EventApprovalEscalated, + EventApprovalRequested, + EventChannelMessage, + EventCronExecuted, + EventCronSkipped, + EventDelegationComplete, + EventDelegationFailed, + EventDelegationSent, + EventDelegationStatus, + EventExternalCredentialsRotated, + EventTaskUpdated, + EventWorkspaceAwaitingAgent, + EventWorkspaceDegraded, + EventWorkspaceHeartbeat, + EventWorkspaceHibernated, + EventWorkspaceOffline, + EventWorkspaceOnline, + EventWorkspacePaused, + EventWorkspaceProvisionFailed, + EventWorkspaceProvisioning, + EventWorkspaceRemoved, +} diff --git a/workspace-server/internal/events/types_test.go b/workspace-server/internal/events/types_test.go new file mode 100644 index 00000000..a16e2e8b --- /dev/null +++ b/workspace-server/internal/events/types_test.go @@ -0,0 +1,117 @@ +package events + +import ( + "sort" + "strings" + "testing" +) + +// TestAllEventTypes_IsSnapshot pins the canonical event taxonomy. +// Adding a new constant in types.go without updating AllEventTypes +// (or vice versa) fails this test. +// +// The snapshot is also the authoritative input to the canvas-side +// parity gate (PR-B-2 follow-up): the TypeScript union members in +// canvas/src/lib/ws-events.ts MUST match this list exactly. A drift +// gate at CI time will assert set equality once the TS file lands. +func TestAllEventTypes_IsSnapshot(t *testing.T) { + // Every named constant must appear in AllEventTypes. Walk via + // reflection over the package-level vars would over-include test + // fixtures, so list the canonical names here. When a constant + // is added in types.go, append the EventType's literal value + // to the expected list below — the failure message names + // exactly what's missing so the diff is one-line obvious. + expected := []string{ + "A2A_RESPONSE", + "ACTIVITY_LOGGED", + "AGENT_ASSIGNED", + "AGENT_CARD_UPDATED", + "AGENT_MESSAGE", + "AGENT_MOVED", + "AGENT_REMOVED", + "AGENT_REPLACED", + "APPROVAL_ESCALATED", + "APPROVAL_REQUESTED", + "CHANNEL_MESSAGE", + "CRON_EXECUTED", + "CRON_SKIPPED", + "DELEGATION_COMPLETE", + "DELEGATION_FAILED", + "DELEGATION_SENT", + "DELEGATION_STATUS", + "EXTERNAL_CREDENTIALS_ROTATED", + "TASK_UPDATED", + "WORKSPACE_AWAITING_AGENT", + "WORKSPACE_DEGRADED", + "WORKSPACE_HEARTBEAT", + "WORKSPACE_HIBERNATED", + "WORKSPACE_OFFLINE", + "WORKSPACE_ONLINE", + "WORKSPACE_PAUSED", + "WORKSPACE_PROVISIONING", + "WORKSPACE_PROVISION_FAILED", + "WORKSPACE_REMOVED", + } + sort.Strings(expected) + + actual := make([]string, 0, len(AllEventTypes)) + for _, e := range AllEventTypes { + actual = append(actual, string(e)) + } + sort.Strings(actual) + + if len(actual) != len(expected) { + t.Errorf("AllEventTypes count = %d, want %d\nactual: %s\nexpected: %s", + len(actual), len(expected), + strings.Join(actual, ", "), + strings.Join(expected, ", ")) + return + } + for i, want := range expected { + if actual[i] != want { + t.Errorf("AllEventTypes[%d] = %q, want %q (full diff:\n actual: %v\n expected: %v\n)", + i, actual[i], want, actual, expected) + } + } +} + +// TestEventType_NoEmptyConstants pins that no constant declared in +// types.go has an accidentally-empty value. The catch is the +// "WORKSPACE_X" → forgot-to-fill pattern: a typo in the literal +// would surface as the empty string, and broadcast pipelines would +// silently filter empty-name events without any error signal. +func TestEventType_NoEmptyConstants(t *testing.T) { + for _, e := range AllEventTypes { + if string(e) == "" { + t.Errorf("found empty EventType in AllEventTypes — typo in types.go?") + } + } +} + +// TestEventType_AllUppercaseSnakeCase pins the wire format. Mixed +// case or kebab-case would break the canvas TypeScript switch +// statements (every consumer's `case "AGENT_MESSAGE":` is upper- +// snake). The check is the catch for an accidental +// `"agent_message"` typo that wouldn't fail the snapshot gate. +func TestEventType_AllUppercaseSnakeCase(t *testing.T) { + for _, e := range AllEventTypes { + s := string(e) + // Allowed chars: A-Z, 0-9, _ — nothing else, no leading/ + // trailing underscores, no consecutive underscores. + if s != strings.ToUpper(s) { + t.Errorf("EventType %q is not all-uppercase — wire format requires upper-snake", s) + } + if strings.HasPrefix(s, "_") || strings.HasSuffix(s, "_") { + t.Errorf("EventType %q has leading/trailing underscore — disallowed", s) + } + if strings.Contains(s, "__") { + t.Errorf("EventType %q has consecutive underscores — disallowed", s) + } + for _, r := range s { + if !((r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '_') { + t.Errorf("EventType %q contains disallowed char %q", s, r) + break + } + } + } +} diff --git a/workspace-server/internal/handlers/agent_message_writer.go b/workspace-server/internal/handlers/agent_message_writer.go index a0f25588..dbbce3e7 100644 --- a/workspace-server/internal/handlers/agent_message_writer.go +++ b/workspace-server/internal/handlers/agent_message_writer.go @@ -160,7 +160,7 @@ func (w *AgentMessageWriter) Send( if len(attachments) > 0 { broadcastPayload["attachments"] = attachments } - w.broadcaster.BroadcastOnly(workspaceID, "AGENT_MESSAGE", broadcastPayload) + w.broadcaster.BroadcastOnly(workspaceID, string(events.EventAgentMessage), broadcastPayload) // 3. Persist for chat-history hydration. response_body shape MUST stay // in sync with extractResponseText + extractFilesFromTask in