Found while adding ClusterStatsPublished in #842.
What is claimed
src/devtools/taps/ClusterTap.ts routes unmatched cluster events through .otherwise into onMemberEvent, whose parameter is an Exclude<ClusterEvent, …> and which dereferences event.member. Two comments in that file state the consequence — onShardRegistrationEvent's says a fall-through "would have typed event.member on something that has none" — and the #842 triage repeated it: adding a non-member-carrying event without an explicit arm is supposed to be a bun run typecheck failure.
What actually happens
Measured on the finished #842 tree:
- Deleting the
ClusterStatsPublished arm and removing the type from onMemberEvent's Exclude: bunx tsc --noEmit exits 0.
tests/unit/devtools/ClusterTap.test.ts under that mutation: 7 pass, 0 fail.- The same, done to
MemberConfigurationMismatch instead: also exit 0. So it is the family's property, not one event's.
Mechanism: Exclude<T, U> removes union members assignable to U. ClusterStatsPublished carries leader: Option<Member>, so it is structurally assignable to LeaderChanged — which is already in the exclude list — and it silently disappears from the parameter type. event.member therefore never has to be explained. Any future event with a field named leader: Option<Member>, or with a superset of any excluded class's fields, hides the same way.
At runtime the fall-through reaches onUnknownEvent(), which is {} — an empty method. So no test can distinguish "handled by an explicit no-op arm" from "fell through the unknown-event path" either.
Net: nothing stops a new ClusterEvent from being routed into a member-event handler it does not belong in, and nothing reports that it happened.
Suggested directions (not decided)
- Make
onUnknownEvent observable — a debug line naming event.constructor.name, or a counter. That is what makes the wrong path detectable, and it is what the path's own comment already implies it should do ("the wrong report").
- Replace the structural
Exclude with an explicit union of the ten member-carrying classes, so a new event has to be named somewhere rather than vanishing by assignability. (The ten are structurally identical, so this is a naming device, not a type-safety one — but it forces the edit.)
- Add a test that enumerates the
ClusterEvent union and asserts every member is either in MEMBER_EVENT_NAMES or has an explicit arm. That is the guard the triage assumed existed; there is no event-inventory test under tests/unit/cluster/ today.
Interim state
#842 keeps the explicit arm, corrects the JSDoc to say the compiler is not the guard here, and pins the mechanism in ClusterTap.test.ts as a compile-checked assignment (const asLeaderChanged: LeaderChanged = new ClusterStatsPublished(…)) that stops compiling the day the shape changes.
Found during the #887 configuration wave, while working on #842.
Found while adding
ClusterStatsPublishedin #842.What is claimed
src/devtools/taps/ClusterTap.tsroutes unmatched cluster events through.otherwiseintoonMemberEvent, whose parameter is anExclude<ClusterEvent, …>and which dereferencesevent.member. Two comments in that file state the consequence —onShardRegistrationEvent's says a fall-through "would have typedevent.memberon something that has none" — and the #842 triage repeated it: adding a non-member-carrying event without an explicit arm is supposed to be abun run typecheckfailure.What actually happens
Measured on the finished #842 tree:
ClusterStatsPublishedarm and removing the type fromonMemberEvent'sExclude:bunx tsc --noEmitexits 0.tests/unit/devtools/ClusterTap.test.tsunder that mutation: 7 pass, 0 fail.MemberConfigurationMismatchinstead: also exit 0. So it is the family's property, not one event's.Mechanism:
Exclude<T, U>removes union members assignable toU.ClusterStatsPublishedcarriesleader: Option<Member>, so it is structurally assignable toLeaderChanged— which is already in the exclude list — and it silently disappears from the parameter type.event.membertherefore never has to be explained. Any future event with a field namedleader: Option<Member>, or with a superset of any excluded class's fields, hides the same way.At runtime the fall-through reaches
onUnknownEvent(), which is{}— an empty method. So no test can distinguish "handled by an explicit no-op arm" from "fell through the unknown-event path" either.Net: nothing stops a new
ClusterEventfrom being routed into a member-event handler it does not belong in, and nothing reports that it happened.Suggested directions (not decided)
onUnknownEventobservable — adebugline namingevent.constructor.name, or a counter. That is what makes the wrong path detectable, and it is what the path's own comment already implies it should do ("the wrong report").Excludewith an explicit union of the ten member-carrying classes, so a new event has to be named somewhere rather than vanishing by assignability. (The ten are structurally identical, so this is a naming device, not a type-safety one — but it forces the edit.)ClusterEventunion and asserts every member is either inMEMBER_EVENT_NAMESor has an explicit arm. That is the guard the triage assumed existed; there is no event-inventory test undertests/unit/cluster/today.Interim state
#842 keeps the explicit arm, corrects the JSDoc to say the compiler is not the guard here, and pins the mechanism in
ClusterTap.test.tsas a compile-checked assignment (const asLeaderChanged: LeaderChanged = new ClusterStatsPublished(…)) that stops compiling the day the shape changes.Found during the #887 configuration wave, while working on #842.