mirror of
https://github.com/openai/codex.git
synced 2026-05-27 22:44:23 +00:00
Add ad-hoc memory note tool (#24562)
## Why Codex memory updates currently rely on instructions that tell agents to create ad-hoc note files directly in the memory workspace. The memories extension already has a `MemoriesBackend` abstraction for local storage and future non-filesystem backends, so the ad-hoc note writer should live behind that same interface instead of baking local filesystem assumptions into the tool shape. ## What - Adds a `memories/add_ad_hoc_note` tool to the existing memories tool bundle. - Extends `MemoriesBackend` with `add_ad_hoc_note` plus request/response types so remote memory stores can implement the same operation later. - Implements the local backend by creating append-only notes under `extensions/ad_hoc/notes`. - Validates the tool-provided filename contract (`YYYY-MM-DDTHH-MM-SS-<slug>.md`), rejects path-like filenames, rejects empty notes, and uses create-new semantics so existing notes are never overwritten. - Keeps memories tool contribution behind the existing commented-out registration path; this defines the tool surface without newly exposing it through app-server. ## Test Plan - `just test -p codex-memories-extension`
This commit is contained in:
@@ -3,13 +3,18 @@ use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use std::future::Future;
|
||||
|
||||
/// Storage interface behind the memories MCP tools.
|
||||
/// Storage interface behind the memories tools.
|
||||
///
|
||||
/// Implementations should return paths relative to the memory store and enforce
|
||||
/// their own storage-specific access rules. The local implementation uses the
|
||||
/// filesystem today; a later implementation can satisfy the same contract from a
|
||||
/// remote backend.
|
||||
pub trait MemoriesBackend: Clone + Send + Sync + 'static {
|
||||
fn add_ad_hoc_note(
|
||||
&self,
|
||||
request: AddAdHocMemoryNoteRequest,
|
||||
) -> impl Future<Output = Result<AddAdHocMemoryNoteResponse, MemoriesBackendError>> + Send;
|
||||
|
||||
fn list(
|
||||
&self,
|
||||
request: ListMemoriesRequest,
|
||||
@@ -26,6 +31,16 @@ pub trait MemoriesBackend: Clone + Send + Sync + 'static {
|
||||
) -> impl Future<Output = Result<SearchMemoriesResponse, MemoriesBackendError>> + Send;
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct AddAdHocMemoryNoteRequest {
|
||||
pub filename: String,
|
||||
pub note: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, JsonSchema)]
|
||||
#[schemars(deny_unknown_fields)]
|
||||
pub struct AddAdHocMemoryNoteResponse {}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct ListMemoriesRequest {
|
||||
pub path: Option<String>,
|
||||
@@ -119,6 +134,12 @@ pub struct MemorySearchMatch {
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
pub enum MemoriesBackendError {
|
||||
#[error("filename '{filename}' {reason}")]
|
||||
InvalidFilename { filename: String, reason: String },
|
||||
#[error("ad-hoc note must not be empty")]
|
||||
EmptyAdHocNote,
|
||||
#[error("ad-hoc note '{filename}' already exists")]
|
||||
AdHocNoteAlreadyExists { filename: String },
|
||||
#[error("path '{path}' {reason}")]
|
||||
InvalidPath { path: String, reason: String },
|
||||
#[error("cursor '{cursor}' {reason}")]
|
||||
@@ -142,6 +163,13 @@ pub enum MemoriesBackendError {
|
||||
}
|
||||
|
||||
impl MemoriesBackendError {
|
||||
pub fn invalid_filename(filename: impl Into<String>, reason: impl Into<String>) -> Self {
|
||||
Self::InvalidFilename {
|
||||
filename: filename.into(),
|
||||
reason: reason.into(),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn invalid_path(path: impl Into<String>, reason: impl Into<String>) -> Self {
|
||||
Self::InvalidPath {
|
||||
path: path.into(),
|
||||
|
||||
@@ -15,6 +15,7 @@ pub(crate) const DEFAULT_READ_MAX_TOKENS: usize = 20_000;
|
||||
pub(crate) const MEMORY_TOOL_DEVELOPER_INSTRUCTIONS_SUMMARY_TOKEN_LIMIT: usize = 2_500;
|
||||
|
||||
pub(crate) const MEMORY_TOOLS_NAMESPACE: &str = "memories/";
|
||||
pub(crate) const ADD_AD_HOC_NOTE_TOOL_NAME: &str = "add_ad_hoc_note";
|
||||
pub(crate) const LIST_TOOL_NAME: &str = "list";
|
||||
pub(crate) const READ_TOOL_NAME: &str = "read";
|
||||
pub(crate) const SEARCH_TOOL_NAME: &str = "search";
|
||||
|
||||
@@ -4,6 +4,8 @@ use std::path::PathBuf;
|
||||
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
|
||||
use crate::backend::AddAdHocMemoryNoteRequest;
|
||||
use crate::backend::AddAdHocMemoryNoteResponse;
|
||||
use crate::backend::ListMemoriesRequest;
|
||||
use crate::backend::ListMemoriesResponse;
|
||||
use crate::backend::MemoriesBackend;
|
||||
@@ -13,6 +15,7 @@ use crate::backend::ReadMemoryResponse;
|
||||
use crate::backend::SearchMemoriesRequest;
|
||||
use crate::backend::SearchMemoriesResponse;
|
||||
|
||||
mod ad_hoc_note;
|
||||
mod list;
|
||||
mod path;
|
||||
mod read;
|
||||
@@ -96,6 +99,13 @@ impl LocalMemoriesBackend {
|
||||
}
|
||||
|
||||
impl MemoriesBackend for LocalMemoriesBackend {
|
||||
async fn add_ad_hoc_note(
|
||||
&self,
|
||||
request: AddAdHocMemoryNoteRequest,
|
||||
) -> Result<AddAdHocMemoryNoteResponse, MemoriesBackendError> {
|
||||
ad_hoc_note::add_ad_hoc_note(self, request).await
|
||||
}
|
||||
|
||||
async fn list(
|
||||
&self,
|
||||
request: ListMemoriesRequest,
|
||||
|
||||
147
codex-rs/ext/memories/src/local/ad_hoc_note.rs
Normal file
147
codex-rs/ext/memories/src/local/ad_hoc_note.rs
Normal file
@@ -0,0 +1,147 @@
|
||||
use std::fs::OpenOptions;
|
||||
use std::io::Write;
|
||||
use std::path::Path;
|
||||
|
||||
use crate::backend::AddAdHocMemoryNoteRequest;
|
||||
use crate::backend::AddAdHocMemoryNoteResponse;
|
||||
use crate::backend::MemoriesBackendError;
|
||||
|
||||
use super::LocalMemoriesBackend;
|
||||
use super::path::reject_symlink;
|
||||
|
||||
const AD_HOC_NOTES_DIR: &[&str] = &["extensions", "ad_hoc", "notes"];
|
||||
const AD_HOC_NOTE_FILENAME_MAX_BYTES: usize = 128;
|
||||
const AD_HOC_NOTE_SLUG_MAX_BYTES: usize = 80;
|
||||
const TIMESTAMP_PREFIX_LEN: usize = "YYYY-MM-DDTHH-MM-SS-".len();
|
||||
|
||||
pub(super) async fn add_ad_hoc_note(
|
||||
backend: &LocalMemoriesBackend,
|
||||
request: AddAdHocMemoryNoteRequest,
|
||||
) -> Result<AddAdHocMemoryNoteResponse, MemoriesBackendError> {
|
||||
validate_filename(&request.filename)?;
|
||||
if request.note.trim().is_empty() {
|
||||
return Err(MemoriesBackendError::EmptyAdHocNote);
|
||||
}
|
||||
|
||||
let notes_dir = ensure_notes_dir(backend).await?;
|
||||
let path = notes_dir.join(&request.filename);
|
||||
let mut file = match OpenOptions::new().write(true).create_new(true).open(&path) {
|
||||
Ok(file) => file,
|
||||
Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => {
|
||||
return Err(MemoriesBackendError::AdHocNoteAlreadyExists {
|
||||
filename: request.filename,
|
||||
});
|
||||
}
|
||||
Err(err) => return Err(err.into()),
|
||||
};
|
||||
file.write_all(request.note.as_bytes())?;
|
||||
|
||||
Ok(AddAdHocMemoryNoteResponse {})
|
||||
}
|
||||
|
||||
async fn ensure_notes_dir(
|
||||
backend: &LocalMemoriesBackend,
|
||||
) -> Result<std::path::PathBuf, MemoriesBackendError> {
|
||||
ensure_directory(&backend.root).await?;
|
||||
let mut path = backend.root.clone();
|
||||
for component in AD_HOC_NOTES_DIR {
|
||||
path.push(component);
|
||||
ensure_directory(&path).await?;
|
||||
}
|
||||
Ok(path)
|
||||
}
|
||||
|
||||
async fn ensure_directory(path: &Path) -> Result<(), MemoriesBackendError> {
|
||||
match LocalMemoriesBackend::metadata_or_none(path).await? {
|
||||
Some(metadata) => {
|
||||
reject_symlink(&path.display().to_string(), &metadata)?;
|
||||
if metadata.is_dir() {
|
||||
return Ok(());
|
||||
}
|
||||
return Err(MemoriesBackendError::invalid_path(
|
||||
path.display().to_string(),
|
||||
"must be a directory",
|
||||
));
|
||||
}
|
||||
None => tokio::fs::create_dir(path).await?,
|
||||
}
|
||||
|
||||
let Some(metadata) = LocalMemoriesBackend::metadata_or_none(path).await? else {
|
||||
return Err(MemoriesBackendError::NotFound {
|
||||
path: path.display().to_string(),
|
||||
});
|
||||
};
|
||||
reject_symlink(&path.display().to_string(), &metadata)?;
|
||||
if !metadata.is_dir() {
|
||||
return Err(MemoriesBackendError::invalid_path(
|
||||
path.display().to_string(),
|
||||
"must be a directory",
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn validate_filename(filename: &str) -> Result<(), MemoriesBackendError> {
|
||||
if filename.len() > AD_HOC_NOTE_FILENAME_MAX_BYTES {
|
||||
return Err(MemoriesBackendError::invalid_filename(
|
||||
filename,
|
||||
"must be at most 128 bytes",
|
||||
));
|
||||
}
|
||||
let Some(stem) = filename.strip_suffix(".md") else {
|
||||
return Err(MemoriesBackendError::invalid_filename(
|
||||
filename,
|
||||
"must end with .md",
|
||||
));
|
||||
};
|
||||
let Some(slug) = stem.get(TIMESTAMP_PREFIX_LEN..) else {
|
||||
return Err(MemoriesBackendError::invalid_filename(
|
||||
filename,
|
||||
"must use YYYY-MM-DDTHH-MM-SS-<slug>.md",
|
||||
));
|
||||
};
|
||||
if !has_valid_timestamp_prefix(stem) {
|
||||
return Err(MemoriesBackendError::invalid_filename(
|
||||
filename,
|
||||
"must use YYYY-MM-DDTHH-MM-SS-<slug>.md",
|
||||
));
|
||||
}
|
||||
if slug.is_empty() || slug.len() > AD_HOC_NOTE_SLUG_MAX_BYTES {
|
||||
return Err(MemoriesBackendError::invalid_filename(
|
||||
filename,
|
||||
"slug must be 1 to 80 bytes",
|
||||
));
|
||||
}
|
||||
if !slug
|
||||
.bytes()
|
||||
.all(|byte| byte.is_ascii_lowercase() || byte.is_ascii_digit() || byte == b'-')
|
||||
{
|
||||
return Err(MemoriesBackendError::invalid_filename(
|
||||
filename,
|
||||
"slug must contain only lowercase ASCII letters, digits, or hyphens",
|
||||
));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn has_valid_timestamp_prefix(stem: &str) -> bool {
|
||||
let bytes = stem.as_bytes();
|
||||
bytes.len() > TIMESTAMP_PREFIX_LEN
|
||||
&& bytes[4] == b'-'
|
||||
&& bytes[7] == b'-'
|
||||
&& bytes[10] == b'T'
|
||||
&& bytes[13] == b'-'
|
||||
&& bytes[16] == b'-'
|
||||
&& bytes[19] == b'-'
|
||||
&& are_digits(&bytes[0..4])
|
||||
&& are_digits(&bytes[5..7])
|
||||
&& are_digits(&bytes[8..10])
|
||||
&& are_digits(&bytes[11..13])
|
||||
&& are_digits(&bytes[14..16])
|
||||
&& are_digits(&bytes[17..19])
|
||||
}
|
||||
|
||||
fn are_digits(bytes: &[u8]) -> bool {
|
||||
bytes.iter().all(u8::is_ascii_digit)
|
||||
}
|
||||
@@ -69,6 +69,7 @@ fn tools_are_contributed_when_enabled() {
|
||||
assert_eq!(
|
||||
tool_names,
|
||||
vec![
|
||||
memory_tool_name(crate::ADD_AD_HOC_NOTE_TOOL_NAME),
|
||||
memory_tool_name(crate::LIST_TOOL_NAME),
|
||||
memory_tool_name(crate::READ_TOOL_NAME),
|
||||
memory_tool_name(crate::SEARCH_TOOL_NAME),
|
||||
@@ -76,6 +77,26 @@ fn tools_are_contributed_when_enabled() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ad_hoc_tool_definition_includes_filename_contract() {
|
||||
let tool = memory_tool(
|
||||
Path::new("/tmp/codex-home/memories"),
|
||||
crate::ADD_AD_HOC_NOTE_TOOL_NAME,
|
||||
);
|
||||
let spec = serde_json::to_value(tool.spec()).expect("serialize tool spec");
|
||||
|
||||
let filename = spec
|
||||
.pointer("/tools/0/parameters/properties/filename")
|
||||
.expect("filename parameter should be in tool schema");
|
||||
assert_eq!(filename.pointer("/type"), Some(&json!("string")));
|
||||
assert!(
|
||||
filename
|
||||
.pointer("/description")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.is_some_and(|description| description.contains("YYYY-MM-DDTHH-MM-SS-<slug>.md"))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn prompt_contribution_uses_memory_summary_when_enabled() {
|
||||
let tempdir = tempfile::tempdir().expect("tempdir");
|
||||
@@ -110,6 +131,79 @@ async fn prompt_contribution_uses_memory_summary_when_enabled() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn add_ad_hoc_note_tool_creates_note_file() {
|
||||
let tempdir = tempfile::tempdir().expect("tempdir");
|
||||
let memory_root = tempdir.path().join("memories");
|
||||
let tool = memory_tool(&memory_root, crate::ADD_AD_HOC_NOTE_TOOL_NAME);
|
||||
let payload = ToolPayload::Function {
|
||||
arguments: json!({
|
||||
"filename": "2026-05-26T13-42-08-remember-review-style.md",
|
||||
"note": "Remember to keep PR review comments concise.",
|
||||
})
|
||||
.to_string(),
|
||||
};
|
||||
|
||||
let output = tool
|
||||
.handle(ToolCall {
|
||||
turn_id: "turn-1".to_string(),
|
||||
call_id: "call-1".to_string(),
|
||||
tool_name: memory_tool_name(crate::ADD_AD_HOC_NOTE_TOOL_NAME),
|
||||
truncation_policy: TruncationPolicy::Bytes(1024),
|
||||
conversation_history: codex_extension_api::ConversationHistory::default(),
|
||||
payload: payload.clone(),
|
||||
})
|
||||
.await
|
||||
.expect("ad-hoc note should be written");
|
||||
|
||||
assert_eq!(
|
||||
output.post_tool_use_response("call-1", &payload),
|
||||
Some(json!({}))
|
||||
);
|
||||
assert_eq!(
|
||||
tokio::fs::read_to_string(
|
||||
memory_root
|
||||
.join("extensions/ad_hoc/notes")
|
||||
.join("2026-05-26T13-42-08-remember-review-style.md")
|
||||
)
|
||||
.await
|
||||
.expect("read ad-hoc note"),
|
||||
"Remember to keep PR review comments concise."
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn add_ad_hoc_note_tool_rejects_paths_as_filenames() {
|
||||
let tempdir = tempfile::tempdir().expect("tempdir");
|
||||
let memory_root = tempdir.path().join("memories");
|
||||
let tool = memory_tool(&memory_root, crate::ADD_AD_HOC_NOTE_TOOL_NAME);
|
||||
let payload = ToolPayload::Function {
|
||||
arguments: json!({
|
||||
"filename": "../2026-05-26T13-42-08-remember-review-style.md",
|
||||
"note": "Remember to keep PR review comments concise.",
|
||||
})
|
||||
.to_string(),
|
||||
};
|
||||
|
||||
let result = tool
|
||||
.handle(ToolCall {
|
||||
turn_id: "turn-1".to_string(),
|
||||
call_id: "call-1".to_string(),
|
||||
tool_name: memory_tool_name(crate::ADD_AD_HOC_NOTE_TOOL_NAME),
|
||||
truncation_policy: TruncationPolicy::Bytes(1024),
|
||||
conversation_history: codex_extension_api::ConversationHistory::default(),
|
||||
payload,
|
||||
})
|
||||
.await;
|
||||
let err = match result {
|
||||
Ok(_) => panic!("path-like filename should be rejected"),
|
||||
Err(err) => err,
|
||||
};
|
||||
|
||||
assert!(err.to_string().contains("filename"));
|
||||
assert!(err.to_string().contains("YYYY-MM-DDTHH-MM-SS"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn read_tool_reads_memory_file() {
|
||||
let tempdir = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
73
codex-rs/ext/memories/src/tools/ad_hoc_note.rs
Normal file
73
codex-rs/ext/memories/src/tools/ad_hoc_note.rs
Normal file
@@ -0,0 +1,73 @@
|
||||
use codex_extension_api::JsonToolOutput;
|
||||
use codex_extension_api::ToolCall;
|
||||
use codex_extension_api::ToolExecutor;
|
||||
use codex_extension_api::ToolName;
|
||||
use codex_extension_api::ToolSpec;
|
||||
use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde_json::json;
|
||||
|
||||
use crate::ADD_AD_HOC_NOTE_TOOL_NAME;
|
||||
use crate::backend::AddAdHocMemoryNoteRequest;
|
||||
use crate::backend::AddAdHocMemoryNoteResponse;
|
||||
use crate::backend::MemoriesBackend;
|
||||
|
||||
use super::backend_error_to_function_call;
|
||||
use super::memory_function_tool;
|
||||
use super::memory_tool_name;
|
||||
use super::parse_args;
|
||||
|
||||
#[derive(Deserialize, JsonSchema)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
struct AddAdHocNoteArgs {
|
||||
/// Name of the note file to create, in
|
||||
/// YYYY-MM-DDTHH-MM-SS-<slug>.md format. The slug must use only lowercase
|
||||
/// ASCII letters, digits, and hyphens.
|
||||
#[schemars(
|
||||
length(min = 24, max = 128),
|
||||
regex(pattern = r"^\d{4}-\d{2}-\d{2}T\d{2}-\d{2}-\d{2}-[a-z0-9][a-z0-9-]{0,79}\.md$")
|
||||
)]
|
||||
filename: String,
|
||||
/// Verbatim Markdown note to append to the ad-hoc memory notes.
|
||||
#[schemars(length(min = 1))]
|
||||
note: String,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(super) struct AddAdHocNoteTool<B> {
|
||||
pub(super) backend: B,
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl<B> ToolExecutor<ToolCall> for AddAdHocNoteTool<B>
|
||||
where
|
||||
B: MemoriesBackend,
|
||||
{
|
||||
fn tool_name(&self) -> ToolName {
|
||||
memory_tool_name(ADD_AD_HOC_NOTE_TOOL_NAME)
|
||||
}
|
||||
|
||||
fn spec(&self) -> ToolSpec {
|
||||
memory_function_tool::<AddAdHocNoteArgs, AddAdHocMemoryNoteResponse>(
|
||||
ADD_AD_HOC_NOTE_TOOL_NAME,
|
||||
"Create one append-only ad-hoc memory note after the user explicitly asks Codex to remember, forget, or update something.",
|
||||
)
|
||||
}
|
||||
|
||||
async fn handle(
|
||||
&self,
|
||||
call: ToolCall,
|
||||
) -> Result<Box<dyn codex_extension_api::ToolOutput>, codex_extension_api::FunctionCallError>
|
||||
{
|
||||
let backend = self.backend.clone();
|
||||
let args: AddAdHocNoteArgs = parse_args(&call)?;
|
||||
let response = backend
|
||||
.add_ad_hoc_note(AddAdHocMemoryNoteRequest {
|
||||
filename: args.filename,
|
||||
note: args.note,
|
||||
})
|
||||
.await
|
||||
.map_err(backend_error_to_function_call)?;
|
||||
Ok(Box::new(JsonToolOutput::new(json!(response))))
|
||||
}
|
||||
}
|
||||
@@ -19,6 +19,7 @@ use crate::backend::MemoriesBackend;
|
||||
use crate::backend::MemoriesBackendError;
|
||||
use crate::schema;
|
||||
|
||||
mod ad_hoc_note;
|
||||
mod list;
|
||||
mod read;
|
||||
mod search;
|
||||
@@ -28,6 +29,9 @@ where
|
||||
B: MemoriesBackend,
|
||||
{
|
||||
vec![
|
||||
Arc::new(ad_hoc_note::AddAdHocNoteTool {
|
||||
backend: backend.clone(),
|
||||
}),
|
||||
Arc::new(list::ListTool {
|
||||
backend: backend.clone(),
|
||||
}),
|
||||
@@ -82,12 +86,15 @@ fn backend_error_to_function_call(err: MemoriesBackendError) -> FunctionCallErro
|
||||
match err {
|
||||
MemoriesBackendError::InvalidPath { .. }
|
||||
| MemoriesBackendError::InvalidCursor { .. }
|
||||
| MemoriesBackendError::InvalidFilename { .. }
|
||||
| MemoriesBackendError::NotFound { .. }
|
||||
| MemoriesBackendError::InvalidLineOffset
|
||||
| MemoriesBackendError::InvalidMaxLines
|
||||
| MemoriesBackendError::LineOffsetExceedsFileLength
|
||||
| MemoriesBackendError::NotFile { .. }
|
||||
| MemoriesBackendError::EmptyQuery
|
||||
| MemoriesBackendError::EmptyAdHocNote
|
||||
| MemoriesBackendError::AdHocNoteAlreadyExists { .. }
|
||||
| MemoriesBackendError::InvalidMatchWindow => {
|
||||
FunctionCallError::RespondToModel(err.to_string())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user