mirror of
https://github.com/openai/codex.git
synced 2026-09-04 15:08:45 +00:00
codex: harden goal file refs (#27337)
This commit is contained in:
@@ -1,10 +1,10 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/goal_menu.rs
|
||||
expression: rendered_goal_summary(&mut rx)
|
||||
expression: rendered
|
||||
---
|
||||
Goal
|
||||
Status: active
|
||||
Objective file: goal.md
|
||||
Objective file: $CODEX_HOME/attachments/<uuid>/goal-objective.md
|
||||
Time used: 1m
|
||||
Tokens used: 12.5K
|
||||
Token budget: 80K
|
||||
|
||||
@@ -80,12 +80,23 @@ async fn goal_menu_managed_file_snapshot() {
|
||||
AppThreadGoalStatus::Active,
|
||||
/*token_budget*/ Some(80_000),
|
||||
);
|
||||
goal.objective =
|
||||
"Codex goal objective file: goal.md\nRead that file before continuing.".to_string();
|
||||
goal.objective = crate::goal_files::materialize_goal_draft(
|
||||
chat.config.codex_home.as_path(),
|
||||
crate::goal_files::GoalDraft {
|
||||
objective: "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.expect("materialize goal objective");
|
||||
let path = crate::goal_files::objective_file_path(&goal.objective).expect("goal file path");
|
||||
|
||||
chat.show_goal_summary(goal);
|
||||
|
||||
assert_chatwidget_snapshot!("goal_menu_managed_file", rendered_goal_summary(&mut rx));
|
||||
let rendered = rendered_goal_summary(&mut rx).replace(
|
||||
&path.display().to_string(),
|
||||
"$CODEX_HOME/attachments/<uuid>/goal-objective.md",
|
||||
);
|
||||
assert_chatwidget_snapshot!("goal_menu_managed_file", rendered);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -168,17 +179,19 @@ async fn goal_edit_prompt_hydrates_and_materializes_oversized_objective_file() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
let thread_id = ThreadId::new();
|
||||
let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1);
|
||||
let path = chat.config.codex_home.join("managed-goal.md");
|
||||
std::fs::write(&path, &objective).expect("write goal file");
|
||||
let mut goal = test_goal(
|
||||
thread_id,
|
||||
AppThreadGoalStatus::Paused,
|
||||
/*token_budget*/ Some(80_000),
|
||||
);
|
||||
goal.objective = format!(
|
||||
"Codex goal objective file: {}\nRead that file before continuing.",
|
||||
path.display()
|
||||
);
|
||||
goal.objective = crate::goal_files::materialize_goal_draft(
|
||||
chat.config.codex_home.as_path(),
|
||||
crate::goal_files::GoalDraft {
|
||||
objective: objective.clone(),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.expect("materialize goal objective");
|
||||
|
||||
chat.show_goal_edit_prompt(thread_id, goal);
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
||||
|
||||
@@ -65,8 +65,11 @@ fn next_goal_objective(
|
||||
|
||||
#[test]
|
||||
fn sentinel_like_objective_is_plain_text() {
|
||||
let objective =
|
||||
"Goal objective file: /tmp/not-managed-by-codex\nRead that file before continuing.";
|
||||
let objective = concat!(
|
||||
"Codex goal objective file: ",
|
||||
"/tmp/attachments/00000000-0000-4000-8000-000000000000/goal-objective.md\n",
|
||||
"Read that file before continuing."
|
||||
);
|
||||
|
||||
assert_eq!(crate::goal_files::objective_file_path(objective), None);
|
||||
}
|
||||
|
||||
@@ -116,14 +116,23 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn goal_usage_summary_formats_managed_file_objective() {
|
||||
let temp_dir = tempfile::tempdir().expect("tempdir");
|
||||
let objective = crate::goal_files::materialize_goal_draft(
|
||||
temp_dir.path(),
|
||||
crate::goal_files::GoalDraft {
|
||||
objective: "x"
|
||||
.repeat(codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.expect("materialize goal objective");
|
||||
let path = crate::goal_files::objective_file_path(&objective).expect("goal file path");
|
||||
let mut goal = test_thread_goal(/*token_budget*/ None, /*tokens_used*/ 0);
|
||||
goal.objective =
|
||||
"Codex goal objective file: /tmp/project/goal.md\nRead that file before continuing."
|
||||
.to_string();
|
||||
goal.objective = objective;
|
||||
|
||||
assert_eq!(
|
||||
goal_usage_summary(&goal),
|
||||
"Objective file: /tmp/project/goal.md Time: 2m."
|
||||
format!("Objective file: {} Time: 2m.", path.display())
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,6 +19,7 @@ use uuid::Uuid;
|
||||
|
||||
const GOAL_ATTACHMENT_DIR: &str = "attachments";
|
||||
const GOAL_FILE_PREFIX: &str = "Codex goal objective file: ";
|
||||
const GOAL_FILE_INSTRUCTION: &str = "Read that file before continuing.";
|
||||
const GOAL_FILE_NAME: &str = "goal-objective.md";
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
@@ -121,13 +122,43 @@ pub(crate) fn objective_text_for_edit(objective: &str) -> Result<String> {
|
||||
}
|
||||
|
||||
pub(crate) fn objective_file_path(objective: &str) -> Option<PathBuf> {
|
||||
objective
|
||||
.lines()
|
||||
let mut lines = objective.lines();
|
||||
let path = lines
|
||||
.next()?
|
||||
.strip_prefix(GOAL_FILE_PREFIX)
|
||||
.map(str::trim)
|
||||
.filter(|path| !path.is_empty())
|
||||
.map(PathBuf::from)
|
||||
.map(PathBuf::from)?;
|
||||
if lines.next() != Some(GOAL_FILE_INSTRUCTION) {
|
||||
return None;
|
||||
}
|
||||
|
||||
let parent = path.parent()?;
|
||||
let attachment_id = parent.file_name()?.to_str()?;
|
||||
let attachment_dir = parent.parent()?.file_name()?.to_str()?;
|
||||
if path.is_file()
|
||||
&& path.file_name()?.to_str()? == GOAL_FILE_NAME
|
||||
&& attachment_dir == GOAL_ATTACHMENT_DIR
|
||||
&& Uuid::parse_str(attachment_id).is_ok()
|
||||
{
|
||||
Some(path)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
fn objective_file_reference(path: &Path) -> Result<String> {
|
||||
let reference = format!(
|
||||
"{GOAL_FILE_PREFIX}{}\n{GOAL_FILE_INSTRUCTION}",
|
||||
path.display()
|
||||
);
|
||||
let actual_chars = reference.chars().count();
|
||||
if actual_chars > MAX_THREAD_GOAL_OBJECTIVE_CHARS {
|
||||
bail!(
|
||||
"Goal objective file reference is too long: {actual_chars} characters. Limit: {MAX_THREAD_GOAL_OBJECTIVE_CHARS} characters."
|
||||
);
|
||||
}
|
||||
Ok(reference)
|
||||
}
|
||||
|
||||
fn ensure_output_dir(codex_home: &Path, output_dir: &mut Option<PathBuf>) -> Result<PathBuf> {
|
||||
@@ -152,20 +183,6 @@ fn write_file(path: &Path, content: &str) -> Result<()> {
|
||||
.with_context(|| format!("Could not write goal file {}", path.display()))
|
||||
}
|
||||
|
||||
fn objective_file_reference(path: &Path) -> Result<String> {
|
||||
let reference = format!(
|
||||
"{GOAL_FILE_PREFIX}{}\nRead that file before continuing.",
|
||||
path.display()
|
||||
);
|
||||
let actual_chars = reference.chars().count();
|
||||
if actual_chars > MAX_THREAD_GOAL_OBJECTIVE_CHARS {
|
||||
bail!(
|
||||
"Goal objective file reference is too long: {actual_chars} characters. Limit: {MAX_THREAD_GOAL_OBJECTIVE_CHARS} characters."
|
||||
);
|
||||
}
|
||||
Ok(reference)
|
||||
}
|
||||
|
||||
fn append_section(objective: &mut String, heading: &str, lines: Vec<String>) {
|
||||
if lines.is_empty() {
|
||||
return;
|
||||
|
||||
Reference in New Issue
Block a user