Uh oh!
There was an error while loading. Please reload this page.
fix(types): declare the server telemetry events the engine already emits - #297
Conversation
Fourteen `relaycast_server_*` names were emitted by the engine but missing from `SERVER_TELEMETRY_EVENTS`. `parseInternalTelemetryEvent` validates the name against a zod enum built from that list, and the hosted sink invokes it as a floating `void capture...(...)`, so each one threw into an unhandled rejection and was dropped before reaching PostHog — silently, for actions, routing, the agent directory, channel mute/unmute, and inbound Relayfile delivery. `relaycast_server_action_invoked` was among them. Adds the missing names with conservative required-property lists (only fields present at every emit site, since a missing required property drops the event the same way), and a guard test in the engine that diffs every `emitServerEvent` call site against the catalog so a new emit site can't go invisible again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxUMkEa2foSTbw88sqEFWA
Warning Review limit reached
Next review available in:49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe server telemetry catalog now declares 14 additional engine-emitted events with required payload properties. A Vitest suite scans engine emission sites to ensure emitted event names remain catalogued, and changelogs document the fix. ChangesServer telemetry catalog coverage
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/types/CHANGELOG.md`:
- Line 18: Shorten the telemetry release-note bullet to one concise,
impact-first statement describing the affected server action, directory, route,
channel-muted, and relayfile telemetry domains without catalog, parser,
validation, or sink details. Apply the same causal-chain reduction at
packages/types/CHANGELOG.md:18-18 and CHANGELOG.md:34-34, preserving only the
shipped user-visible impact at both sites.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 24d91597-45b4-4fcf-b9d6-6a5beed5be84
📒 Files selected for processing (4)
CHANGELOG.mdpackages/engine/src/lib/__tests__/serverTelemetryCatalog.test.tspackages/types/CHANGELOG.mdpackages/types/src/telemetry.ts
| ### Fixed | ||
| - `SERVER_TELEMETRY_EVENTS` now declares the 14 event names the engine already emits (`relaycast_server_action_*`, `relaycast_server_directory_*`, `relaycast_server_route*`, `relaycast_server_channel_muted` / `_unmuted`, `relaycast_server_relayfile_inbound_delivered`). `parseInternalTelemetryEvent` rejected them as unknown, so every one was dropped instead of captured. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Shorten the telemetry release note. Keep this as one impact-first bullet; omit catalog, parser, validation, and sink implementation details.
packages/types/CHANGELOG.md#L18-L18: Replace the detailed internal explanation with a concise impact statement.CHANGELOG.md#L34-L34: Trim the causal chain while retaining the affected telemetry domains.
As per coding guidelines, changelog entries should be “one short, impact-first bullet per user-visible change” and omit internal details unless they explain shipped impact.
📍 Affects 2 files
packages/types/CHANGELOG.md#L18-L18(this comment)CHANGELOG.md#L34-L34
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/types/CHANGELOG.md` at line 18, Shorten the telemetry release-note
bullet to one concise, impact-first statement describing the affected server
action, directory, route, channel-muted, and relayfile telemetry domains without
catalog, parser, validation, or sink details. Apply the same causal-chain
reduction at packages/types/CHANGELOG.md:18-18 and CHANGELOG.md:34-34,
preserving only the shipped user-visible impact at both sites.
Source: Coding guidelines
The root changelog is the cross-package, user-facing narrative. This change is single-package (the engine side is test-only, which AGENTS.md says to omit) and no self-hoster or hosted user sees a behavior difference — it is hosted-analytics plumbing. The entry also carried the implementation backstory and referenced the hosted sink, which lives in another repo. Keeps one concise bullet in the types changelog, under Added since what ships there is the widened export surface. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxUMkEa2foSTbw88sqEFWA
Main refiled the pending 6.0.4–6.2.0 entries under release headings (#298) and added scripts/cut-changelog.mjs (#299). The textual merge landed this branch's types entry inside the now-released [6.2.0] block, which would have credited an unreleased change to a shipped version — moved it back under [Unreleased], and raised that heading from Patch to Minor since widening an exported const/union is additive. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxUMkEa2foSTbw88sqEFWA
Summary
Fourteen
relaycast_server_*event names are emitted by the engine but missing fromSERVER_TELEMETRY_EVENTS.parseInternalTelemetryEventvalidates the name against a zod enum built from that list, and the hosted sink invokes the capture as a floatingvoid capture...(...)— so each of these threw into an unhandled rejection and was dropped before reaching PostHog, silently and with nothing logged.The affected events:
relaycast_server_action_invokedis among them, which means action usage — arguably the clearest signal that a workspace is doing real work — has never been visible in analytics.Changes
packages/types/src/telemetry.ts: add the 14 names toSERVER_TELEMETRY_EVENTS, withREQUIRED_SERVER_EVENT_PROPSentries. The required lists are deliberately conservative — only properties present at every emit site — because a missing required property throws and drops the event through the exact same path this PR is fixing. (emitServerEventalways injectsworkspace_id, so that one is always safe.)packages/engine/src/lib/__tests__/serverTelemetryCatalog.test.ts: a guard test that scans everyemitServerEventcall site in the engine and asserts its event name is declared. A second assertion fails if a call shape changes such that the guard can no longer read the name, so it can't silently stop guarding.Not included, flagged for a follow-up rather than widened into this PR:
command_registered/command_deleted/command_invokedandworkspace_stream_updatedare declared but emitted by nothing (thecommand_*trio looks like the pre-rename ancestor ofaction_*). Deciding whether to delete or wire those up is a separate call.Test Plan
relaycast_server_action_invokedfrom the catalog fails the test with that exact name, and passes again when restorednpx turbo test --filter=@relaycast/types --filter=@relaycast/engine— 511 tests passingnpx turbo buildfor both packagesGenerated by Claude Code