Skip to content

Conversation

@friggeri
Copy link
Collaborator

Add overloaded on() method that accepts an event type string as the first argument, enabling type-safe event subscriptions:

  session.on('assistant.message', (event) => {
    // event is typed as SessionEventPayload<'assistant.message'>
    console.log(event.data.content);
  });

The original on(handler) signature remains supported for wildcard subscriptions.

Changes:

  • Add SessionEventType, SessionEventPayload, TypedSessionEventHandler types
  • Update CopilotSession.on() with typed overload
  • Update _dispatchEvent() to dispatch to typed handlers
  • Export new types from index.ts
  • Update documentation with new usage patterns

Copilot AI review requested due to automatic review settings January 29, 2026 22:53
@friggeri friggeri requested a review from a team as a code owner January 29, 2026 22:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds typed event filtering to the CopilotSession.on() method, enabling developers to subscribe to specific event types with full TypeScript type inference. The implementation maintains backward compatibility with the existing wildcard subscription pattern.

Changes:

  • Added type definitions (SessionEventType, SessionEventPayload, TypedSessionEventHandler) to support typed event subscriptions
  • Overloaded the on() method to accept either a specific event type with typed handler, or a wildcard handler for all events
  • Updated documentation and examples to demonstrate the new typed event subscription pattern

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
nodejs/src/types.ts Adds new type definitions for event type strings, payload extraction, and typed event handlers
nodejs/src/session.ts Implements overloaded on() method and updates event dispatch logic to support typed handlers
nodejs/src/index.ts Exports the new type definitions for public API usage
nodejs/README.md Updates API documentation and examples to show typed event subscription patterns
docs/getting-started.md Updates getting started examples to use the new typed event handler syntax
Comments suppressed due to low confidence (3)

