generated from amazon-archives/__template_Apache-2.0
-
Notifications
You must be signed in to change notification settings - Fork 31
Re-implement conversation-manager as a hook provider #222
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
Merged
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
zastrowm
commented
Nov 20, 2025
src/conversation-manager/__tests__/sliding-window-conversation-manager.test.ts
Outdated
Show resolved
Hide resolved
- Use hooks.addHook() instead of manual registerCallbacks()
- Create createMockAgent() helper in fixtures
- Simplify test helpers to use invokeCallbacks directly
- Update all tests to use createMockAgent helper
- Make all tests properly async with await
Addresses PR review feedback.
refactor: keep conversationManager as first-class config option
- Restore conversationManager to AgentConfig and Agent class
- Agent automatically registers conversation manager hooks internally
- Update tests to use HookRegistryImplementation and focus on behavior
- Remove public mentions of hooks from conversation manager docs
- Simplify NullConversationManager tests to focus on behavior
This maintains conversationManager as a top-level concept while using
hooks internally as an implementation detail.
refactor!: convert conversation managers to hook providers
BREAKING CHANGE: Removed ConversationManager base class and Agent.conversationManager field.
Conversation managers are now hook providers that must be registered via the hooks config.
Changes:
- Added retryModelCall field to AfterModelCallEvent for retry control
- Converted NullConversationManager to HookProvider (0 hooks registered)
- Converted SlidingWindowConversationManager to HookProvider with AfterInvocationEvent and AfterModelCallEvent callbacks
- Removed Agent.conversationManager field and related config
- Updated Agent retry logic to use retryModelCall flag instead of catching ContextWindowOverflowError
- Added messages field to AgentData interface for hook access
- Deleted ConversationManager base class and ConversationContext interface
- Updated public API exports to remove ConversationManager
Migration:
Before: new Agent({ conversationManager: new SlidingWindowConversationManager() })
After: new Agent({ hooks: [new SlidingWindowConversationManager()] })
Resolves: #210
b72faf3 to
11f2138
Compare
- Create MockAgentData interface with optional messages and state - Update createMockAgent to accept interface-based parameter - Update all test usages to use object syntax - Add retryModelCall tests to agent.hook.test.ts - Update test helpers to use registry.addHook(manager) Addresses review comments on PR #222
Unshure
approved these changes
Nov 20, 2025
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.
Resolves: #210
Summary
This PR refactors conversation management to use the hook system as an internal implementation mechanism while maintaining
conversationManageras a first-class configuration option. The key architectural change is that conversation managers now implementHookProviderand register callbacks for lifecycle events, but this is an implementation detail hidden from users who continue to configure conversation management through the dedicatedconversationManagerfield.Motivation
The original implementation had Agent directly calling conversation manager methods at specific points in the execution flow. This created tight coupling between the Agent class and the conversation manager interface. By leveraging the existing hook system, we achieve better separation of concerns while maintaining the same user-facing API.
The hook-based approach provides natural extension points for observability and custom behavior without requiring changes to the core Agent implementation. It also enables future enhancements like composable conversation strategies or pluggable context management policies.
Functionally, there should be no change in behavior from the customer's perspective.
Design Rationale
Conversation management is a fundamental agent capability, not an optional extension. By keeping it as a first-class configuration option, we provide better discoverability, IntelliSense support, and the ability to set sensible defaults without explicit configuration.