mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[1 of 3] Support long raw TUI goal objectives (#27508)
## Stack 1. **[1 of 3] Support long raw TUI goal objectives** - this PR 2. [2 of 3] Support long pasted text in TUI goals - #27509 3. [3 of 3] Support images in TUI goals - #27510 ## Why `thread/goal/set` limits persisted objective text to 4000 characters. The TUI used to reject raw `/goal` objectives above that limit, even though the client can make them usable by writing the long text to a file and storing a short objective that points at that file. This also needs to work for remote app-server sessions: filesystem API calls must create files on the app-server host, and the stored path must be meaningful to the agent on that host. ## What Changed - Adds an app-server-host path helper so TUI code can build paths that are resolved on the app-server host rather than the TUI host. - Adds TUI app-server session helpers for `fs/createDirectory`, `fs/writeFile`, `fs/readFile`, and `fs/remove` that work for embedded and remote app-server sessions without changing the app-server protocol. - Materializes oversized raw `/goal` objectives into `$CODEX_HOME/attachments/<uuid>/goal-objective.md` through the app-server filesystem APIs, then stores a short, readable objective that directs the agent to that file. - Reads managed objective files back for `/goal edit`. Other goal UI renders the readable stored objective normally, without managed-file-specific presentation logic. - Recognizes managed references only when they name the expected generated file under the app server's reported `$CODEX_HOME`, and cleans up newly materialized files when goal replacement or setting does not complete. ## Verification - Added/updated TUI tests for raw oversized `/goal` submission, large inline-paste expansion, queued oversized goals, app-facing materialization before `thread/goal/set`, managed-path validation, editing, and cleanup. - Added/updated app-server-client remote coverage for initialized remote Codex home handling. ## Manual Testing - Ran the real TUI against a Unix-socket app server with different local and server `$CODEX_HOME` directories. Oversized goals wrote only under the server home, and persisted references used the server-canonical path rather than the TUI path. - Exercised 3,999-, 4,000-, and 4,001-character raw objectives. The first two stayed inline without new files; the 4,001-character objective became a managed objective file. - Submitted a larger 8,275-character objective, verified its full contents on the app-server host, and observed the goal continuation open the referenced server-side file. - Opened `/goal edit` for a managed objective and verified the full text was restored through remote `fs/readFile`. - Submitted an oversized replacement while a goal was active, verified no file was written before confirmation, then canceled and confirmed that the existing goal and attachment count were unchanged.
This commit is contained in:
committed by
GitHub
Unverified
parent
d61dfeb23a
commit
78bab04116
@@ -1,64 +0,0 @@
|
||||
//! Validation helpers for `/goal` objective text.
|
||||
|
||||
use super::*;
|
||||
use crate::bottom_pane::ChatComposer;
|
||||
use codex_protocol::num_format::format_with_separators;
|
||||
use codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS;
|
||||
|
||||
const GOAL_TOO_LONG_FILE_HINT: &str = "Put longer instructions in a file and refer to that file in the goal, for example: /goal follow the instructions in docs/goal.md.";
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub(super) enum GoalObjectiveValidationSource {
|
||||
Live,
|
||||
Queued,
|
||||
}
|
||||
|
||||
impl ChatWidget {
|
||||
pub(super) fn goal_objective_with_pending_pastes_is_allowed(
|
||||
&mut self,
|
||||
args: &str,
|
||||
text_elements: &[TextElement],
|
||||
) -> bool {
|
||||
let pending_pastes = self.bottom_pane.composer_pending_pastes();
|
||||
let objective_chars = if pending_pastes.is_empty() {
|
||||
args.trim().chars().count()
|
||||
} else {
|
||||
let (expanded, _) =
|
||||
ChatComposer::expand_pending_pastes(args, text_elements.to_vec(), &pending_pastes);
|
||||
expanded.trim().chars().count()
|
||||
};
|
||||
self.goal_objective_char_count_is_allowed(
|
||||
objective_chars,
|
||||
GoalObjectiveValidationSource::Live,
|
||||
)
|
||||
}
|
||||
|
||||
pub(super) fn goal_objective_is_allowed(
|
||||
&mut self,
|
||||
objective: &str,
|
||||
source: GoalObjectiveValidationSource,
|
||||
) -> bool {
|
||||
self.goal_objective_char_count_is_allowed(objective.chars().count(), source)
|
||||
}
|
||||
|
||||
fn goal_objective_char_count_is_allowed(
|
||||
&mut self,
|
||||
actual_chars: usize,
|
||||
source: GoalObjectiveValidationSource,
|
||||
) -> bool {
|
||||
if actual_chars <= MAX_THREAD_GOAL_OBJECTIVE_CHARS {
|
||||
return true;
|
||||
}
|
||||
let actual_chars = format_with_separators(actual_chars as i64);
|
||||
let max_chars = format_with_separators(MAX_THREAD_GOAL_OBJECTIVE_CHARS as i64);
|
||||
self.add_error_message(format!(
|
||||
"Goal objective is too long: {actual_chars} characters. Limit: {max_chars} characters. {GOAL_TOO_LONG_FILE_HINT}"
|
||||
));
|
||||
if source == GoalObjectiveValidationSource::Live {
|
||||
self.bottom_pane
|
||||
.set_composer_text(String::new(), Vec::new(), Vec::new());
|
||||
self.bottom_pane.drain_pending_submission_state();
|
||||
}
|
||||
false
|
||||
}
|
||||
}
|
||||
@@ -5,7 +5,6 @@
|
||||
//! dispatch step and records the staged entry once the command has been handled, so
|
||||
//! slash-command recall follows the same submitted-input rule as ordinary text.
|
||||
|
||||
use super::goal_validation::GoalObjectiveValidationSource;
|
||||
use super::*;
|
||||
use crate::app_event::ThreadGoalSetMode;
|
||||
use crate::bottom_pane::prompt_args::parse_slash_name;
|
||||
@@ -570,12 +569,6 @@ impl ChatWidget {
|
||||
return;
|
||||
}
|
||||
|
||||
if cmd == SlashCommand::Goal
|
||||
&& !self.goal_objective_with_pending_pastes_is_allowed(&args, &text_elements)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
let Some((prepared_args, prepared_elements)) =
|
||||
self.prepare_live_inline_args(args, text_elements)
|
||||
else {
|
||||
@@ -607,6 +600,12 @@ impl ChatWidget {
|
||||
}
|
||||
}
|
||||
|
||||
fn clear_live_goal_submission(&mut self) {
|
||||
self.bottom_pane
|
||||
.set_composer_text(String::new(), Vec::new(), Vec::new());
|
||||
self.bottom_pane.drain_pending_submission_state();
|
||||
}
|
||||
|
||||
fn prepared_inline_user_message(
|
||||
&mut self,
|
||||
args: String,
|
||||
@@ -714,6 +713,9 @@ impl ChatWidget {
|
||||
}
|
||||
SlashCommand::Goal if !trimmed.is_empty() => {
|
||||
if !self.config.features.enabled(Feature::Goals) {
|
||||
if source == SlashCommandDispatchSource::Live {
|
||||
self.clear_live_goal_submission();
|
||||
}
|
||||
return;
|
||||
}
|
||||
enum GoalControlCommand {
|
||||
@@ -727,7 +729,7 @@ impl ChatWidget {
|
||||
thread_id: self.thread_id,
|
||||
});
|
||||
if source == SlashCommandDispatchSource::Live {
|
||||
self.bottom_pane.drain_pending_submission_state();
|
||||
self.clear_live_goal_submission();
|
||||
}
|
||||
return;
|
||||
}
|
||||
@@ -743,6 +745,9 @@ impl ChatWidget {
|
||||
"The session must start before you can change a goal.".to_string(),
|
||||
),
|
||||
);
|
||||
if source == SlashCommandDispatchSource::Live {
|
||||
self.clear_live_goal_submission();
|
||||
}
|
||||
return;
|
||||
};
|
||||
match command {
|
||||
@@ -757,29 +762,11 @@ impl ChatWidget {
|
||||
}
|
||||
self.append_message_history_entry(format!("/goal {trimmed}"));
|
||||
if source == SlashCommandDispatchSource::Live {
|
||||
self.bottom_pane.drain_pending_submission_state();
|
||||
self.clear_live_goal_submission();
|
||||
}
|
||||
return;
|
||||
}
|
||||
let objective = args.trim();
|
||||
if objective.is_empty() {
|
||||
self.add_error_message("Goal objective must not be empty.".to_string());
|
||||
self.add_info_message(
|
||||
GOAL_USAGE.to_string(),
|
||||
Some(GOAL_USAGE_HINT.to_string()),
|
||||
);
|
||||
if source == SlashCommandDispatchSource::Live {
|
||||
self.bottom_pane.drain_pending_submission_state();
|
||||
}
|
||||
return;
|
||||
}
|
||||
let validation_source = match source {
|
||||
SlashCommandDispatchSource::Live => GoalObjectiveValidationSource::Live,
|
||||
SlashCommandDispatchSource::Queued => GoalObjectiveValidationSource::Queued,
|
||||
};
|
||||
if !self.goal_objective_is_allowed(objective, validation_source) {
|
||||
return;
|
||||
}
|
||||
let Some(thread_id) = self.thread_id else {
|
||||
if source == SlashCommandDispatchSource::Live {
|
||||
self.queue_user_message_with_options(
|
||||
@@ -792,7 +779,7 @@ impl ChatWidget {
|
||||
},
|
||||
QueuedInputAction::ParseSlash,
|
||||
);
|
||||
self.bottom_pane.drain_pending_submission_state();
|
||||
self.clear_live_goal_submission();
|
||||
} else {
|
||||
self.add_info_message(
|
||||
GOAL_USAGE.to_string(),
|
||||
@@ -808,7 +795,7 @@ impl ChatWidget {
|
||||
});
|
||||
self.append_message_history_entry(format!("/goal {trimmed}"));
|
||||
if source == SlashCommandDispatchSource::Live {
|
||||
self.bottom_pane.drain_pending_submission_state();
|
||||
self.clear_live_goal_submission();
|
||||
}
|
||||
}
|
||||
SlashCommand::Side | SlashCommand::Btw if !trimmed.is_empty() => {
|
||||
@@ -945,11 +932,6 @@ impl ChatWidget {
|
||||
rest_offset + leading_trimmed,
|
||||
&text_elements,
|
||||
);
|
||||
if cmd == SlashCommand::Goal
|
||||
&& !self.goal_objective_is_allowed(trimmed_rest, GoalObjectiveValidationSource::Queued)
|
||||
{
|
||||
return QueueDrain::Continue;
|
||||
}
|
||||
self.dispatch_prepared_command_with_args(
|
||||
cmd,
|
||||
PreparedSlashCommandArgs {
|
||||
|
||||
@@ -227,7 +227,7 @@ mod exec_flow;
|
||||
mod goal_menu;
|
||||
mod goal_validation;
|
||||
mod guardian;
|
||||
mod helpers;
|
||||
pub(crate) mod helpers;
|
||||
mod history_replay;
|
||||
mod mcp_startup;
|
||||
mod permissions;
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
use super::*;
|
||||
use codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS;
|
||||
use codex_protocol::user_input::MAX_USER_INPUT_TEXT_CHARS;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn complete_turn_with_message(chat: &mut ChatWidget, turn_id: &str, message: Option<&str>) {
|
||||
@@ -33,25 +32,22 @@ fn queue_composer_text_with_tab(chat: &mut ChatWidget, text: &str) {
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE));
|
||||
}
|
||||
|
||||
fn drain_app_events(rx: &mut tokio::sync::mpsc::UnboundedReceiver<AppEvent>) -> Vec<AppEvent> {
|
||||
std::iter::from_fn(|| rx.try_recv().ok()).collect()
|
||||
}
|
||||
|
||||
fn rendered_insert_history(events: &[AppEvent]) -> String {
|
||||
events
|
||||
.iter()
|
||||
.filter_map(|event| match event {
|
||||
AppEvent::InsertHistoryCell(cell) => Some(
|
||||
cell.display_lines(/*width*/ 80)
|
||||
.into_iter()
|
||||
.map(|line| line.to_string())
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n"),
|
||||
),
|
||||
_ => None,
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n")
|
||||
fn next_goal_objective(
|
||||
rx: &mut tokio::sync::mpsc::UnboundedReceiver<AppEvent>,
|
||||
expected_thread_id: ThreadId,
|
||||
) -> String {
|
||||
loop {
|
||||
let event = rx.try_recv().expect("expected goal objective event");
|
||||
if let AppEvent::SetThreadGoalObjective {
|
||||
thread_id,
|
||||
objective,
|
||||
..
|
||||
} = event
|
||||
{
|
||||
assert_eq!(thread_id, expected_thread_id);
|
||||
return objective;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -104,40 +100,29 @@ async fn goal_slash_command_accepts_multiline_objective_after_blank_first_line()
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn goal_slash_command_rejects_oversized_objective() {
|
||||
async fn goal_slash_command_emits_oversized_objective() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.set_feature_enabled(Feature::Goals, /*enabled*/ true);
|
||||
chat.thread_id = Some(ThreadId::new());
|
||||
let thread_id = ThreadId::new();
|
||||
chat.thread_id = Some(thread_id);
|
||||
let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1);
|
||||
|
||||
submit_composer_text(&mut chat, &format!("/goal {objective}"));
|
||||
|
||||
let events = drain_app_events(&mut rx);
|
||||
assert!(
|
||||
!events
|
||||
.iter()
|
||||
.any(|event| matches!(event, AppEvent::SetThreadGoalObjective { .. })),
|
||||
"oversized goal should not emit a SetThreadGoalObjective event: {events:?}"
|
||||
);
|
||||
let rendered = rendered_insert_history(&events);
|
||||
assert!(rendered.contains("Goal objective is too long"));
|
||||
assert!(rendered.contains("Put longer instructions in a file"));
|
||||
assert!(
|
||||
!rendered.contains("Message exceeds the maximum length"),
|
||||
"expected goal-specific length error, got {rendered:?}"
|
||||
);
|
||||
assert_eq!(next_goal_objective(&mut rx, thread_id), objective);
|
||||
assert_no_submit_op(&mut op_rx);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn goal_slash_command_rejects_large_paste_using_expanded_length() {
|
||||
async fn goal_slash_command_expands_large_pasted_objective() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.set_feature_enabled(Feature::Goals, /*enabled*/ true);
|
||||
chat.thread_id = Some(ThreadId::new());
|
||||
let thread_id = ThreadId::new();
|
||||
chat.thread_id = Some(thread_id);
|
||||
let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1);
|
||||
chat.bottom_pane
|
||||
.set_composer_text("/goal ".to_string(), Vec::new(), Vec::new());
|
||||
let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1);
|
||||
chat.handle_paste(objective);
|
||||
chat.handle_paste(objective.clone());
|
||||
|
||||
assert!(
|
||||
chat.bottom_pane.composer_text().contains("[Pasted Content"),
|
||||
@@ -145,56 +130,16 @@ async fn goal_slash_command_rejects_large_paste_using_expanded_length() {
|
||||
);
|
||||
submit_current_composer(&mut chat);
|
||||
|
||||
let events = drain_app_events(&mut rx);
|
||||
assert!(
|
||||
!events
|
||||
.iter()
|
||||
.any(|event| matches!(event, AppEvent::SetThreadGoalObjective { .. })),
|
||||
"oversized pasted goal should not emit a SetThreadGoalObjective event: {events:?}"
|
||||
);
|
||||
let rendered = rendered_insert_history(&events);
|
||||
assert!(rendered.contains("Goal objective is too long"));
|
||||
assert!(rendered.contains("Put longer instructions in a file"));
|
||||
assert!(
|
||||
!rendered.contains("Message exceeds the maximum length"),
|
||||
"expected goal-specific length error, got {rendered:?}"
|
||||
);
|
||||
assert_eq!(next_goal_objective(&mut rx, thread_id), objective);
|
||||
assert_no_submit_op(&mut op_rx);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn goal_slash_command_giant_paste_uses_goal_specific_error() {
|
||||
async fn queued_goal_slash_command_emits_oversized_objective_and_stops_queue() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.set_feature_enabled(Feature::Goals, /*enabled*/ true);
|
||||
chat.thread_id = Some(ThreadId::new());
|
||||
chat.bottom_pane
|
||||
.set_composer_text("/goal ".to_string(), Vec::new(), Vec::new());
|
||||
chat.handle_paste("x".repeat(MAX_USER_INPUT_TEXT_CHARS + 1));
|
||||
|
||||
submit_current_composer(&mut chat);
|
||||
|
||||
let events = drain_app_events(&mut rx);
|
||||
assert!(
|
||||
!events
|
||||
.iter()
|
||||
.any(|event| matches!(event, AppEvent::SetThreadGoalObjective { .. })),
|
||||
"giant pasted goal should not emit a SetThreadGoalObjective event: {events:?}"
|
||||
);
|
||||
let rendered = rendered_insert_history(&events);
|
||||
assert!(rendered.contains("Goal objective is too long"));
|
||||
assert!(rendered.contains("Put longer instructions in a file"));
|
||||
assert!(
|
||||
!rendered.contains("Message exceeds the maximum length"),
|
||||
"expected goal-specific length error, got {rendered:?}"
|
||||
);
|
||||
assert_no_submit_op(&mut op_rx);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn queued_goal_slash_command_rejects_oversized_objective_and_drains_next_input() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.set_feature_enabled(Feature::Goals, /*enabled*/ true);
|
||||
chat.thread_id = Some(ThreadId::new());
|
||||
let thread_id = ThreadId::new();
|
||||
chat.thread_id = Some(thread_id);
|
||||
handle_turn_started(&mut chat, "turn-1");
|
||||
let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1);
|
||||
|
||||
@@ -204,26 +149,7 @@ async fn queued_goal_slash_command_rejects_oversized_objective_and_drains_next_i
|
||||
|
||||
complete_turn_with_message(&mut chat, "turn-1", Some("done"));
|
||||
|
||||
let events = drain_app_events(&mut rx);
|
||||
assert!(
|
||||
!events
|
||||
.iter()
|
||||
.any(|event| matches!(event, AppEvent::SetThreadGoalObjective { .. })),
|
||||
"oversized queued goal should not emit a SetThreadGoalObjective event: {events:?}"
|
||||
);
|
||||
let rendered = rendered_insert_history(&events);
|
||||
assert!(rendered.contains("Goal objective is too long"));
|
||||
assert!(rendered.contains("Put longer instructions in a file"));
|
||||
match next_submit_op(&mut op_rx) {
|
||||
Op::UserTurn { items, .. } => assert_eq!(
|
||||
items,
|
||||
vec![UserInput::Text {
|
||||
text: "continue".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}]
|
||||
),
|
||||
other => panic!("expected queued follow-up after oversized goal, got {other:?}"),
|
||||
}
|
||||
assert!(chat.input_queue.queued_user_messages.is_empty());
|
||||
assert_eq!(next_goal_objective(&mut rx, thread_id), objective);
|
||||
assert_eq!(chat.input_queue.queued_user_messages.len(), 1);
|
||||
assert_no_submit_op(&mut op_rx);
|
||||
}
|
||||
|
||||
@@ -1207,7 +1207,7 @@ pub(super) fn render_bottom_first_row(chat: &ChatWidget, width: u16) -> String {
|
||||
String::new()
|
||||
}
|
||||
|
||||
pub(super) fn render_bottom_popup(chat: &ChatWidget, width: u16) -> String {
|
||||
pub(crate) fn render_bottom_popup(chat: &ChatWidget, width: u16) -> String {
|
||||
let height = chat.desired_height(width);
|
||||
let area = Rect::new(0, 0, width, height);
|
||||
let mut buf = Buffer::empty(area);
|
||||
|
||||
Reference in New Issue
Block a user