Skip to content

Conversation

@heyitsaamir
Copy link
Collaborator

@heyitsaamir heyitsaamir commented Dec 16, 2025

Separate activity sending from HTTP transport layer

The previous architecture tightly coupled HTTP transport concerns with activity sending logic:

Previous Architecture:

HttpPlugin (transport) → implements ISender (sending)
                      → has send() method (creates new Client per call)
                      → has createStream() method
                      → knows about Activity protocol details

ActivityContext → depends on ISender plugin
               → cannot work without transport plugin
               → conflates transport and sending concerns

There are a few issues with this:

  • HttpPlugin created NEW Client instances on every send() call. So there's really no benefit of this logic being in the "httpclient" plugin.
  • Transport plugins (HttpPlugin) were forced to implement send/createStream. This makes it more cumbersome to build your own HttpPlugin with your own servier.
  • Users couldn't "bring their own server" without implementing ISender
  • ActivityContext was tightly coupled to plugin architecture. ("Sender" was coupled with an activity, without any necessary benefits.)

New Architecture

HttpPlugin (transport) → only handles HTTP server/routing/auth
                      → emits ICoreActivity (minimal protocol knowledge)
                      → just passes body payload to app

ActivitySender (NEW)  → dedicated class for sending activities
                     → receives injected, reusable Client
                     → handles all send/stream logic
                     → private to App class

ActivityContext       → uses send callback (abstraction)
                     → receives pre-created stream
                     → no direct dependency on ActivitySender

In this PR, I am mainly decoupling responsibilities of HttpPlugin from being BOTH a listener AND a sender, to being just a listener. The sender bit is now separated to a different ActivitySender class. Other than better code organization, the main thing this lets us do is not require the app to run to be able to send proactive messages. This is a huge plus point because now the App can be used in scenarios where it doesn't necessarily need to listen to incoming messages (like agentic notifications!)

Major Decisions

1. Created ActivitySender Class

  • Centralized all activity sending logic
  • Receives reusable Client in constructor (no per-send instantiation)
  • Private to App class - internal implementation detail
  • Provides send() and createStream() methods
  • Separate from HttpPlugin

2. Introduced ICoreActivity Interface

  • Minimal fields transport layer needs: serviceUrl, id, type
  • Extensible via [key: string]: any for protocol-specific fields
  • Transport plugins work with this instead of full Activity type. So it's easier to create these.
  • Parsing to Activity happens in app.process.ts now, NOT in HttpPlugin.

3. Removed ISender Interface

  • No longer needed - plugins don't send activities
  • Plugins only handle transport (receiving requests)
  • Breaking change, but simplifies plugin architecture. This pattern wasn't documented (intentionally) because the design was subject to change. So it should be okay hopefully to change this.

Breaking Changes

For Plugin Authors:

  1. ISender removed - Custom plugins should implement IPlugin only
  2. IActivityEvent changed - Now has body: ICoreActivity instead of activity: Activity

PR Dependency Tree

This tree was auto-generated by Charcoal

@heyitsaamir heyitsaamir changed the title Aamirj/simlify http Simplify HTTP Plugin architecture Dec 16, 2025
@heyitsaamir heyitsaamir marked this pull request as ready for review January 16, 2026 06:58
* Activity type for basic routing
*/
sender: ISender;
type?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of string, should we be more specific here? or wait till it passes into app.process

Copy link
Collaborator Author

@heyitsaamir heyitsaamir Jan 23, 2026

Choose a reason for hiding this comment

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

yeah, similar to the core in dotnet, this layer should be as unopinionated as possible. So ideally it doesn't really even know about Activity

heyitsaamir and others added 7 commits January 23, 2026 10:05
The previous architecture tightly coupled HTTP transport concerns with activity sending logic:

**Previous Architecture:**
```
HttpPlugin (transport) → implements ISender (sending)
                      → has send() method (creates new Client per call)
                      → has createStream() method
                      → knows about Activity protocol details

ActivityContext → depends on ISender plugin
               → cannot work without transport plugin
               → conflates transport and sending concerns
```

