Skip to content

Commit f8987ce

Browse files
LEON-gittechsa-buc
authored andcommitted
Add oh-my-codex memory features: compaction protocol, directive priority, bounded overlay, file locking, merge-write
1. Compaction survival protocol - instructions in read_path.md template 2. Directive priority - MemoryFrontmatter.priority field, high-priority directives injected into overlay 3. Bounded overlay injection - 3500 char cap on developer instructions 4. File locking for notepad - directory-based lock with PID stale detection 5. Merge vs replace for memory_write - optional merge parameter preserves existing frontmatter
1 parent d66275d commit f8987ce

6 files changed

Lines changed: 346 additions & 90 deletions

File tree

codex-rs/core/src/memories/memdir.rs

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ pub(crate) struct MemoryFrontmatter {
4545
pub keywords: Vec<String>,
4646
#[serde(default = "default_source")]
4747
pub source: String,
48+
/// Directive priority: "high" directives are injected into the memory
49+
/// overlay; "normal" (default) are only available via memory_search.
50+
#[serde(default)]
51+
pub priority: Option<String>,
4852
#[serde(default)]
4953
pub updated_at: Option<DateTime<Utc>>,
5054
}
@@ -108,6 +112,7 @@ pub(crate) fn parse_topic(path: &Path, raw: &str) -> (MemoryFrontmatter, String)
108112
r#type: default_memory_type(),
109113
keywords: Vec::new(),
110114
source: default_source(),
115+
priority: None,
111116
updated_at: None,
112117
};
113118
(frontmatter, raw.to_string())
@@ -275,8 +280,29 @@ pub(crate) async fn build_memory_prompt_content(
275280
parts.push(format!("## Memory Index (MEMORY.md)\n{index}"));
276281
}
277282