nodejs/src/session.ts:267

  • If the handler parameter is undefined at runtime (which could occur through type assertions, JavaScript usage, or other bypasses of TypeScript's type system), the check on line 248 would fail, causing the code to fall through to line 263 where it would treat the string eventType as a SessionEventHandler. This would add a string to eventHandlers, leading to a runtime error when _dispatchEvent tries to invoke it as a function.

Consider adding explicit validation: if (typeof eventTypeOrHandler === "string" && handler === undefined) { throw new Error("Handler is required when subscribing to specific event types"); } before the fallthrough to the wildcard handler logic. This provides a clearer error message for this edge case.

                this.typedEventHandlers.set(eventType, new Set());
            }
            this.typedEventHandlers.get(eventType)!.add(handler);
            return () => {
                const handlers = this.typedEventHandlers.get(eventType);
                if (handlers) {
                    handlers.delete(handler);
                }
            };
        }

        // Overload 2: on(handler) - wildcard subscription
        const wildcardHandler = eventTypeOrHandler as SessionEventHandler;
        this.eventHandlers.add(wildcardHandler);
        return () => {
            this.eventHandlers.delete(wildcardHandler);
        };
    }

    /**

nodejs/src/session.ts:268

  • The new typed event handler functionality (calling on() with a specific event type) lacks test coverage. While tests exist for the wildcard handler pattern, there are no tests verifying:
  1. Typed handlers receive only events of the specified type
  2. Type inference works correctly for typed handlers
  3. The unsubscribe function works correctly for typed handlers
  4. Both typed and wildcard handlers can coexist and both receive events appropriately

Consider adding unit tests covering these scenarios to ensure the new feature works as expected.

    ): () => void {
        // Overload 1: on(eventType, handler) - typed event subscription
        if (typeof eventTypeOrHandler === "string" && handler) {
            const eventType = eventTypeOrHandler;
            if (!this.typedEventHandlers.has(eventType)) {
                this.typedEventHandlers.set(eventType, new Set());
            }
            this.typedEventHandlers.get(eventType)!.add(handler);
            return () => {
                const handlers = this.typedEventHandlers.get(eventType);
                if (handlers) {
                    handlers.delete(handler);
                }
            };
        }

        // Overload 2: on(handler) - wildcard subscription
        const wildcardHandler = eventTypeOrHandler as SessionEventHandler;
        this.eventHandlers.add(wildcardHandler);
        return () => {
            this.eventHandlers.delete(wildcardHandler);
        };
    }

    /**
     * Dispatches an event to all registered handlers.

nodejs/src/session.ts:282

  • The type assertion event as SessionEventPayload<typeof event.type> is not correctly narrowing the type. The typeof event.type expression evaluates to string rather than the specific literal type (e.g., "assistant.message"). This means TypeScript won't properly narrow the event type for handlers.

Since the event is already the correct discriminated union type (SessionEvent), and you're dispatching it to handlers that were registered for event.type, the event should already be type-safe. The cast is unnecessary - you can simply pass event directly to the handler.

                }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 59 to 60
// eslint-disable-next-line @typescript-eslint/no-explicit-any
private typedEventHandlers: Map<SessionEventType, Set<(event: any) => void>> = new Map();
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of any type for handler storage (Map<SessionEventType, Set<(event: any) => void>>) bypasses TypeScript's type safety. While this might seem necessary for storing handlers of different types in the same Map, it undermines the type safety that the typed event system is designed to provide.

Consider using a more type-safe approach, such as defining the Map as Map<string, Set<(event: SessionEvent) => void>> since the handlers will receive the full SessionEvent anyway (after the type cast issue on line 282 is fixed). This maintains type safety while still allowing storage of handlers for different event types.

Suggested change
// eslint-disable-next-line @typescript-eslint/no-explicit-any
private typedEventHandlers: Map<SessionEventType, Set<(event: any) => void>> = new Map();
private typedEventHandlers: Map<SessionEventType, Set<(event: SessionEvent) => void>> =
new Map();

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

Cross-SDK Consistency Review

This PR adds a valuable developer experience improvement to the Node.js SDK by introducing typed event filtering via an overloaded on() method. However, this creates a feature parity gap with the other SDK implementations.

Summary

Node.js/TypeScript (after this PR):

  • session.on(handler) - wildcard subscription
  • session.on(eventType, handler) - typed event filtering ⭐ NEW

Python, Go, and .NET (current state):

  • Only wildcard subscription: session.on(handler) / Session.On(handler)
  • No typed event filtering capability

Impact

This enhancement significantly improves the Node.js developer experience by:

  1. Better type safety - TypeScript can infer the exact event payload type
  2. Cleaner code - No need for manual if (event.type === ...) checks
  3. Better IDE support - Autocomplete for event types and event properties

However, developers using Python, Go, or .NET will need to continue using the manual filtering pattern:

# Python developers still need this pattern:
session.on(lambda event: 
    process_delta(event.data.delta_content) 
    if event.type == "assistant.message_delta" 
    else None
)

Recommendation

Consider implementing equivalent typed event filtering in the other SDKs to maintain feature parity:

Python: Could use method overloading or a separate on_event() method:

session.on("assistant.message", lambda event: print(event.data.content))

Go: Could use a generic typed subscription method:

copilot.On[copilot.AssistantMessageEvent](session, func(event copilot.AssistantMessageEvent) {
    fmt.Println(event.Data.Content)
})

C#/.NET: Could leverage event types for filtering:

session.On(AssistantMessageEvent)(evt => Console.WriteLine(evt.Data?.Content));

This is not a blocking issue, but tracking this gap will help maintain consistent developer experience across all SDK languages. Great feature for Node.js! 🎉

AI generated by SDK Consistency Review Agent

Add overloaded on() method that accepts an event type string as the first
argument, enabling type-safe event subscriptions:

  session.on('assistant.message', (event) => {
    // event is typed as SessionEventPayload<'assistant.message'>
    console.log(event.data.content);
  });

The original on(handler) signature remains supported for wildcard subscriptions.

Changes:
- Add SessionEventType, SessionEventPayload, TypedSessionEventHandler types
- Update CopilotSession.on() with typed overload
- Update _dispatchEvent() to dispatch to typed handlers
- Export new types from index.ts
- Update documentation with new usage patterns
@friggeri friggeri force-pushed the frg/typed-session-event-handlers branch from 5764124 to f3514a0 Compare January 29, 2026 23:01
@github-actions
Copy link

Cross-SDK Consistency Review

Summary

This PR adds typed event filtering to the Node.js/TypeScript SDK, introducing an overloaded session.on() method that accepts a specific event type string as the first parameter. This enables type-safe event subscriptions with full TypeScript type inference.

Current State Across SDKs

✅ Node.js/TypeScript (This PR)

After this PR:

// New: Type-safe event filtering
session.on("assistant.message", (event) => {
    console.log(event.data.content); // TypeScript knows event.data structure
});

// Existing: Wildcard subscription still supported
session.on((event) => {
    if (event.type === "assistant.message") { ... }
});

❌ Python SDK

Current state:

# Only supports wildcard subscription
def handler(event: SessionEvent) -> None:
    if event.type == SessionEventType.ASSISTANT_MESSAGE:
        print(event.data.content)

session.on(handler)

Observation: Python SDK does NOT have typed event filtering. All event subscriptions receive all events and must manually filter by event.type.

❌ Go SDK

Current state:

// Only supports wildcard subscription
session.On(func(event copilot.SessionEvent) {
    switch event.Type {
    case copilot.AssistantMessage:
        fmt.Println(event.Data.Content)
    }
})

Observation: Go SDK does NOT have typed event filtering. All handlers receive all events and use switch statements to filter.

❌ .NET SDK

Current state:

// Only supports wildcard subscription with pattern matching
session.On(evt =>
{
    if (evt is AssistantMessageEvent assistantMessage)
    {
        Console.WriteLine(assistantMessage.Data?.Content);
    }
});

Observation: .NET SDK does NOT have typed event filtering. It uses C# pattern matching for event discrimination.

Consistency Assessment

🟡 Feature Parity Gap Detected

This PR introduces a developer experience enhancement exclusive to the Node.js SDK. While the change is TypeScript-specific and leverages language features unavailable in other languages, the API design pattern could be adapted to other SDKs using their respective language idioms.

Language-Appropriate Adaptations

Each language could implement similar event filtering with their native patterns:

Python (suggested):

# Option 1: Using literal strings (simpler)
session.on("assistant.message", lambda event: print(event.data.content))

# Option 2: Using enum values (more type-safe)
session.on(SessionEventType.ASSISTANT_MESSAGE, lambda event: print(event.data.content))

Go (suggested):

// Using constant strings
session.OnEvent(copilot.AssistantMessage, func(event copilot.SessionEvent) {
    fmt.Println(event.Data.Content)
})

.NET (suggested):

// Using generic overload
session.On(AssistantMessageEvent)(evt => 
{
    Console.WriteLine(evt.Data?.Content);
});

// Or using string literal
session.On("assistant.message", (AssistantMessageEvent evt) => 
{
    Console.WriteLine(evt.Data?.Content);
});

Recommendation

✅ This PR is acceptable as-is

The changes are well-designed and maintain backward compatibility. The Node.js SDK is leveraging TypeScript's advanced type system features.

💡 Consider follow-up work

For feature parity and consistent developer experience across all SDKs, consider:

  1. Python SDK: Add an overloaded on() method accepting event type strings or enum values
  2. Go SDK: Add an OnEvent() method or similar that accepts event type constants
  3. .NET SDK: Add a generic On(TEvent)() overload for type-safe event filtering

These enhancements would provide a more ergonomic API across all languages while respecting each language's idioms and type system capabilities.

📝 Documentation Note

The getting-started guide has been updated to showcase the new typed event handlers, which is excellent. If other SDKs implement similar features, their documentation should be updated similarly to maintain consistency in the learning experience.


Verdict:No blocking issues. This is a TypeScript-specific enhancement that improves developer experience. Consider adding equivalent functionality to other SDKs to maintain API design consistency (accounting for language differences).

AI generated by SDK Consistency Review Agent

@friggeri friggeri merged commit 0b004e0 into main Jan 29, 2026
15 checks passed
@friggeri friggeri deleted the frg/typed-session-event-handlers branch January 29, 2026 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants