Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03da94aa77
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if config.memory_tools { | ||
| builder.push_spec(create_get_memory_tool()); | ||
| builder.register_handler("get_memory", get_memory_handler); |
There was a problem hiding this comment.
Gate get_memory on Sqlite or return explicit error
When memory_tools is enabled, the tool is always registered, but it relies on the SQLite state DB. If Feature::Sqlite is off (the default), session.state_db() is None and state_db::get_thread_memory(...) returns None, so the handler reports “memory not found” even though storage is disabled. This makes the tool unusable and misleading unless callers also enable Sqlite. Consider gating registration on Feature::Sqlite (or emitting a clear error when the DB is unavailable) so users don’t see false “not found” failures in that configuration.
Useful? React with 👍 / 👎.
Add a tool for memory to retrieve a full memory based on the memory ID