mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
refactor: split memories extension crate modules (#22500)
## Why The memories extension has several distinct responsibilities: registering its prompt and tool contributors, enforcing local-memory filesystem boundaries, implementing list/read/search behavior, and wrapping that backend as extension tools. Those responsibilities were concentrated in `lib.rs`, `local.rs`, and the tool modules, which made follow-up work harder to review and risked growing files through unrelated edits. This PR reorganizes the crate so each responsibility has a narrower owner while preserving the same extension entrypoint and memory tool behavior. ## What Changed - Moved extension lifecycle, prompt, and tool registration into `src/extension.rs`, leaving `src/lib.rs` as the small crate entrypoint. - Split `LocalMemoriesBackend` helpers into `local/list.rs`, `local/path.rs`, `local/read.rs`, and `local/search.rs`. - Centralized tool names and limits at the crate level, and kept the backend and extension implementation crate-private. - Made `memory_list`, `memory_read`, and `memory_search` tool executors generic over `MemoriesBackend`, so tests can exercise the full executor path without depending on tool internals. - Consolidated and expanded memory extension tests in `src/tests.rs`, including read/search tool output coverage, multi-query search, windowed `all_within_lines`, and legacy `query` rejection. ## Testing - Not run locally.
This commit is contained in:
jif-oai authored and GitHub committed
1 parent
3d517fbd00
commit
2edae8d858
13 files changed
+1029
-866
No files matched your search
@@ -8,14 +8,13 @@ use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde_json::json;
|
||||
|
||||
use crate::backend::DEFAULT_LIST_MAX_RESULTS;
|
||||
use crate::DEFAULT_LIST_MAX_RESULTS;
|
||||
use crate::LIST_TOOL_NAME;
|
||||
use crate::MAX_LIST_RESULTS;
|
||||
use crate::backend::ListMemoriesRequest;
|
||||
use crate::backend::ListMemoriesResponse;
|
||||
use crate::backend::MAX_LIST_RESULTS;
|
||||
use crate::backend::MemoriesBackend;
|
||||
use crate::local::LocalMemoriesBackend;
|
||||
|
||||
use super::LIST_TOOL_NAME;
|
||||
use super::backend_error_to_function_call;
|
||||
use super::clamp_max_results;
|
||||
use super::function_tool;
|
||||
@@ -31,11 +30,14 @@ struct ListArgs {
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(super) struct ListTool {
|
||||
pub(super) backend: LocalMemoriesBackend,
|
||||
pub(super) struct ListTool<B> {
|
||||
pub(super) backend: B,
|
||||
}
|
||||
|
||||
impl ExtensionToolExecutor for ListTool {
|
||||
impl<B> ExtensionToolExecutor for ListTool<B>
|
||||
where
|
||||
B: MemoriesBackend,
|
||||
{
|
||||
fn tool_name(&self) -> ToolName {
|
||||
ToolName::plain(LIST_TOOL_NAME)
|
||||
}
|
||||
|
||||
@@ -10,19 +10,18 @@ use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde_json::Value;
|
||||
|
||||
use crate::backend::MemoriesBackend;
|
||||
use crate::backend::MemoriesBackendError;
|
||||
use crate::local::LocalMemoriesBackend;
|
||||
use crate::schema;
|
||||
|
||||
mod list;
|
||||
mod read;
|
||||
mod search;
|
||||
|
||||
const LIST_TOOL_NAME: &str = "memory_list";
|
||||
const READ_TOOL_NAME: &str = "memory_read";
|
||||
const SEARCH_TOOL_NAME: &str = "memory_search";
|
||||
|
||||
pub(crate) fn memory_tools(backend: LocalMemoriesBackend) -> Vec<Arc<dyn ExtensionToolExecutor>> {
|
||||
pub(crate) fn memory_tools<B>(backend: B) -> Vec<Arc<dyn ExtensionToolExecutor>>
|
||||
where
|
||||
B: MemoriesBackend,
|
||||
{
|
||||
vec![
|
||||
Arc::new(list::ListTool {
|
||||
backend: backend.clone(),
|
||||
|
||||
@@ -8,13 +8,12 @@ use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde_json::json;
|
||||
|
||||
use crate::backend::DEFAULT_READ_MAX_TOKENS;
|
||||
use crate::DEFAULT_READ_MAX_TOKENS;
|
||||
use crate::READ_TOOL_NAME;
|
||||
use crate::backend::MemoriesBackend;
|
||||
use crate::backend::ReadMemoryRequest;
|
||||
use crate::backend::ReadMemoryResponse;
|
||||
use crate::local::LocalMemoriesBackend;
|
||||
|
||||
use super::READ_TOOL_NAME;
|
||||
use super::backend_error_to_function_call;
|
||||
use super::function_tool;
|
||||
use super::parse_args;
|
||||
@@ -30,11 +29,14 @@ struct ReadArgs {
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(super) struct ReadTool {
|
||||
pub(super) backend: LocalMemoriesBackend,
|
||||
pub(super) struct ReadTool<B> {
|
||||
pub(super) backend: B,
|
||||
}
|
||||
|
||||
impl ExtensionToolExecutor for ReadTool {
|
||||
impl<B> ExtensionToolExecutor for ReadTool<B>
|
||||
where
|
||||
B: MemoriesBackend,
|
||||
{
|
||||
fn tool_name(&self) -> ToolName {
|
||||
ToolName::plain(READ_TOOL_NAME)
|
||||
}
|
||||
@@ -63,58 +65,3 @@ impl ExtensionToolExecutor for ReadTool {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use codex_extension_api::ToolPayload;
|
||||
use codex_tools::ToolOutput;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
|
||||
use super::*;
|
||||
|
||||
#[tokio::test]
|
||||
async fn read_tool_reads_memory_file() {
|
||||
let tempdir = tempfile::tempdir().expect("tempdir");
|
||||
let memory_root = tempdir.path().join("memories");
|
||||
tokio::fs::create_dir_all(&memory_root)
|
||||
.await
|
||||
.expect("create memories dir");
|
||||
tokio::fs::write(
|
||||
memory_root.join("MEMORY.md"),
|
||||
"first line\nsecond needle line\nthird line\n",
|
||||
)
|
||||
.await
|
||||
.expect("write memory");
|
||||
let tool = ReadTool {
|
||||
backend: LocalMemoriesBackend::from_memory_root(&memory_root),
|
||||
};
|
||||
let payload = ToolPayload::Function {
|
||||
arguments: json!({
|
||||
"path": "MEMORY.md",
|
||||
"line_offset": 2,
|
||||
"max_lines": 1
|
||||
})
|
||||
.to_string(),
|
||||
};
|
||||
|
||||
let output = tool
|
||||
.handle(ToolCall {
|
||||
call_id: "call-1".to_string(),
|
||||
tool_name: ToolName::plain(READ_TOOL_NAME),
|
||||
payload: payload.clone(),
|
||||
})
|
||||
.await
|
||||
.expect("read should succeed");
|
||||
|
||||
assert_eq!(
|
||||
output.post_tool_use_response("call-1", &payload),
|
||||
Some(json!({
|
||||
"path": "MEMORY.md",
|
||||
"content": "second needle line\n",
|
||||
"start_line_number": 2,
|
||||
"truncated": true
|
||||
}))
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -8,15 +8,14 @@ use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde_json::json;
|
||||
|
||||
use crate::backend::DEFAULT_SEARCH_MAX_RESULTS;
|
||||
use crate::backend::MAX_SEARCH_RESULTS;
|
||||
use crate::DEFAULT_SEARCH_MAX_RESULTS;
|
||||
use crate::MAX_SEARCH_RESULTS;
|
||||
use crate::SEARCH_TOOL_NAME;
|
||||
use crate::backend::MemoriesBackend;
|
||||
use crate::backend::SearchMatchMode;
|
||||
use crate::backend::SearchMemoriesRequest;
|
||||
use crate::backend::SearchMemoriesResponse;
|
||||
use crate::local::LocalMemoriesBackend;
|
||||
|
||||
use super::SEARCH_TOOL_NAME;
|
||||
use super::backend_error_to_function_call;
|
||||
use super::clamp_max_results;
|
||||
use super::function_tool;
|
||||
@@ -39,11 +38,14 @@ struct SearchArgs {
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(super) struct SearchTool {
|
||||
pub(super) backend: LocalMemoriesBackend,
|
||||
pub(super) struct SearchTool<B> {
|
||||
pub(super) backend: B,
|
||||
}
|
||||
|
||||
impl ExtensionToolExecutor for SearchTool {
|
||||
impl<B> ExtensionToolExecutor for SearchTool<B>
|
||||
where
|
||||
B: MemoriesBackend,
|
||||
{
|
||||
fn tool_name(&self) -> ToolName {
|
||||
ToolName::plain(SEARCH_TOOL_NAME)
|
||||
}
|
||||
@@ -86,75 +88,3 @@ impl SearchArgs {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn search_args_accept_multiple_queries() {
|
||||
let args: SearchArgs = serde_json::from_value(json!({
|
||||
"queries": ["alpha", "needle"],
|
||||
"case_sensitive": false
|
||||
}))
|
||||
.expect("multi-query args should parse");
|
||||
|
||||
let request = args.into_request();
|
||||
|
||||
assert_eq!(
|
||||
request,
|
||||
SearchMemoriesRequest {
|
||||
queries: vec!["alpha".to_string(), "needle".to_string()],
|
||||
match_mode: SearchMatchMode::Any,
|
||||
path: None,
|
||||
cursor: None,
|
||||
context_lines: 0,
|
||||
case_sensitive: false,
|
||||
normalized: false,
|
||||
max_results: DEFAULT_SEARCH_MAX_RESULTS,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn search_args_accept_windowed_all_match_mode() {
|
||||
let args: SearchArgs = serde_json::from_value(json!({
|
||||
"queries": ["alpha", "needle"],
|
||||
"match_mode": {
|
||||
"type": "all_within_lines",
|
||||
"line_count": 3
|
||||
}
|
||||
}))
|
||||
.expect("windowed all args should parse");
|
||||
|
||||
let request = args.into_request();
|
||||
|
||||
assert_eq!(
|
||||
request,
|
||||
SearchMemoriesRequest {
|
||||
queries: vec!["alpha".to_string(), "needle".to_string()],
|
||||
match_mode: SearchMatchMode::AllWithinLines { line_count: 3 },
|
||||
path: None,
|
||||
cursor: None,
|
||||
context_lines: 0,
|
||||
case_sensitive: true,
|
||||
normalized: false,
|
||||
max_results: DEFAULT_SEARCH_MAX_RESULTS,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn search_args_reject_legacy_single_query() {
|
||||
let err = serde_json::from_value::<SearchArgs>(json!({
|
||||
"query": "needle",
|
||||
}))
|
||||
.expect_err("legacy query field should be rejected");
|
||||
|
||||
assert!(err.to_string().contains("unknown field"));
|
||||
assert!(err.to_string().contains("query"));
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user