Fix ACP CLI memory bootstrap#892
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a _bootstrap_acp_memory function in the ACP server to handle memory and middleware initialization, along with a new test case to verify the setup. A review comment suggests refactoring this logic into a shared utility to avoid code duplication with the existing CLI bootstrapping process.
| def _bootstrap_acp_memory(*, init_middlewares_fn: Any | None = None) -> None: | ||
| from aworld.memory.main import _default_file_memory_store | ||
| from aworld_cli.memory import bootstrap as memory_bootstrap | ||
| from aworld_cli.runtime_bootstrap import _supports_memory_config | ||
|
|
||
| if init_middlewares_fn is None: | ||
| from aworld.core.context.amni.config import init_middlewares | ||
|
|
||
| init_middlewares_fn = init_middlewares | ||
|
|
||
| memory_bootstrap.register_cli_memory_provider() | ||
| memory_config = memory_bootstrap.build_cli_memory_config() | ||
| init_middlewares_kwargs = dict( | ||
| init_memory=True, | ||
| init_retriever=False, | ||
| custom_memory_store=_default_file_memory_store(), | ||
| ) | ||
| if _supports_memory_config(init_middlewares_fn): | ||
| init_middlewares_kwargs["memory_config"] = memory_config | ||
|
|
||
| init_middlewares_fn(**init_middlewares_kwargs) |
There was a problem hiding this comment.
The logic for bootstrapping CLI memory is duplicated from aworld_cli/runtime_bootstrap.py (lines 59-69). This includes registering the memory provider, building the configuration, and initializing middlewares with specific arguments like init_retriever=False.
To improve maintainability and ensure consistency across the CLI and ACP server, consider extracting this logic into a shared utility function (e.g., bootstrap_cli_memory) in a common location like aworld_cli/runtime_bootstrap.py or aworld_cli/memory/bootstrap.py.
Summary
Tests