-
Notifications
You must be signed in to change notification settings - Fork 25
Simplify HTTP Plugin architecture #424
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
heyitsaamir
wants to merge
17
commits into
main
Choose a base branch
from
aamirj/simlifyHttp
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
f07e077 to
d9a529e
Compare
lilyydu
reviewed
Jan 21, 2026
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 <[email protected]>
2fa2032 to
4590e50
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Separate activity sending from HTTP transport layer
The previous architecture tightly coupled HTTP transport concerns with activity sending logic:
Previous Architecture:
There are a few issues with this:
New Architecture
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
ActivitySenderclass. 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
2. Introduced ICoreActivity Interface
3. Removed ISender Interface
Breaking Changes
For Plugin Authors:
PR Dependency Tree
This tree was auto-generated by Charcoal