278-
// 2. Topics.
283+
// 2. High-priority directives (top 3) + topic scoring.
279284
let topics = scan_memory_topics(&root).await;
285+
let high_directives: Vec<_> = topics
286+
.iter()
287+
.filter(|t| t.frontmatter.priority.as_deref() == Some("high"))
288+
.take(3)
289+
.collect();
290+
if !high_directives.is_empty() {
291+
let directive_lines: Vec<String> = high_directives
292+
.iter()
293+
.map(|t| {
294+
let desc = if t.frontmatter.description.is_empty() {
295+
t.frontmatter.name.clone()
296+
} else {
297+
t.frontmatter.description.clone()
298+
};
299+
format!("- **{}**: {}", t.frontmatter.name, desc)
300+
})
301+
.collect();
302+
parts.push(format!("## Directives\n{}", directive_lines.join("\n")));
303+
}
304+
305+
// 3. Topics (scored by relevance).
280306
if !topics.is_empty() {
281307
let mut scored: Vec<_> = topics
282308
.into_iter()

codex-rs/core/src/memories/notepad.rs

Lines changed: 200 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@
77
//!
88
//! Storage: `{codex_home}/memories/notepad.md`
99
//! Writes use atomic rename (`tmp.{pid}` + `rename`) for crash safety.
10+
//! Concurrent access is guarded by a directory-based lock with stale detection.
1011
1112
use std::path::Path;
1213
use std::path::PathBuf;
14+
use std::time::Duration;
1315

1416
use anyhow::Context;
1517
use chrono::Utc;
@@ -22,6 +24,12 @@ const PRIORITY_MAX_CHARS: usize = 500;
2224
/// Default number of days before WORKING MEMORY entries are pruned.
2325
const DEFAULT_PRUNE_DAYS: u64 = 7;
2426

27+
/// Lock acquisition timeout in milliseconds.
28+
const LOCK_TIMEOUT_MS: u64 = 5000;
29+
30+
/// Lock polling interval in milliseconds.
31+
const LOCK_POLL_MS: u64 = 100;
32+
2533
/// Section header markers.
2634
const PRIORITY_HEADER: &str = "## PRIORITY";
2735
const WORKING_HEADER: &str = "## WORKING MEMORY";
@@ -124,6 +132,86 @@ async fn atomic_write(path: &Path, content: &str) -> anyhow::Result<()> {
124132
Ok(())
125133
}
126134

135+
// ---------------------------------------------------------------------------
136+
// Directory-based file locking (inspired by oh-my-codex agents-overlay.ts)
137+
// ---------------------------------------------------------------------------
138+
139+
/// Returns the lock directory path for the notepad under `root`.
140+
fn lock_dir(root: &Path) -> PathBuf {
141+
root.join(".notepad.lock")
142+
}
143+
144+
/// Acquire a directory-based lock with PID owner metadata and stale detection.
145+
///
146+
/// The lock is a directory created atomically via `mkdir`. If the directory
147+
/// already exists, the owner PID is checked: if the owner process is dead,
148+
/// the stale lock is reaped and we retry.
149+
async fn acquire_lock(root: &Path) -> anyhow::Result<()> {
150+
let lock = lock_dir(root);
151+
if let Some(parent) = lock.parent() {
152+
fs::create_dir_all(parent).await.ok();
153+
}
154+
155+
let start = std::time::Instant::now();
156+
let timeout = Duration::from_millis(LOCK_TIMEOUT_MS);
157+
158+
while start.elapsed() < timeout {
159+
// Attempt atomic directory creation.
160+
match fs::create_dir(&lock).await {
161+
Ok(()) => {
162+
// Write owner metadata for stale detection.
163+
let owner = lock.join("owner.json");
164+
let meta = format!(r#"{{"pid":{},"ts":{}}}"#, std::process::id(), start.elapsed().as_millis());
165+
fs::write(&owner, meta).await.ok();
166+
return Ok(());
167+
}
168+
Err(_) => {
169+
// Lock exists — check if owner is dead.
170+
let owner_path = lock.join("owner.json");
171+
if let Ok(raw) = fs::read_to_string(&owner_path).await {
172+
if let Some(pid_str) = raw.split(r#""pid":"#).nth(1) {
173+
if let Some(end) = pid_str.find(',') {
174+
if let Ok(pid) = pid_str[..end].parse::<u32>() {
175+
// Check if PID is still alive (Unix: signal 0).
176+
#[cfg(unix)]
177+
{
178+
if unsafe { libc::kill(pid as i32, 0) } != 0 {
179+
// Owner is dead — reap stale lock.
180+
fs::remove_dir_all(&lock).await.ok();
181+
continue;
182+
}
183+
}
184+
let _ = pid; // Silence unused warning on non-Unix.
185+
}
186+
}
187+
}
188+
}
189+
tokio::time::sleep(Duration::from_millis(LOCK_POLL_MS)).await;
190+
}
191+
}
192+
}
193+
194+
anyhow::bail!("failed to acquire notepad lock within {}ms", LOCK_TIMEOUT_MS)
195+
}
196+
197+
/// Release the directory-based lock.
198+
async fn release_lock(root: &Path) {
199+
let lock = lock_dir(root);
200+
fs::remove_dir_all(&lock).await.ok();
201+
}
202+
203+
/// Execute a closure while holding the notepad lock.
204+
async fn with_notepad_lock<F, Fut, T>(root: &Path, f: F) -> anyhow::Result<T>
205+
where
206+
F: FnOnce() -> Fut,
207+
Fut: std::future::Future<Output = anyhow::Result<T>>,
208+
{
209+
acquire_lock(root).await?;
210+
let result = f().await;
211+
release_lock(root).await;
212+
result
213+
}
214+
127215
/// Read the notepad file, optionally filtering to a specific section.
128216
///
129217
/// Returns `None` if the file doesn't exist.
@@ -160,110 +248,146 @@ pub async fn read_notepad(root: &Path, section: Option<NotepadSection>) -> Optio
160248

161249
/// Write (replace) the PRIORITY section content.
162250
/// Content is truncated to `PRIORITY_MAX_CHARS` if it exceeds the limit.
251+
/// Acquires a file lock to prevent concurrent writes.
163252
pub async fn write_priority(root: &Path, content: &str) -> anyhow::Result<()> {
164-
let path = notepad_path(root);
165-
let truncated = if content.len() > PRIORITY_MAX_CHARS {
166-
&content[..content.floor_char_boundary(PRIORITY_MAX_CHARS)]
167-
} else {
168-
content
169-
};
253+
let root = root.to_path_buf();
254+
let content = content.to_string();
255+
with_notepad_lock(&root, || {
256+
let root = root.clone();
257+
let content = content.clone();
258+
async move {
259+
let path = notepad_path(&root);
260+
let truncated = if content.len() > PRIORITY_MAX_CHARS {
261+
&content[..content.floor_char_boundary(PRIORITY_MAX_CHARS)]
262+
} else {
263+
&content
264+
};
170265

171-
// Load existing sections to preserve WORKING and MANUAL.
172-
let (.., working, manual) = if path.exists() {
173-
let raw = fs::read_to_string(&path).await.unwrap_or_default();
174-
parse_sections(&raw)
175-
} else {
176-
(String::new(), String::new(), String::new())
177-
};
266+
let (.., working, manual) = if path.exists() {
267+
let raw = fs::read_to_string(&path).await.unwrap_or_default();
268+
parse_sections(&raw)
269+
} else {
270+
(String::new(), String::new(), String::new())
271+
};
178272

179-
let new_content = build_notepad_content(truncated, &working, &manual);
180-
atomic_write(&path, &new_content).await
273+
let new_content = build_notepad_content(truncated, &working, &manual);
274+
atomic_write(&path, &new_content).await
275+
}
276+
})
277+
.await
181278
}
182279

183280
/// Append a timestamped entry to the WORKING MEMORY section.
281+
/// Acquires a file lock to prevent concurrent writes.
184282
pub async fn append_working(root: &Path, entry: &str) -> anyhow::Result<()> {
185-
let path = notepad_path(root);
186-
let timestamp = Utc::now().to_rfc3339();
187-
let new_entry = format!("[{timestamp}] {entry}");
188-
189-
let (priority, mut working, manual) = if path.exists() {
190-
let raw = fs::read_to_string(&path).await.unwrap_or_default();
191-
parse_sections(&raw)
192-
} else {
193-
(String::new(), String::new(), String::new())
194-
};
283+
let root = root.to_path_buf();
284+
let entry = entry.to_string();
285+
with_notepad_lock(&root, || {
286+
let root = root.clone();
287+
let entry = entry.clone();
288+
async move {
289+
let path = notepad_path(&root);
290+
let timestamp = Utc::now().to_rfc3339();
291+
let new_entry = format!("[{timestamp}] {entry}");
292+
293+
let (priority, mut working, manual) = if path.exists() {
294+
let raw = fs::read_to_string(&path).await.unwrap_or_default();
295+
parse_sections(&raw)
296+
} else {
297+
(String::new(), String::new(), String::new())
298+
};
195299

196-
if !working.is_empty() {
197-
working.push('\n');
198-
}
199-
working.push_str(&new_entry);
300+
if !working.is_empty() {
301+
working.push('\n');
302+
}
303+
working.push_str(&new_entry);
200304

201-
let new_content = build_notepad_content(&priority, &working, &manual);
202-
atomic_write(&path, &new_content).await
305+
let new_content = build_notepad_content(&priority, &working, &manual);
306+
atomic_write(&path, &new_content).await
307+
}
308+
})
309+
.await
203310
}
204311

205312
/// Append an entry to the MANUAL section (never auto-pruned).
313+
/// Acquires a file lock to prevent concurrent writes.
206314
pub async fn append_manual(root: &Path, entry: &str) -> anyhow::Result<()> {
207-
let path = notepad_path(root);
208-
209-
let (priority, working, mut manual) = if path.exists() {
210-
let raw = fs::read_to_string(&path).await.unwrap_or_default();
211-
parse_sections(&raw)
212-
} else {
213-
(String::new(), String::new(), String::new())
214-
};
315+
let root = root.to_path_buf();
316+
let entry = entry.to_string();
317+
with_notepad_lock(&root, || {
318+
let root = root.clone();
319+
let entry = entry.clone();
320+
async move {
321+
let path = notepad_path(&root);
322+
323+
let (priority, working, mut manual) = if path.exists() {
324+
let raw = fs::read_to_string(&path).await.unwrap_or_default();
325+
parse_sections(&raw)
326+
} else {
327+
(String::new(), String::new(), String::new())
328+
};
215329

216-
if !manual.is_empty() {
217-
manual.push('\n');
218-
}
219-
manual.push_str(entry);
330+
if !manual.is_empty() {
331+
manual.push('\n');
332+
}
333+
manual.push_str(&entry);
220334

221-
let new_content = build_notepad_content(&priority, &working, &manual);
222-
atomic_write(&path, &new_content).await
335+
let new_content = build_notepad_content(&priority, &working, &manual);
336+
atomic_write(&path, &new_content).await
337+
}
338+
})
339+
.await
223340
}
224341

225342
/// Prune WORKING MEMORY entries older than `max_age_days`.
343+
/// Acquires a file lock to prevent concurrent writes.
226344
///
227345
/// Returns the number of entries removed.
228346
pub async fn prune_working(root: &Path, max_age_days: Option<u64>) -> anyhow::Result<usize> {
347+
let root = root.to_path_buf();
229348
let max_days = max_age_days.unwrap_or(DEFAULT_PRUNE_DAYS);
230-
let path = notepad_path(root);
231-
232-
if !path.exists() {
233-
return Ok(0);
234-
}
349+
with_notepad_lock(&root, || {
350+
let root = root.clone();
351+
async move {
352+
let path = notepad_path(&root);
235353

236-
let raw = fs::read_to_string(&path).await.unwrap_or_default();
237-
let (priority, working, manual) = parse_sections(&raw);
238-
239-
let cutoff = Utc::now() - chrono::Duration::days(max_days as i64);
240-
let mut retained = String::new();
241-
let mut removed = 0usize;
354+
if !path.exists() {
355+
return Ok(0);
356+
}
242357

243-
for line in working.lines() {
244-
let trimmed = line.trim();
245-
// Parse timestamp from entries like "[2026-04-27T10:30:00+00:00] some note"
246-
if let Some(ts_end) = trimmed.find("] ") {
247-
let ts_str = &trimmed[1..ts_end];
248-
if let Ok(ts) = chrono::DateTime::parse_from_rfc3339(ts_str) {
249-
if ts.with_timezone(&Utc) < cutoff {
250-
removed += 1;
251-
continue;
358+
let raw = fs::read_to_string(&path).await.unwrap_or_default();
359+
let (priority, working, manual) = parse_sections(&raw);
360+
361+
let cutoff = Utc::now() - chrono::Duration::days(max_days as i64);
362+
let mut retained = String::new();
363+
let mut removed = 0usize;
364+
365+
for line in working.lines() {
366+
let trimmed = line.trim();
367+
if let Some(ts_end) = trimmed.find("] ") {
368+
let ts_str = &trimmed[1..ts_end];
369+
if let Ok(ts) = chrono::DateTime::parse_from_rfc3339(ts_str) {
370+
if ts.with_timezone(&Utc) < cutoff {
371+
removed += 1;
372+
continue;
373+
}
374+
}
252375
}
376+
if !retained.is_empty() {
377+
retained.push('\n');
378+
}
379+
retained.push_str(line);
253380
}
254-
}
255-
if !retained.is_empty() {
256-
retained.push('\n');
257-
}
258-
retained.push_str(line);
259-
}
260381

261-
if removed > 0 {
262-
let new_content = build_notepad_content(&priority, &retained, &manual);
263-
atomic_write(&path, &new_content).await?;
264-
}
382+
if removed > 0 {
383+
let new_content = build_notepad_content(&priority, &retained, &manual);
384+
atomic_write(&path, &new_content).await?;
385+
}
265386

266-
Ok(removed)
387+
Ok(removed)
388+
}
389+
})
390+
.await
267391
}
268392

269393
/// Return notepad statistics: file size, entry counts per section, oldest/newest timestamps.

0 commit comments

Comments
 (0)