Key Issues:
- HttpPlugin created NEW Client instances on every send() call
- Transport plugins (HttpPlugin) were forced to implement send/createStream
- Users couldn't "bring their own server" without implementing ISender
- ActivityContext was tightly coupled to plugin architecture
- HttpPlugin knew about Bot Framework Activity protocol details

```
HttpPlugin (transport) → only handles HTTP server/routing/auth
                      → emits ICoreActivity (minimal protocol knowledge)
                      → just passes body payload to app

ActivitySender (NEW)  → dedicated class for sending activities
                     → receives injected, reusable Client
                     → handles all send/stream logic
                     → private to App class

ActivityContext       → uses send callback (abstraction)
                     → receives pre-created stream
                     → no direct dependency on ActivitySender
```

- Centralized all activity sending logic
- Receives reusable Client in constructor (no per-send instantiation)
- Private to App class - internal implementation detail
- Provides send() and createStream() methods

- Minimal fields transport layer needs: serviceUrl, id, type
- Extensible via [key: string]: any for protocol-specific fields
- Transport plugins work with this instead of full Activity type
- Parsing to Activity happens in app.process.ts

- No longer needed - plugins don't send activities
- Plugins only handle transport (receiving requests)
- Breaking change, but simplifies plugin architecture

- Constructor accepts send callback function
- Receives pre-created stream (not factory function)
- No knowledge of ActivitySender implementation
- Proper abstraction via dependency injection

- Initially renamed to ActivityStream
- Reverted because it's still HTTP-specific (uses Bot Framework HTTP Client API)
- Moved from src/plugins/http/stream.ts to src/http-stream.ts
- Still transport-specific, just not plugin-owned

1. **ISender removed** - Custom plugins should implement IPlugin only
2. **IActivityEvent changed** - Now has body: ICoreActivity instead of activity: Activity
3. **Plugin.onActivity** - Still receives parsed activity: Activity (unchanged)
4. **App.process signature** - Internal change, not exposed to plugin API

Before:
```typescript
class MyPlugin implements ISender {
  send(activity, ref) { ... }        // Required
  createStream(ref) { ... }          // Required

  async onRequest(req, res) {
    const activity: Activity = req.body;  // Need Activity type
    await this.$onActivity({ activity, token });
  }
}
```

After:
```typescript
class MyPlugin implements IPlugin {
  // No send() or createStream() needed!

  async onRequest(req, res) {
    // Just pass body - don't need Activity type knowledge
    await this.$onActivity({
      body: req.body,  // ICoreActivity (minimal fields)
      token
    });
  }
}
```

1. **Client Reuse** - ActivitySender reuses same Client, no per-send instantiation
2. **Separation of Concerns** - Transport vs sending clearly separated
3. **Bring Your Own Server** - Easy to implement custom transports (Socket.io, gRPC, etc.)
4. **Less Protocol Knowledge** - Transport layer only needs ICoreActivity, not full Activity
5. **Cleaner Architecture** - Each class has single responsibility
6. **Better Abstraction** - ActivityContext uses callbacks, not direct dependencies

- src/activity-sender.ts - Dedicated activity sending class
- src/http-stream.ts - Moved from src/plugins/http/stream.ts

- src/app.ts - Added activitySender, updated send()
- src/app.process.ts - Removed sender param, uses activitySender
- src/plugins/http/plugin.ts - Removed send/createStream, works with ICoreActivity
- src/contexts/activity.ts - Uses callbacks instead of ISender plugin
- src/events/activity.ts - Added ICoreActivity, changed to body field
- src/types/plugin/sender.ts - Removed ISender, kept IActivitySender

- src/plugins/http/stream.ts - Moved to src/http-stream.ts
- ISender interface - Completely removed
- Add validation for missing activity body in app.process.ts
- Add try-catch in HttpPlugin.onRequest to prevent server hanging on errors
- Fix DevtoolsPlugin to properly handle promise rejections
- Add default type fallback in devtools activity creation route

Fixes issue where server would become unresponsive after activity processing errors.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
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.

3 participants