Skip to content

Commit 8637c59

Browse files
committed
fix(core): treat relatedRequestId 0 as present in debounce guard
`0` is a valid JSON-RPC RequestId (and is also the first id a Protocol instance uses). The debounce guard in `Protocol.notification` used a truthy check `!options?.relatedRequestId` which treated 0 as absent, and incorrectly coalesced related notifications that should have bypassed debouncing. Compare the asymmetry: | relatedRequestId | debounced? (pre-fix) | | 0 | YES (bug) | | 1 | no | | "0" | no | Switch to `options?.relatedRequestId == null` - catches anull/undefined, preserves 0 as a valid id. `relatedTask` is a structured object and cannot be falsy, so its guard is unchanged. Closes #2117.
1 parent 48251fe commit 8637c59

2 files changed

Lines changed: 20 additions & 1 deletion

File tree

packages/core/src/shared/protocol.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1010,7 +1010,10 @@ export abstract class Protocol<ContextT extends BaseContext> {
10101010
// A notification can only be debounced if it's in the list AND it's "simple"
10111011
// (i.e., has no parameters and no related request ID or related task that could be lost).
10121012
const canDebounce =
1013-
debouncedMethods.includes(notification.method) && !notification.params && !options?.relatedRequestId && !options?.relatedTask;
1013+
debouncedMethods.includes(notification.method) &&
1014+
!notification.params &&
1015+
options?.relatedRequestId == null &&
1016+
!options?.relatedTask;
10141017

10151018
if (canDebounce) {
10161019
// If a notification of this type is already scheduled, do nothing.

packages/core/test/shared/protocol.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -768,6 +768,22 @@ describe('protocol tests', () => {
768768
expect(sendSpy).toHaveBeenCalledWith(expect.any(Object), { relatedRequestId: 'req-2' });
769769
});
770770

771+
it('should NOT debounce a notification that has relatedRequestId 0 (regression for #2117)', async () => {
772+
// ARRANGE: id=0 is a valid JSON-RPC RequestId (and the first id a Protocol uses).
773+
// The truthy guard \`!options?.relatedRequestId\` treated 0 as absent and
774+
// incorrectly debounced these related notifications. Pin the fixed behaviour.
775+
protocol = new TestProtocolImpl({ debouncedNotificationMethods: ['test/debounced_with_options'] });
776+
await protocol.connect(transport);
777+
778+
// ACT
779+
await protocol.notification({ method: 'test/debounced_with_options' }, { relatedRequestId: 0 });
780+
await protocol.notification({ method: 'test/debounced_with_options' }, { relatedRequestId: 0 });
781+
782+
// ASSERT: both calls must reach the transport (no coalescing).
783+
expect(sendSpy).toHaveBeenCalledTimes(2);
784+
expect(sendSpy).toHaveBeenCalledWith(expect.any(Object), { relatedRequestId: 0 });
785+
});
786+
771787
it('should clear pending debounced notifications on connection close', async () => {
772788
// ARRANGE
773789
protocol = new TestProtocolImpl({ debouncedNotificationMethods: ['test/debounced'] });

0 commit comments

Comments
 (0)