From dd24ac6b26888a88aeb276ed936c2a86c5ac995b Mon Sep 17 00:00:00 2001 From: mjr-openai Date: Tue, 27 Jan 2026 12:19:43 -0500 Subject: [PATCH 1/7] update pnpm to 10.28.2 to address security issues (#9992) Updates pnpm to 10.28.2. to address security issues in prior versions of pnpm that can allow deps to execute lifecycle scripts against policy. I have read the CLA Document and I hereby sign the CLA --- PNPM.md | 6 +++--- package.json | 2 +- pnpm-workspace.yaml | 4 ++++ 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/PNPM.md b/PNPM.md index 860633c8e1..9f05e39cf8 100644 --- a/PNPM.md +++ b/PNPM.md @@ -15,7 +15,7 @@ This project has been migrated from npm to pnpm to improve dependency management ```bash # Global installation of pnpm -npm install -g pnpm@10.8.1 +npm install -g pnpm@10.28.2 # Or with corepack (available with Node.js 22+) corepack enable @@ -59,12 +59,12 @@ codex/ ## CI/CD -CI/CD workflows have been updated to use pnpm instead of npm. Make sure your CI environments use pnpm 10.8.1 or higher. +CI/CD workflows have been updated to use pnpm instead of npm. Make sure your CI environments use pnpm 10.28.2 or higher. ## Known issues If you encounter issues with pnpm, try the following solutions: 1. Remove the `node_modules` folder and `pnpm-lock.yaml` file, then run `pnpm install` -2. Make sure you're using pnpm 10.8.1 or higher +2. Make sure you're using pnpm 10.28.2 or higher 3. Verify that Node.js 22 or higher is installed diff --git a/package.json b/package.json index ae676d8230..9f1452a972 100644 --- a/package.json +++ b/package.json @@ -21,5 +21,5 @@ "node": ">=22", "pnpm": ">=9.0.0" }, - "packageManager": "pnpm@10.8.1" + "packageManager": "pnpm@10.28.2+sha512.41872f037ad22f7348e3b1debbaf7e867cfd448f2726d9cf74c08f19507c31d2c8e7a11525b983febc2df640b5438dee6023ebb1f84ed43cc2d654d2bc326264" } diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index ddd5674bf1..78899b9321 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -5,3 +5,7 @@ packages: ignoredBuiltDependencies: - esbuild + +minimumReleaseAge: 10080 + +blockExoticSubdeps: true From 067922a734f4abb11ef3db7290daed4feeeda60c Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 27 Jan 2026 17:20:07 +0000 Subject: [PATCH 2/7] description in role type (#9993) --- codex-rs/core/src/agent/role.rs | 38 ++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/src/agent/role.rs b/codex-rs/core/src/agent/role.rs index dd05c18470..7a9dda005b 100644 --- a/codex-rs/core/src/agent/role.rs +++ b/codex-rs/core/src/agent/role.rs @@ -44,6 +44,8 @@ pub struct AgentProfile { pub reasoning_effort: Option, /// Whether to force a read-only sandbox policy. pub read_only: bool, + /// Description to include in the tool specs. + pub description: &'static str, } impl AgentRole { @@ -51,7 +53,19 @@ impl AgentRole { pub fn enum_values() -> Vec { ALL_ROLES .iter() - .filter_map(|role| serde_json::to_string(role).ok()) + .filter_map(|role| { + let description = role.profile().description; + serde_json::to_string(role) + .map(|role| { + let description = if !description.is_empty() { + format!(r#", "description": {description}"#) + } else { + String::new() + }; + format!(r#"{{ "name": {role}{description}}}"#) + }) + .ok() + }) .collect() } @@ -66,11 +80,33 @@ impl AgentRole { AgentRole::Worker => AgentProfile { // base_instructions: Some(WORKER_PROMPT), // model: Some(WORKER_MODEL), + description: r#"Use for execution and production work. +Typical tasks: +- Implement part of a feature +- Fix tests or bugs +- Split large refactors into independent chunks +Rules: +- Explicitly assign **ownership** of the task (files / responsibility). +- Always tell workers they are **not alone in the codebase**, and they should ignore edits made by others without touching them"#, ..Default::default() }, AgentRole::Explorer => AgentProfile { model: Some(EXPLORER_MODEL), reasoning_effort: Some(ReasoningEffort::Low), + description: r#"Use for fast codebase understanding and information gathering. +`explorer` are extremely fast agents so use them as much as you can to speed up the resolution of the global task. +Typical tasks: +- Locate usages of a symbol or concept +- Understand how X is handled in Y +- Review a section of code for issues +- Assess impact of a potential change +Rules: +- Be explicit in what you are looking for. A good usage of `explorer` would mean that don't need to read the same code after the explorer send you the result. +- **Always** prefer asking explorers rather than exploring the codebase yourself. +- Spawn multiple explorers in parallel when useful and wait for all results. +- You can ask the `explorer` to return file name, lines, entire code snippets, ... +- Reuse the same explorer when it is relevant. If later in your process you have more questions on some code an explorer already covered, reuse this same explorer to be more efficient. + "#, ..Default::default() }, } From 538e1059a3b4eaa78b16b04de8de0043b9f21ff8 Mon Sep 17 00:00:00 2001 From: Charley Cunningham Date: Tue, 27 Jan 2026 09:43:09 -0800 Subject: [PATCH 3/7] TUI footer: right-align context and degrade shortcut summary + mode cleanly (#9944) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Refines the bottom footer layout to keep `% context left` right-aligned while making the left side degrade cleanly ## Behavior with empty textarea Full width: Screenshot 2026-01-26 at 2 59 59 PM Less: Screenshot 2026-01-26 at 2 59 48 PM Min width: Screenshot 2026-01-26 at 2 59 33 PM ## Behavior with message in textarea and agent running (steer enabled) Full width: Screenshot 2026-01-26 at 4 33 54 PM Less: Screenshot 2026-01-26 at 4 30 12 PM Less: Screenshot 2026-01-26 at 4 30 51 PM Less: Screenshot 2026-01-26 at 4 30 56 PM Less: Screenshot 2026-01-26 at 4 31 08 PM ## Notes / Edge Cases - In steer mode while typing, the queue hint no longer replaces the mode label; it renders as `tab to queue message · {Mode}`. - Collapse priorities differ by state: - With the queue hint active, `% context left` is hidden before shortening or dropping the queue hint. - In the empty + non-running state, `? for shortcuts` is dropped first, and `% context left` is only shown if `(shift+tab to cycle)` can also fit. - Transient instructional states (`?` overlay, Esc hint, Ctrl+C/D reminders, and flash/override hints) intentionally suppress the mode label (and context) to focus the next action. ## Implementation Notes - Renamed the base footer modes to make the state explicit: `ComposerEmpty` and `ComposerHasDraft`, and compute the base mode directly from emptiness. - Unified collapse behavior in `single_line_footer_layout` for both base modes, with: - Queue-hint behavior that prefers keeping the queue hint over context. - A cycle-hint guard that prevents context from reappearing after `(shift+tab to cycle)` is dropped. - Kept rendering responsibilities explicit: - `single_line_footer_layout` decides what fits. - `render_footer_line` renders a chosen line. - `render_footer_from_props` renders the canonical mode-to-text mapping. - Expanded snapshot coverage: - Added `footer_collapse_snapshots` in `chat_composer.rs` to lock the distinct collapse states across widths. - Consolidated the width-aware snapshot helper usage (e.g., `snapshot_composer_state_with_width`, `snapshot_footer_with_mode_indicator`). --- .codespellrc | 2 +- codex-rs/tui/src/bottom_pane/chat_composer.rs | 316 ++++++++- codex-rs/tui/src/bottom_pane/footer.rs | 610 +++++++++++++++--- ...mposer__tests__backspace_after_pastes.snap | 2 +- ...tom_pane__chat_composer__tests__empty.snap | 2 +- ...er__tests__footer_collapse_empty_full.snap | 13 + ...ollapse_empty_mode_cycle_with_context.snap | 13 + ...apse_empty_mode_cycle_without_context.snap | 13 + ...ests__footer_collapse_empty_mode_only.snap | 13 + ...er__tests__footer_collapse_queue_full.snap | 13 + ...ollapse_queue_message_without_context.snap | 13 + ...ests__footer_collapse_queue_mode_only.snap | 13 + ...ter_collapse_queue_short_with_context.snap | 13 + ..._collapse_queue_short_without_context.snap | 13 + ...ests__footer_mode_hidden_while_typing.snap | 2 +- ...er__tests__image_placeholder_multiple.snap | 3 +- ...oser__tests__image_placeholder_single.snap | 3 +- ...tom_pane__chat_composer__tests__large.snap | 2 +- ...chat_composer__tests__multiple_pastes.snap | 2 +- ...tom_pane__chat_composer__tests__small.snap | 2 +- ...omposer_has_draft_queue_hint_disabled.snap | 5 + ...composer_has_draft_queue_hint_enabled.snap | 5 + ...oter_context_only_queue_hint_disabled.snap | 5 - ...ooter_context_only_queue_hint_enabled.snap | 5 - ...er__tests__footer_context_tokens_used.snap | 2 +- ...r_mode_indicator_narrow_overlap_hides.snap | 2 +- ...ter_mode_indicator_running_hides_hint.snap | 2 +- ...er__tests__footer_mode_indicator_wide.snap | 2 +- ...sts__footer_shortcuts_context_running.snap | 2 +- ...oter__tests__footer_shortcuts_default.snap | 2 +- ...s_visible_when_status_hidden_snapshot.snap | 2 +- ...er_fill_height_without_bottom_padding.snap | 2 +- ...__status_and_queued_messages_snapshot.snap | 2 +- ...tom_pane__tests__status_only_snapshot.snap | 2 +- ..._details_and_queued_messages_snapshot.snap | 2 +- ...exec_and_status_layout_vt100_snapshot.snap | 2 +- ...i__chatwidget__tests__chatwidget_tall.snap | 2 +- ...et__tests__mcp_startup_header_booting.snap | 2 +- ..._review_queues_user_messages_snapshot.snap | 2 +- ...atwidget__tests__status_widget_active.snap | 3 +- 40 files changed, 949 insertions(+), 167 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_empty_full.snap create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_empty_mode_cycle_with_context.snap create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_empty_mode_cycle_without_context.snap create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_empty_mode_only.snap create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_full.snap create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_message_without_context.snap create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_mode_only.snap create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_short_with_context.snap create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_short_without_context.snap create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_composer_has_draft_queue_hint_disabled.snap create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_composer_has_draft_queue_hint_enabled.snap delete mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_context_only_queue_hint_disabled.snap delete mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_context_only_queue_hint_enabled.snap diff --git a/.codespellrc b/.codespellrc index 84b4495e31..a3f0cd501a 100644 --- a/.codespellrc +++ b/.codespellrc @@ -1,6 +1,6 @@ [codespell] # Ref: https://github.com/codespell-project/codespell#using-a-config-file -skip = .git*,vendor,*-lock.yaml,*.lock,.codespellrc,*test.ts,*.jsonl,frame*.txt +skip = .git*,vendor,*-lock.yaml,*.lock,.codespellrc,*test.ts,*.jsonl,frame*.txt,*.snap,*.snap.new check-hidden = true ignore-regex = ^\s*"image/\S+": ".*|\b(afterAll)\b ignore-words-list = ratatui,ser,iTerm,iterm2,iterm diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index c283b10ab1..909061aa68 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -96,15 +96,20 @@ use super::file_search_popup::FileSearchPopup; use super::footer::CollaborationModeIndicator; use super::footer::FooterMode; use super::footer::FooterProps; +use super::footer::SummaryLeft; +use super::footer::can_show_left_with_context; +use super::footer::context_window_line; use super::footer::esc_hint_mode; use super::footer::footer_height; use super::footer::footer_hint_items_width; use super::footer::footer_line_width; use super::footer::inset_footer_hint_area; -use super::footer::render_footer; +use super::footer::render_context_right; +use super::footer::render_footer_from_props; use super::footer::render_footer_hint_items; -use super::footer::render_mode_indicator; +use super::footer::render_footer_line; use super::footer::reset_mode_after_activity; +use super::footer::single_line_footer_layout; use super::footer::toggle_shortcut_mode; use super::paste_burst::CharDecision; use super::paste_burst::PasteBurst; @@ -341,7 +346,7 @@ impl ChatComposer { paste_burst: PasteBurst::default(), disable_paste_burst: false, custom_prompts: Vec::new(), - footer_mode: FooterMode::ShortcutSummary, + footer_mode: FooterMode::ComposerEmpty, footer_hint_override: None, footer_flash: None, context_window_percent: None, @@ -2234,7 +2239,11 @@ impl ChatComposer { return false; } - let next = toggle_shortcut_mode(self.footer_mode, self.quit_shortcut_hint_visible()); + let next = toggle_shortcut_mode( + self.footer_mode, + self.quit_shortcut_hint_visible(), + self.is_empty(), + ); let changed = next != self.footer_mode; self.footer_mode = next; changed @@ -2254,19 +2263,32 @@ impl ChatComposer { } } + /// Resolve the effective footer mode via a small priority waterfall. + /// + /// The base mode is derived solely from whether the composer is empty: + /// `ComposerEmpty` iff empty, otherwise `ComposerHasDraft`. Transient + /// modes (Esc hint, overlay, quit reminder) can override that base when + /// their conditions are active. fn footer_mode(&self) -> FooterMode { + let base_mode = if self.is_empty() { + FooterMode::ComposerEmpty + } else { + FooterMode::ComposerHasDraft + }; + match self.footer_mode { FooterMode::EscHint => FooterMode::EscHint, FooterMode::ShortcutOverlay => FooterMode::ShortcutOverlay, FooterMode::QuitShortcutReminder if self.quit_shortcut_hint_visible() => { FooterMode::QuitShortcutReminder } - FooterMode::QuitShortcutReminder => FooterMode::ShortcutSummary, - FooterMode::ShortcutSummary if self.quit_shortcut_hint_visible() => { + FooterMode::ComposerEmpty | FooterMode::ComposerHasDraft + if self.quit_shortcut_hint_visible() => + { FooterMode::QuitShortcutReminder } - FooterMode::ShortcutSummary if !self.is_empty() => FooterMode::ContextOnly, - other => other, + FooterMode::QuitShortcutReminder => base_mode, + FooterMode::ComposerEmpty | FooterMode::ComposerHasDraft => base_mode, } } @@ -2584,6 +2606,28 @@ impl Renderable for ChatComposer { } ActivePopup::None => { let footer_props = self.footer_props(); + let show_cycle_hint = !footer_props.is_task_running; + let show_shortcuts_hint = match footer_props.mode { + FooterMode::ComposerEmpty => !self.is_in_paste_burst(), + FooterMode::QuitShortcutReminder + | FooterMode::ShortcutOverlay + | FooterMode::EscHint + | FooterMode::ComposerHasDraft => false, + }; + let show_queue_hint = match footer_props.mode { + FooterMode::ComposerHasDraft => { + footer_props.is_task_running && footer_props.steer_enabled + } + FooterMode::QuitShortcutReminder + | FooterMode::ComposerEmpty + | FooterMode::ShortcutOverlay + | FooterMode::EscHint => false, + }; + let context_line = context_window_line( + footer_props.context_window_percent, + footer_props.context_window_used_tokens, + ); + let context_width = context_line.width() as u16; let custom_height = self.custom_footer_height(); let footer_hint_height = custom_height.unwrap_or_else(|| footer_height(footer_props)); @@ -2598,26 +2642,102 @@ impl Renderable for ChatComposer { } else { popup_rect }; - let mut left_content_width = None; - if self.footer_flash_visible() { + let left_width = if self.footer_flash_visible() { + self.footer_flash + .as_ref() + .map(|flash| flash.line.width() as u16) + .unwrap_or(0) + } else if let Some(items) = self.footer_hint_override.as_ref() { + footer_hint_items_width(items) + } else { + footer_line_width( + footer_props, + self.collaboration_mode_indicator, + show_cycle_hint, + show_shortcuts_hint, + show_queue_hint, + ) + }; + let can_show_left_and_context = + can_show_left_with_context(hint_rect, left_width, context_width); + let has_override = + self.footer_flash_visible() || self.footer_hint_override.is_some(); + let single_line_layout = if has_override { + None + } else { + match footer_props.mode { + FooterMode::ComposerEmpty | FooterMode::ComposerHasDraft => { + // Both of these modes render the single-line footer style (with + // either the shortcuts hint or the optional queue hint). We still + // want the single-line collapse rules so the mode label can win over + // the context indicator on narrow widths. + Some(single_line_footer_layout( + hint_rect, + context_width, + self.collaboration_mode_indicator, + show_cycle_hint, + show_shortcuts_hint, + show_queue_hint, + )) + } + FooterMode::EscHint + | FooterMode::QuitShortcutReminder + | FooterMode::ShortcutOverlay => None, + } + }; + let show_context = if matches!( + footer_props.mode, + FooterMode::EscHint + | FooterMode::QuitShortcutReminder + | FooterMode::ShortcutOverlay + ) { + false + } else { + single_line_layout + .as_ref() + .map(|(_, show_context)| *show_context) + .unwrap_or(can_show_left_and_context) + }; + + if let Some((summary_left, _)) = single_line_layout { + match summary_left { + SummaryLeft::Default => { + render_footer_from_props( + hint_rect, + buf, + footer_props, + self.collaboration_mode_indicator, + show_cycle_hint, + show_shortcuts_hint, + show_queue_hint, + ); + } + SummaryLeft::Custom(line) => { + render_footer_line(hint_rect, buf, line); + } + SummaryLeft::None => {} + } + } else if self.footer_flash_visible() { if let Some(flash) = self.footer_flash.as_ref() { flash.line.render(inset_footer_hint_area(hint_rect), buf); - left_content_width = Some(flash.line.width() as u16); } } else if let Some(items) = self.footer_hint_override.as_ref() { render_footer_hint_items(hint_rect, buf, items); - left_content_width = Some(footer_hint_items_width(items)); } else { - render_footer(hint_rect, buf, footer_props); - left_content_width = Some(footer_line_width(footer_props)); + render_footer_from_props( + hint_rect, + buf, + footer_props, + self.collaboration_mode_indicator, + show_cycle_hint, + show_shortcuts_hint, + show_queue_hint, + ); + } + + if show_context { + render_context_right(hint_rect, buf, &context_line); } - render_mode_indicator( - hint_rect, - buf, - self.collaboration_mode_indicator, - !footer_props.is_task_running, - left_content_width, - ); } } let style = user_message_style(); @@ -2647,8 +2767,11 @@ impl Renderable for ChatComposer { .unwrap_or("Input disabled.") .to_string() }; - let placeholder = Span::from(text).dim(); - Line::from(vec![placeholder]).render_ref(textarea_rect.inner(Margin::new(0, 0)), buf); + if !textarea_rect.is_empty() { + let placeholder = Span::from(text).dim(); + Line::from(vec![placeholder]) + .render_ref(textarea_rect.inner(Margin::new(0, 0)), buf); + } } } } @@ -2861,14 +2984,17 @@ mod tests { ); } - fn snapshot_composer_state(name: &str, enhanced_keys_supported: bool, setup: F) - where + fn snapshot_composer_state_with_width( + name: &str, + width: u16, + enhanced_keys_supported: bool, + setup: F, + ) where F: FnOnce(&mut ChatComposer), { use ratatui::Terminal; use ratatui::backend::TestBackend; - let width = 100; let (tx, _rx) = unbounded_channel::(); let sender = AppEventSender::new(tx); let mut composer = ChatComposer::new( @@ -2890,6 +3016,13 @@ mod tests { insta::assert_snapshot!(name, terminal.backend()); } + fn snapshot_composer_state(name: &str, enhanced_keys_supported: bool, setup: F) + where + F: FnOnce(&mut ChatComposer), + { + snapshot_composer_state_with_width(name, 100, enhanced_keys_supported, setup); + } + #[test] fn footer_mode_snapshots() { use crossterm::event::KeyCode; @@ -2942,6 +3075,100 @@ mod tests { }); } + #[test] + fn footer_collapse_snapshots() { + fn setup_collab_footer( + composer: &mut ChatComposer, + context_percent: i64, + indicator: CollaborationModeIndicator, + ) { + composer.set_collaboration_modes_enabled(true); + composer.set_collaboration_mode_indicator(Some(indicator)); + composer.set_context_window(Some(context_percent), None); + } + + // Empty textarea, agent idle: shortcuts hint can show, and cycle hint is available. + snapshot_composer_state_with_width("footer_collapse_empty_full", 120, true, |composer| { + setup_collab_footer(composer, 100, CollaborationModeIndicator::Code); + }); + snapshot_composer_state_with_width( + "footer_collapse_empty_mode_cycle_with_context", + 60, + true, + |composer| { + setup_collab_footer(composer, 100, CollaborationModeIndicator::Code); + }, + ); + snapshot_composer_state_with_width( + "footer_collapse_empty_mode_cycle_without_context", + 44, + true, + |composer| { + setup_collab_footer(composer, 100, CollaborationModeIndicator::Code); + }, + ); + snapshot_composer_state_with_width( + "footer_collapse_empty_mode_only", + 26, + true, + |composer| { + setup_collab_footer(composer, 100, CollaborationModeIndicator::Code); + }, + ); + + // Textarea has content, agent running, steer enabled: queue hint is shown. + snapshot_composer_state_with_width("footer_collapse_queue_full", 120, true, |composer| { + setup_collab_footer(composer, 98, CollaborationModeIndicator::Code); + composer.set_steer_enabled(true); + composer.set_task_running(true); + composer.set_text_content("Test".to_string(), Vec::new(), Vec::new()); + }); + snapshot_composer_state_with_width( + "footer_collapse_queue_short_with_context", + 50, + true, + |composer| { + setup_collab_footer(composer, 98, CollaborationModeIndicator::Code); + composer.set_steer_enabled(true); + composer.set_task_running(true); + composer.set_text_content("Test".to_string(), Vec::new(), Vec::new()); + }, + ); + snapshot_composer_state_with_width( + "footer_collapse_queue_message_without_context", + 40, + true, + |composer| { + setup_collab_footer(composer, 98, CollaborationModeIndicator::Code); + composer.set_steer_enabled(true); + composer.set_task_running(true); + composer.set_text_content("Test".to_string(), Vec::new(), Vec::new()); + }, + ); + snapshot_composer_state_with_width( + "footer_collapse_queue_short_without_context", + 30, + true, + |composer| { + setup_collab_footer(composer, 98, CollaborationModeIndicator::Code); + composer.set_steer_enabled(true); + composer.set_task_running(true); + composer.set_text_content("Test".to_string(), Vec::new(), Vec::new()); + }, + ); + snapshot_composer_state_with_width( + "footer_collapse_queue_mode_only", + 20, + true, + |composer| { + setup_collab_footer(composer, 98, CollaborationModeIndicator::Code); + composer.set_steer_enabled(true); + composer.set_task_running(true); + composer.set_text_content("Test".to_string(), Vec::new(), Vec::new()); + }, + ); + } + #[test] fn esc_hint_stays_hidden_with_draft_content() { use crossterm::event::KeyCode; @@ -2962,15 +3189,40 @@ mod tests { assert!(!composer.is_empty()); assert_eq!(composer.current_text(), "d"); - assert_eq!(composer.footer_mode, FooterMode::ShortcutSummary); + assert_eq!(composer.footer_mode, FooterMode::ComposerEmpty); assert!(matches!(composer.active_popup, ActivePopup::None)); let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); - assert_eq!(composer.footer_mode, FooterMode::ShortcutSummary); + assert_eq!(composer.footer_mode, FooterMode::ComposerEmpty); assert!(!composer.esc_backtrack_hint); } + #[test] + fn base_footer_mode_tracks_empty_state_after_quit_hint_expires() { + use crossterm::event::KeyCode; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + type_chars_humanlike(&mut composer, &['d']); + composer.show_quit_shortcut_hint(key_hint::ctrl(KeyCode::Char('c')), true); + composer.quit_shortcut_expires_at = + Some(Instant::now() - std::time::Duration::from_secs(1)); + + assert_eq!(composer.footer_mode(), FooterMode::ComposerHasDraft); + + composer.set_text_content(String::new(), Vec::new(), Vec::new()); + assert_eq!(composer.footer_mode(), FooterMode::ComposerEmpty); + } + #[test] fn clear_for_ctrl_c_records_cleared_draft() { let (tx, _rx) = unbounded_channel::(); @@ -3031,11 +3283,11 @@ mod tests { // Toggle back to prompt mode so subsequent typing captures characters. let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('?'), KeyModifiers::NONE)); - assert_eq!(composer.footer_mode, FooterMode::ShortcutSummary); + assert_eq!(composer.footer_mode, FooterMode::ComposerEmpty); type_chars_humanlike(&mut composer, &['h']); assert_eq!(composer.textarea.text(), "h"); - assert_eq!(composer.footer_mode(), FooterMode::ContextOnly); + assert_eq!(composer.footer_mode(), FooterMode::ComposerHasDraft); let (result, needs_redraw) = composer.handle_key_event(KeyEvent::new(KeyCode::Char('?'), KeyModifiers::NONE)); @@ -3043,8 +3295,8 @@ mod tests { assert!(needs_redraw, "typing should still mark the view dirty"); let _ = flush_after_paste_burst(&mut composer); assert_eq!(composer.textarea.text(), "h?"); - assert_eq!(composer.footer_mode, FooterMode::ShortcutSummary); - assert_eq!(composer.footer_mode(), FooterMode::ContextOnly); + assert_eq!(composer.footer_mode, FooterMode::ComposerEmpty); + assert_eq!(composer.footer_mode(), FooterMode::ComposerHasDraft); } /// Behavior: while a paste-like burst is being captured, `?` must not toggle the shortcut diff --git a/codex-rs/tui/src/bottom_pane/footer.rs b/codex-rs/tui/src/bottom_pane/footer.rs index 16f4f22a94..69c5e7bb00 100644 --- a/codex-rs/tui/src/bottom_pane/footer.rs +++ b/codex-rs/tui/src/bottom_pane/footer.rs @@ -8,6 +8,30 @@ //! Some footer content is time-based rather than event-based, such as the "press again to quit" //! hint. The owning widgets schedule redraws so time-based hints can expire even if the UI is //! otherwise idle. +//! +//! Single-line collapse overview: +//! 1. The composer decides the current `FooterMode` and hint flags, then calls +//! `single_line_footer_layout` for the base single-line modes. +//! 2. `single_line_footer_layout` applies the width-based fallback rules: +//! (If this description is hard to follow, just try it out by resizing +//! your terminal width; these rules were built out of trial and error.) +//! - Start with the fullest left-side hint plus the right-side context. +//! - When the queue hint is active, prefer keeping that queue hint visible, +//! even if it means dropping the right-side context earlier; the queue +//! hint may also be shortened before it is removed. +//! - When the queue hint is not active but the mode cycle hint is applicable, +//! drop "? for shortcuts" before dropping "(shift+tab to cycle)". +//! - If "(shift+tab to cycle)" cannot fit, also hide the right-side +//! context to avoid too many state transitions in quick succession. +//! - Finally, try a mode-only line (with and without context), and fall +//! back to no left-side footer if nothing can fit. +//! 3. When collapse chooses a specific line, callers render it via +//! `render_footer_line`. Otherwise, callers render the straightforward +//! mode-to-text mapping via `render_footer_from_props`. +//! +//! In short: `single_line_footer_layout` chooses *what* best fits, and the two +//! render helpers choose whether to draw the chosen line or the default +//! `FooterProps` mapping. #[cfg(target_os = "linux")] use crate::clipboard_paste::is_probably_wsl; use crate::key_hint; @@ -27,9 +51,10 @@ use ratatui::widgets::Widget; /// The rendering inputs for the footer area under the composer. /// /// Callers are expected to construct `FooterProps` from higher-level state (`ChatComposer`, -/// `BottomPane`, and `ChatWidget`) and pass it to `render_footer`. The footer treats these values as -/// authoritative and does not attempt to infer missing state (for example, it does not query -/// whether a task is running). +/// `BottomPane`, and `ChatWidget`) and pass it to the footer render helpers +/// (`render_footer_from_props` or the single-line collapse logic). The footer +/// treats these values as authoritative and does not attempt to infer missing +/// state (for example, it does not query whether a task is running). #[derive(Clone, Copy, Debug)] pub(crate) struct FooterProps { pub(crate) mode: FooterMode, @@ -77,7 +102,7 @@ impl CollaborationModeIndicator { let label = self.label(show_cycle_hint); match self { CollaborationModeIndicator::Plan => Span::from(label).magenta(), - CollaborationModeIndicator::Code => Span::from(label).cyan(), + CollaborationModeIndicator::Code => Span::from(label).dim(), CollaborationModeIndicator::PairProgramming => Span::from(label).cyan(), CollaborationModeIndicator::Execute => Span::from(label).dim(), } @@ -92,21 +117,36 @@ impl CollaborationModeIndicator { pub(crate) enum FooterMode { /// Transient "press again to quit" reminder (Ctrl+C/Ctrl+D). QuitShortcutReminder, - ShortcutSummary, + /// Multi-line shortcut overlay shown after pressing `?`. ShortcutOverlay, + /// Transient "press Esc again" hint shown after the first Esc while idle. EscHint, - ContextOnly, + /// Base single-line footer when the composer is empty. + ComposerEmpty, + /// Base single-line footer when the composer contains a draft. + /// + /// The shortcuts hint is suppressed here; when a task is running with + /// steer enabled, this mode can show the queue hint instead. + ComposerHasDraft, } -pub(crate) fn toggle_shortcut_mode(current: FooterMode, ctrl_c_hint: bool) -> FooterMode { +pub(crate) fn toggle_shortcut_mode( + current: FooterMode, + ctrl_c_hint: bool, + is_empty: bool, +) -> FooterMode { if ctrl_c_hint && matches!(current, FooterMode::QuitShortcutReminder) { return current; } + let base_mode = if is_empty { + FooterMode::ComposerEmpty + } else { + FooterMode::ComposerHasDraft + }; + match current { - FooterMode::ShortcutOverlay | FooterMode::QuitShortcutReminder => { - FooterMode::ShortcutSummary - } + FooterMode::ShortcutOverlay | FooterMode::QuitShortcutReminder => base_mode, _ => FooterMode::ShortcutOverlay, } } @@ -124,57 +164,347 @@ pub(crate) fn reset_mode_after_activity(current: FooterMode) -> FooterMode { FooterMode::EscHint | FooterMode::ShortcutOverlay | FooterMode::QuitShortcutReminder - | FooterMode::ContextOnly => FooterMode::ShortcutSummary, + | FooterMode::ComposerHasDraft => FooterMode::ComposerEmpty, other => other, } } pub(crate) fn footer_height(props: FooterProps) -> u16 { - footer_lines(props).len() as u16 + let show_shortcuts_hint = match props.mode { + FooterMode::ComposerEmpty => true, + FooterMode::QuitShortcutReminder + | FooterMode::ShortcutOverlay + | FooterMode::EscHint + | FooterMode::ComposerHasDraft => false, + }; + let show_queue_hint = match props.mode { + FooterMode::ComposerHasDraft => props.is_task_running && props.steer_enabled, + FooterMode::QuitShortcutReminder + | FooterMode::ComposerEmpty + | FooterMode::ShortcutOverlay + | FooterMode::EscHint => false, + }; + footer_from_props_lines(props, None, false, show_shortcuts_hint, show_queue_hint).len() as u16 } -pub(crate) fn render_footer(area: Rect, buf: &mut Buffer, props: FooterProps) { +/// Render a single precomputed footer line. +pub(crate) fn render_footer_line(area: Rect, buf: &mut Buffer, line: Line<'static>) { Paragraph::new(prefix_lines( - footer_lines(props), + vec![line], " ".repeat(FOOTER_INDENT_COLS).into(), " ".repeat(FOOTER_INDENT_COLS).into(), )) .render(area, buf); } -pub(crate) fn render_mode_indicator( +/// Render footer content directly from `FooterProps`. +/// +/// This is intentionally not part of the width-based collapse/fallback logic. +/// Transient instructional states (shortcut overlay, Esc hint, quit reminder) +/// prioritize "what to do next" instructions and currently suppress the +/// collaboration mode label entirely. When collapse logic has already chosen a +/// specific single line, prefer `render_footer_line`. +pub(crate) fn render_footer_from_props( area: Rect, buf: &mut Buffer, - indicator: Option, + props: FooterProps, + collaboration_mode_indicator: Option, show_cycle_hint: bool, - left_content_width: Option, + show_shortcuts_hint: bool, + show_queue_hint: bool, ) { - let Some(indicator) = indicator else { - return; + Paragraph::new(prefix_lines( + footer_from_props_lines( + props, + collaboration_mode_indicator, + show_cycle_hint, + show_shortcuts_hint, + show_queue_hint, + ), + " ".repeat(FOOTER_INDENT_COLS).into(), + " ".repeat(FOOTER_INDENT_COLS).into(), + )) + .render(area, buf); +} + +pub(crate) fn left_fits(area: Rect, left_width: u16) -> bool { + let max_width = area.width.saturating_sub(FOOTER_INDENT_COLS as u16); + left_width <= max_width +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum SummaryHintKind { + None, + Shortcuts, + QueueMessage, + QueueShort, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +struct LeftSideState { + hint: SummaryHintKind, + show_cycle_hint: bool, +} + +fn left_side_line( + collaboration_mode_indicator: Option, + state: LeftSideState, +) -> Line<'static> { + let mut line = Line::from(""); + match state.hint { + SummaryHintKind::None => {} + SummaryHintKind::Shortcuts => { + line.push_span(key_hint::plain(KeyCode::Char('?'))); + line.push_span(" for shortcuts".dim()); + } + SummaryHintKind::QueueMessage => { + line.push_span(key_hint::plain(KeyCode::Tab)); + line.push_span(" to queue message".dim()); + } + SummaryHintKind::QueueShort => { + line.push_span(key_hint::plain(KeyCode::Tab)); + line.push_span(" to queue".dim()); + } }; + + if let Some(collaboration_mode_indicator) = collaboration_mode_indicator { + if !matches!(state.hint, SummaryHintKind::None) { + line.push_span(" · ".dim()); + } + line.push_span(collaboration_mode_indicator.styled_span(state.show_cycle_hint)); + } + + line +} + +pub(crate) enum SummaryLeft { + Default, + Custom(Line<'static>), + None, +} + +/// Compute the single-line footer layout and whether the right-side context +/// indicator can be shown alongside it. +pub(crate) fn single_line_footer_layout( + area: Rect, + context_width: u16, + collaboration_mode_indicator: Option, + show_cycle_hint: bool, + show_shortcuts_hint: bool, + show_queue_hint: bool, +) -> (SummaryLeft, bool) { + let hint_kind = if show_queue_hint { + SummaryHintKind::QueueMessage + } else if show_shortcuts_hint { + SummaryHintKind::Shortcuts + } else { + SummaryHintKind::None + }; + let default_state = LeftSideState { + hint: hint_kind, + show_cycle_hint, + }; + let default_line = left_side_line(collaboration_mode_indicator, default_state); + let default_width = default_line.width() as u16; + if default_width > 0 && can_show_left_with_context(area, default_width, context_width) { + return (SummaryLeft::Default, true); + } + + let state_line = |state: LeftSideState| -> Line<'static> { + if state == default_state { + default_line.clone() + } else { + left_side_line(collaboration_mode_indicator, state) + } + }; + let state_width = |state: LeftSideState| -> u16 { state_line(state).width() as u16 }; + // When the mode cycle hint is applicable (idle, non-queue mode), only show + // the right-side context indicator if the "(shift+tab to cycle)" variant + // can also fit. + let context_requires_cycle_hint = show_cycle_hint && !show_queue_hint; + + if show_queue_hint { + // In queue mode, prefer dropping context before dropping the queue hint. + let queue_states = [ + default_state, + LeftSideState { + hint: SummaryHintKind::QueueMessage, + show_cycle_hint: false, + }, + LeftSideState { + hint: SummaryHintKind::QueueShort, + show_cycle_hint: false, + }, + ]; + + // Pass 1: keep the right-side context indicator if any queue variant + // can fit alongside it. We skip adjacent duplicates because + // `default_state` can already be the no-cycle queue variant. + let mut previous_state: Option = None; + for state in queue_states { + if previous_state == Some(state) { + continue; + } + previous_state = Some(state); + let width = state_width(state); + if width > 0 && can_show_left_with_context(area, width, context_width) { + if state == default_state { + return (SummaryLeft::Default, true); + } + return (SummaryLeft::Custom(state_line(state)), true); + } + } + + // Pass 2: if context cannot fit, drop it before dropping the queue + // hint. Reuse the same dedupe so we do not try equivalent states twice. + let mut previous_state: Option = None; + for state in queue_states { + if previous_state == Some(state) { + continue; + } + previous_state = Some(state); + let width = state_width(state); + if width > 0 && left_fits(area, width) { + if state == default_state { + return (SummaryLeft::Default, false); + } + return (SummaryLeft::Custom(state_line(state)), false); + } + } + } else if collaboration_mode_indicator.is_some() { + if show_cycle_hint { + // First fallback: drop shortcut hint but keep the cycle + // hint on the mode label if it can fit. + let cycle_state = LeftSideState { + hint: SummaryHintKind::None, + show_cycle_hint: true, + }; + let cycle_width = state_width(cycle_state); + if cycle_width > 0 && can_show_left_with_context(area, cycle_width, context_width) { + return (SummaryLeft::Custom(state_line(cycle_state)), true); + } + if cycle_width > 0 && left_fits(area, cycle_width) { + return (SummaryLeft::Custom(state_line(cycle_state)), false); + } + } + + // Next fallback: mode label only. If the cycle hint is applicable but + // cannot fit, we also suppress context so the right side does not + // outlive "(shift+tab to cycle)" on the left. + let mode_only_state = LeftSideState { + hint: SummaryHintKind::None, + show_cycle_hint: false, + }; + let mode_only_width = state_width(mode_only_state); + if !context_requires_cycle_hint + && mode_only_width > 0 + && can_show_left_with_context(area, mode_only_width, context_width) + { + return ( + SummaryLeft::Custom(state_line(mode_only_state)), + true, // show_context + ); + } + if mode_only_width > 0 && left_fits(area, mode_only_width) { + return ( + SummaryLeft::Custom(state_line(mode_only_state)), + false, // show_context + ); + } + } + + // Final fallback: if queue variants (or other earlier states) could not fit + // at all, drop every hint and try to show just the mode label. + if let Some(collaboration_mode_indicator) = collaboration_mode_indicator { + let mode_only_state = LeftSideState { + hint: SummaryHintKind::None, + show_cycle_hint: false, + }; + // Compute the width without going through `state_line` so we do not + // depend on `default_state` (which may still be a queue variant). + let mode_only_width = + left_side_line(Some(collaboration_mode_indicator), mode_only_state).width() as u16; + if !context_requires_cycle_hint + && can_show_left_with_context(area, mode_only_width, context_width) + { + return ( + SummaryLeft::Custom(left_side_line( + Some(collaboration_mode_indicator), + mode_only_state, + )), + true, // show_context + ); + } + if left_fits(area, mode_only_width) { + return ( + SummaryLeft::Custom(left_side_line( + Some(collaboration_mode_indicator), + mode_only_state, + )), + false, // show_context + ); + } + } + + (SummaryLeft::None, true) +} + +fn right_aligned_x(area: Rect, content_width: u16) -> Option { + if area.is_empty() { + return None; + } + + let right_padding = FOOTER_INDENT_COLS as u16; + let max_width = area.width.saturating_sub(right_padding); + if content_width == 0 || max_width == 0 { + return None; + } + + if content_width >= max_width { + return Some(area.x.saturating_add(right_padding)); + } + + Some( + area.x + .saturating_add(area.width) + .saturating_sub(content_width) + .saturating_sub(right_padding), + ) +} + +pub(crate) fn can_show_left_with_context(area: Rect, left_width: u16, context_width: u16) -> bool { + let Some(context_x) = right_aligned_x(area, context_width) else { + return true; + }; + let left_extent = FOOTER_INDENT_COLS as u16 + left_width; + left_extent <= context_x.saturating_sub(area.x) +} + +pub(crate) fn render_context_right(area: Rect, buf: &mut Buffer, line: &Line<'static>) { if area.is_empty() { return; } - let span = indicator.styled_span(show_cycle_hint); - let label_width = span.width() as u16; - if label_width == 0 || label_width > area.width { + let context_width = line.width() as u16; + let Some(mut x) = right_aligned_x(area, context_width) else { return; - } - - let x = area - .x - .saturating_add(area.width) - .saturating_sub(label_width) - .saturating_sub(FOOTER_INDENT_COLS as u16); + }; let y = area.y + area.height.saturating_sub(1); - if let Some(left_content_width) = left_content_width { - let left_extent = FOOTER_INDENT_COLS as u16 + left_content_width; - if left_extent >= x.saturating_sub(area.x) { - return; + let max_x = area.x.saturating_add(area.width); + + for span in &line.spans { + if x >= max_x { + break; } + let span_width = span.width() as u16; + if span_width == 0 { + continue; + } + let remaining = max_x.saturating_sub(x); + let draw_width = span_width.min(remaining); + buf.set_span(x, y, span, draw_width); + x = x.saturating_add(span_width); } - buf.set_span(x, y, &span, label_width); } pub(crate) fn inset_footer_hint_area(mut area: Rect) -> Rect { @@ -193,26 +523,35 @@ pub(crate) fn render_footer_hint_items(area: Rect, buf: &mut Buffer, items: &[(S footer_hint_items_line(items).render(inset_footer_hint_area(area), buf); } -fn footer_lines(props: FooterProps) -> Vec> { - // Show the context indicator on the left, appended after the primary hint - // (e.g., "? for shortcuts"). Keep it visible even when typing (i.e., when - // the shortcut hint is hidden). Hide it only for the multi-line - // ShortcutOverlay. +/// Map `FooterProps` to footer lines without width-based collapse. +/// +/// This is the canonical FooterMode-to-text mapping. It powers transient, +/// instructional states (shortcut overlay, Esc hint, quit reminder) and also +/// the default rendering for base states when collapse is not applied (or when +/// `single_line_footer_layout` returns `SummaryLeft::Default`). Collapse and +/// fallback decisions live in `single_line_footer_layout`; this function only +/// formats the chosen/default content. +fn footer_from_props_lines( + props: FooterProps, + collaboration_mode_indicator: Option, + show_cycle_hint: bool, + show_shortcuts_hint: bool, + show_queue_hint: bool, +) -> Vec> { match props.mode { FooterMode::QuitShortcutReminder => { vec![quit_shortcut_reminder_line(props.quit_shortcut_key)] } - FooterMode::ShortcutSummary => { - let mut line = context_window_line( - props.context_window_percent, - props.context_window_used_tokens, - ); - line.push_span(" · ".dim()); - line.extend(vec![ - key_hint::plain(KeyCode::Char('?')).into(), - " for shortcuts".dim(), - ]); - vec![line] + FooterMode::ComposerEmpty => { + let state = LeftSideState { + hint: if show_shortcuts_hint { + SummaryHintKind::Shortcuts + } else { + SummaryHintKind::None + }, + show_cycle_hint, + }; + vec![left_side_line(collaboration_mode_indicator, state)] } FooterMode::ShortcutOverlay => { #[cfg(target_os = "linux")] @@ -229,26 +568,37 @@ fn footer_lines(props: FooterProps) -> Vec> { shortcut_overlay_lines(state) } FooterMode::EscHint => vec![esc_hint_line(props.esc_backtrack_hint)], - FooterMode::ContextOnly => { - let mut line = context_window_line( - props.context_window_percent, - props.context_window_used_tokens, - ); - if props.is_task_running && props.steer_enabled { - line.push_span(" · ".dim()); - line.push_span(key_hint::plain(KeyCode::Tab)); - line.push_span(" to queue message".dim()); - } - vec![line] + FooterMode::ComposerHasDraft => { + let state = LeftSideState { + hint: if show_queue_hint { + SummaryHintKind::QueueMessage + } else { + SummaryHintKind::None + }, + show_cycle_hint, + }; + vec![left_side_line(collaboration_mode_indicator, state)] } } } -pub(crate) fn footer_line_width(props: FooterProps) -> u16 { - footer_lines(props) - .last() - .map(|line| line.width() as u16) - .unwrap_or(0) +pub(crate) fn footer_line_width( + props: FooterProps, + collaboration_mode_indicator: Option, + show_cycle_hint: bool, + show_shortcuts_hint: bool, + show_queue_hint: bool, +) -> u16 { + footer_from_props_lines( + props, + collaboration_mode_indicator, + show_cycle_hint, + show_shortcuts_hint, + show_queue_hint, + ) + .last() + .map(|line| line.width() as u16) + .unwrap_or(0) } pub(crate) fn footer_hint_items_width(items: &[(String, String)]) -> u16 { @@ -396,7 +746,7 @@ fn build_columns(entries: Vec>) -> Vec> { .collect() } -fn context_window_line(percent: Option, used_tokens: Option) -> Line<'static> { +pub(crate) fn context_window_line(percent: Option, used_tokens: Option) -> Line<'static> { if let Some(percent) = percent { let percent = percent.clamp(0, 100); return Line::from(vec![Span::from(format!("{percent}% context left")).dim()]); @@ -615,36 +965,102 @@ mod tests { use ratatui::backend::TestBackend; fn snapshot_footer(name: &str, props: FooterProps) { - let height = footer_height(props).max(1); - let mut terminal = Terminal::new(TestBackend::new(80, height)).unwrap(); - terminal - .draw(|f| { - let area = Rect::new(0, 0, f.area().width, height); - render_footer(area, f.buffer_mut(), props); - }) - .unwrap(); - assert_snapshot!(name, terminal.backend()); + snapshot_footer_with_mode_indicator(name, 80, props, None); } - fn snapshot_footer_with_indicator( + fn snapshot_footer_with_mode_indicator( name: &str, width: u16, props: FooterProps, - indicator: Option, + collaboration_mode_indicator: Option, ) { let height = footer_height(props).max(1); let mut terminal = Terminal::new(TestBackend::new(width, height)).unwrap(); terminal .draw(|f| { let area = Rect::new(0, 0, f.area().width, height); - render_footer(area, f.buffer_mut(), props); - render_mode_indicator( - area, - f.buffer_mut(), - indicator, - !props.is_task_running, - Some(footer_line_width(props)), + let context_line = context_window_line( + props.context_window_percent, + props.context_window_used_tokens, ); + let context_width = context_line.width() as u16; + let show_cycle_hint = !props.is_task_running; + let show_shortcuts_hint = match props.mode { + FooterMode::ComposerEmpty => true, + FooterMode::QuitShortcutReminder + | FooterMode::ShortcutOverlay + | FooterMode::EscHint + | FooterMode::ComposerHasDraft => false, + }; + let show_queue_hint = match props.mode { + FooterMode::ComposerHasDraft => props.is_task_running && props.steer_enabled, + FooterMode::QuitShortcutReminder + | FooterMode::ComposerEmpty + | FooterMode::ShortcutOverlay + | FooterMode::EscHint => false, + }; + let left_width = footer_line_width( + props, + collaboration_mode_indicator, + show_cycle_hint, + show_shortcuts_hint, + show_queue_hint, + ); + let can_show_left_and_context = + can_show_left_with_context(area, left_width, context_width); + if matches!( + props.mode, + FooterMode::ComposerEmpty | FooterMode::ComposerHasDraft + ) { + let (summary_left, show_context) = single_line_footer_layout( + area, + context_width, + collaboration_mode_indicator, + show_cycle_hint, + show_shortcuts_hint, + show_queue_hint, + ); + match summary_left { + SummaryLeft::Default => { + render_footer_from_props( + area, + f.buffer_mut(), + props, + collaboration_mode_indicator, + show_cycle_hint, + show_shortcuts_hint, + show_queue_hint, + ); + } + SummaryLeft::Custom(line) => { + render_footer_line(area, f.buffer_mut(), line); + } + SummaryLeft::None => {} + } + if show_context { + render_context_right(area, f.buffer_mut(), &context_line); + } + } else { + render_footer_from_props( + area, + f.buffer_mut(), + props, + collaboration_mode_indicator, + show_cycle_hint, + show_shortcuts_hint, + show_queue_hint, + ); + let show_context = can_show_left_and_context + && !matches!( + props.mode, + FooterMode::EscHint + | FooterMode::QuitShortcutReminder + | FooterMode::ShortcutOverlay + ); + if show_context { + render_context_right(area, f.buffer_mut(), &context_line); + } + } }) .unwrap(); assert_snapshot!(name, terminal.backend()); @@ -655,7 +1071,7 @@ mod tests { snapshot_footer( "footer_shortcuts_default", FooterProps { - mode: FooterMode::ShortcutSummary, + mode: FooterMode::ComposerEmpty, esc_backtrack_hint: false, use_shift_enter_hint: false, is_task_running: false, @@ -760,7 +1176,7 @@ mod tests { snapshot_footer( "footer_shortcuts_context_running", FooterProps { - mode: FooterMode::ShortcutSummary, + mode: FooterMode::ComposerEmpty, esc_backtrack_hint: false, use_shift_enter_hint: false, is_task_running: true, @@ -775,7 +1191,7 @@ mod tests { snapshot_footer( "footer_context_tokens_used", FooterProps { - mode: FooterMode::ShortcutSummary, + mode: FooterMode::ComposerEmpty, esc_backtrack_hint: false, use_shift_enter_hint: false, is_task_running: false, @@ -788,9 +1204,9 @@ mod tests { ); snapshot_footer( - "footer_context_only_queue_hint_disabled", + "footer_composer_has_draft_queue_hint_disabled", FooterProps { - mode: FooterMode::ContextOnly, + mode: FooterMode::ComposerHasDraft, esc_backtrack_hint: false, use_shift_enter_hint: false, is_task_running: true, @@ -803,9 +1219,9 @@ mod tests { ); snapshot_footer( - "footer_context_only_queue_hint_enabled", + "footer_composer_has_draft_queue_hint_enabled", FooterProps { - mode: FooterMode::ContextOnly, + mode: FooterMode::ComposerHasDraft, esc_backtrack_hint: false, use_shift_enter_hint: false, is_task_running: true, @@ -818,7 +1234,7 @@ mod tests { ); let props = FooterProps { - mode: FooterMode::ShortcutSummary, + mode: FooterMode::ComposerEmpty, esc_backtrack_hint: false, use_shift_enter_hint: false, is_task_running: false, @@ -829,14 +1245,14 @@ mod tests { context_window_used_tokens: None, }; - snapshot_footer_with_indicator( + snapshot_footer_with_mode_indicator( "footer_mode_indicator_wide", 120, props, Some(CollaborationModeIndicator::Plan), ); - snapshot_footer_with_indicator( + snapshot_footer_with_mode_indicator( "footer_mode_indicator_narrow_overlap_hides", 50, props, @@ -844,7 +1260,7 @@ mod tests { ); let props = FooterProps { - mode: FooterMode::ShortcutSummary, + mode: FooterMode::ComposerEmpty, esc_backtrack_hint: false, use_shift_enter_hint: false, is_task_running: true, @@ -855,7 +1271,7 @@ mod tests { context_window_used_tokens: None, }; - snapshot_footer_with_indicator( + snapshot_footer_with_mode_indicator( "footer_mode_indicator_running_hides_hint", 120, props, diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__backspace_after_pastes.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__backspace_after_pastes.snap index e4cc9ffefd..0b88e19a22 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__backspace_after_pastes.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__backspace_after_pastes.snap @@ -11,4 +11,4 @@ expression: terminal.backend() " " " " " " -" 100% context left " +" 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__empty.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__empty.snap index 53e0aee4cf..47c97c74d2 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__empty.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__empty.snap @@ -11,4 +11,4 @@ expression: terminal.backend() " " " " " " -" 100% context left · ? for shortcuts " +" ? for shortcuts 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_empty_full.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_empty_full.snap new file mode 100644 index 0000000000..90a7b12e4a --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_empty_full.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/bottom_pane/chat_composer.rs +expression: terminal.backend() +--- +" " +"› Ask Codex to do anything " +" " +" " +" " +" " +" " +" " +" ? for shortcuts · Code mode (shift+tab to cycle) 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_empty_mode_cycle_with_context.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_empty_mode_cycle_with_context.snap new file mode 100644 index 0000000000..cfc93be1e6 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_empty_mode_cycle_with_context.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/bottom_pane/chat_composer.rs +expression: terminal.backend() +--- +" " +"› Ask Codex to do anything " +" " +" " +" " +" " +" " +" " +" Code mode (shift+tab to cycle) 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_empty_mode_cycle_without_context.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_empty_mode_cycle_without_context.snap new file mode 100644 index 0000000000..9901a8fbbf --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_empty_mode_cycle_without_context.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/bottom_pane/chat_composer.rs +expression: terminal.backend() +--- +" " +"› Ask Codex to do anything " +" " +" " +" " +" " +" " +" " +" Code mode (shift+tab to cycle) " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_empty_mode_only.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_empty_mode_only.snap new file mode 100644 index 0000000000..98c7c33889 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_empty_mode_only.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/bottom_pane/chat_composer.rs +expression: terminal.backend() +--- +" " +"› Ask Codex to do anythin " +" " +" " +" " +" " +" " +" " +" Code mode " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_full.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_full.snap new file mode 100644 index 0000000000..cfb47fa9d7 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_full.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/bottom_pane/chat_composer.rs +expression: terminal.backend() +--- +" " +"› Test " +" " +" " +" " +" " +" " +" " +" tab to queue message · Code mode 98% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_message_without_context.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_message_without_context.snap new file mode 100644 index 0000000000..0a91488b58 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_message_without_context.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/bottom_pane/chat_composer.rs +expression: terminal.backend() +--- +" " +"› Test " +" " +" " +" " +" " +" " +" " +" tab to queue message · Code mode " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_mode_only.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_mode_only.snap new file mode 100644 index 0000000000..9adcc6e059 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_mode_only.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/bottom_pane/chat_composer.rs +expression: terminal.backend() +--- +" " +"› Test " +" " +" " +" " +" " +" " +" " +" Code mode " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_short_with_context.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_short_with_context.snap new file mode 100644 index 0000000000..0910602616 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_short_with_context.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/bottom_pane/chat_composer.rs +expression: terminal.backend() +--- +" " +"› Test " +" " +" " +" " +" " +" " +" " +" tab to queue · Code mode 98% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_short_without_context.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_short_without_context.snap new file mode 100644 index 0000000000..993dc34035 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_collapse_queue_short_without_context.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/bottom_pane/chat_composer.rs +expression: terminal.backend() +--- +" " +"› Test " +" " +" " +" " +" " +" " +" " +" tab to queue · Code mode " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_mode_hidden_while_typing.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_mode_hidden_while_typing.snap index 67e616e917..5faacfa64f 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_mode_hidden_while_typing.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_mode_hidden_while_typing.snap @@ -10,4 +10,4 @@ expression: terminal.backend() " " " " " " -" 100% context left " +" 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__image_placeholder_multiple.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__image_placeholder_multiple.snap index 3f1adf6291..49eca416c2 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__image_placeholder_multiple.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__image_placeholder_multiple.snap @@ -1,6 +1,5 @@ --- source: tui/src/bottom_pane/chat_composer.rs -assertion_line: 2116 expression: terminal.backend() --- " " @@ -11,4 +10,4 @@ expression: terminal.backend() " " " " " " -" 100% context left " +" 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__image_placeholder_single.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__image_placeholder_single.snap index e46fa0a740..3a5dd7a758 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__image_placeholder_single.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__image_placeholder_single.snap @@ -1,6 +1,5 @@ --- source: tui/src/bottom_pane/chat_composer.rs -assertion_line: 2116 expression: terminal.backend() --- " " @@ -11,4 +10,4 @@ expression: terminal.backend() " " " " " " -" 100% context left " +" 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__large.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__large.snap index 6b018021ec..d2f77dbec3 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__large.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__large.snap @@ -11,4 +11,4 @@ expression: terminal.backend() " " " " " " -" 100% context left " +" 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__multiple_pastes.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__multiple_pastes.snap index 40098faee0..0d16cec0b4 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__multiple_pastes.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__multiple_pastes.snap @@ -11,4 +11,4 @@ expression: terminal.backend() " " " " " " -" 100% context left " +" 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__small.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__small.snap index 498ed76936..8d3f8216db 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__small.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__small.snap @@ -11,4 +11,4 @@ expression: terminal.backend() " " " " " " -" 100% context left " +" 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_composer_has_draft_queue_hint_disabled.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_composer_has_draft_queue_hint_disabled.snap new file mode 100644 index 0000000000..6fdeda07b1 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_composer_has_draft_queue_hint_disabled.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/bottom_pane/footer.rs +expression: terminal.backend() +--- +" 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_composer_has_draft_queue_hint_enabled.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_composer_has_draft_queue_hint_enabled.snap new file mode 100644 index 0000000000..71370d83ba --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_composer_has_draft_queue_hint_enabled.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/bottom_pane/footer.rs +expression: terminal.backend() +--- +" tab to queue message 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_context_only_queue_hint_disabled.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_context_only_queue_hint_disabled.snap deleted file mode 100644 index ce36b2ada8..0000000000 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_context_only_queue_hint_disabled.snap +++ /dev/null @@ -1,5 +0,0 @@ ---- -source: tui/src/bottom_pane/footer.rs -expression: terminal.backend() ---- -" 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_context_only_queue_hint_enabled.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_context_only_queue_hint_enabled.snap deleted file mode 100644 index b9733866d7..0000000000 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_context_only_queue_hint_enabled.snap +++ /dev/null @@ -1,5 +0,0 @@ ---- -source: tui/src/bottom_pane/footer.rs -expression: terminal.backend() ---- -" 100% context left · tab to queue message " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_context_tokens_used.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_context_tokens_used.snap index a77ca5565b..b7ee60704c 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_context_tokens_used.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_context_tokens_used.snap @@ -2,4 +2,4 @@ source: tui/src/bottom_pane/footer.rs expression: terminal.backend() --- -" 123K used · ? for shortcuts " +" ? for shortcuts 123K used " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_mode_indicator_narrow_overlap_hides.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_mode_indicator_narrow_overlap_hides.snap index ed9fea7c88..6266f43d0b 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_mode_indicator_narrow_overlap_hides.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_mode_indicator_narrow_overlap_hides.snap @@ -2,4 +2,4 @@ source: tui/src/bottom_pane/footer.rs expression: terminal.backend() --- -" 100% context left · ? for shortcuts " +" Plan mode (shift+tab to cycle) " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_mode_indicator_running_hides_hint.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_mode_indicator_running_hides_hint.snap index fe5868b87c..9f9be080da 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_mode_indicator_running_hides_hint.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_mode_indicator_running_hides_hint.snap @@ -2,4 +2,4 @@ source: tui/src/bottom_pane/footer.rs expression: terminal.backend() --- -" 100% context left · ? for shortcuts Plan mode " +" ? for shortcuts · Plan mode 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_mode_indicator_wide.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_mode_indicator_wide.snap index 7212d6de51..8c32ee50dc 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_mode_indicator_wide.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_mode_indicator_wide.snap @@ -2,4 +2,4 @@ source: tui/src/bottom_pane/footer.rs expression: terminal.backend() --- -" 100% context left · ? for shortcuts Plan mode (shift+tab to cycle) " +" ? for shortcuts · Plan mode (shift+tab to cycle) 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_shortcuts_context_running.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_shortcuts_context_running.snap index d05ac90a91..2a81b85576 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_shortcuts_context_running.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_shortcuts_context_running.snap @@ -2,4 +2,4 @@ source: tui/src/bottom_pane/footer.rs expression: terminal.backend() --- -" 72% context left · ? for shortcuts " +" ? for shortcuts 72% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_shortcuts_default.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_shortcuts_default.snap index c95a5dc0b3..02804e5735 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_shortcuts_default.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_shortcuts_default.snap @@ -2,4 +2,4 @@ source: tui/src/bottom_pane/footer.rs expression: terminal.backend() --- -" 100% context left · ? for shortcuts " +" ? for shortcuts 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__queued_messages_visible_when_status_hidden_snapshot.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__queued_messages_visible_when_status_hidden_snapshot.snap index 5aea415190..47581631c2 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__queued_messages_visible_when_status_hidden_snapshot.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__queued_messages_visible_when_status_hidden_snapshot.snap @@ -7,4 +7,4 @@ expression: "render_snapshot(&pane, area)" › Ask Codex to do anything - 100% context left · ? for shortcuts + ? for shortcuts 100% context left diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_and_composer_fill_height_without_bottom_padding.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_and_composer_fill_height_without_bottom_padding.snap index 86e3da4573..494883e4c3 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_and_composer_fill_height_without_bottom_padding.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_and_composer_fill_height_without_bottom_padding.snap @@ -7,4 +7,4 @@ expression: "render_snapshot(&pane, area)" › Ask Codex to do anything - 100% context left · ? for sh + 100% context left diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_and_queued_messages_snapshot.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_and_queued_messages_snapshot.snap index e651ec9274..a82f6512cf 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_and_queued_messages_snapshot.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_and_queued_messages_snapshot.snap @@ -9,4 +9,4 @@ expression: "render_snapshot(&pane, area)" › Ask Codex to do anything - 100% context left · ? for shortcuts + ? for shortcuts 100% context left diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_only_snapshot.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_only_snapshot.snap index 79e1e126eb..136c358055 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_only_snapshot.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_only_snapshot.snap @@ -7,4 +7,4 @@ expression: "render_snapshot(&pane, area)" › Ask Codex to do anything - 100% context left · ? for shortcuts + ? for shortcuts 100% context left diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_with_details_and_queued_messages_snapshot.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_with_details_and_queued_messages_snapshot.snap index 12090d09e9..b714c69d88 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_with_details_and_queued_messages_snapshot.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_with_details_and_queued_messages_snapshot.snap @@ -11,4 +11,4 @@ expression: "render_snapshot(&pane, area)" › Ask Codex to do anything - 100% context left · ? for shortcuts + ? for shortcuts 100% context left diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap index b51d759fe7..52779fd840 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap @@ -41,4 +41,4 @@ expression: term.backend().vt100().screen().contents() › Summarize recent commits - 100% context left · tab to queue message + tab to queue message 100% context left diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_tall.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_tall.snap index 64361e90f9..ebffeb8f53 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_tall.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_tall.snap @@ -25,4 +25,4 @@ expression: term.backend().vt100().screen().contents() › Ask Codex to do anything - 100% context left · ? for shortcuts + ? for shortcuts 100% context left diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mcp_startup_header_booting.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mcp_startup_header_booting.snap index c6866c1b51..6074ed1f20 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mcp_startup_header_booting.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mcp_startup_header_booting.snap @@ -8,4 +8,4 @@ expression: terminal.backend() " " "› Ask Codex to do anything " " " -" 100% context left · ? for shortcuts " +" ? for shortcuts 100% context left " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__review_queues_user_messages_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__review_queues_user_messages_snapshot.snap index 1c02350a6d..ce28175ea6 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__review_queues_user_messages_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__review_queues_user_messages_snapshot.snap @@ -19,4 +19,4 @@ expression: term.backend().vt100().screen().contents() › Ask Codex to do anything - 100% context left · ? for shortcuts + ? for shortcuts 100% context left diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap index 9fbebfb500..3acfd95eec 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap @@ -1,6 +1,5 @@ --- source: tui/src/chatwidget/tests.rs -assertion_line: 1577 expression: terminal.backend() --- " " @@ -9,4 +8,4 @@ expression: terminal.backend() " " "› Ask Codex to do anything " " " -" 100% context left · ? for shortcuts " +" ? for shortcuts 100% context left " From 877b76bb9d95969bac667d1facd2b37831588d15 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 27 Jan 2026 10:09:39 -0800 Subject: [PATCH 4/7] feat(network-proxy): add a SOCKS5 proxy with policy enforcement (#9803) ### Summary - Adds an optional SOCKS5 listener via `rama-socks5` - SOCKS5 is disabled by default and gated by config - Reuses existing policy enforcement and blocked-request recording - Blocks SOCKS5 in limited mode to prevent method-policy bypass - Applies bind clamping to the SOCKS5 listener ### Config New/used fields under `network_proxy`: - `enable_socks5` - `socks_url` - `enable_socks5_udp` ### Scope - Changes limited to `codex-rs/network-proxy` (+ `codex-rs/Cargo.lock`) ### Testing ```bash cd codex-rs just fmt cargo test -p codex-network-proxy --offline --- codex-rs/Cargo.lock | 28 +++ codex-rs/network-proxy/Cargo.toml | 1 + codex-rs/network-proxy/README.md | 17 +- codex-rs/network-proxy/src/config.rs | 50 ++++- codex-rs/network-proxy/src/lib.rs | 1 + codex-rs/network-proxy/src/proxy.rs | 56 ++++- codex-rs/network-proxy/src/socks5.rs | 318 +++++++++++++++++++++++++++ 7 files changed, 452 insertions(+), 19 deletions(-) create mode 100644 codex-rs/network-proxy/src/socks5.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 75d72d360f..43916d1e14 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1683,6 +1683,7 @@ dependencies = [ "rama-http", "rama-http-backend", "rama-net", + "rama-socks5", "rama-tcp", "rama-tls-boring", "rama-unix", @@ -5959,6 +5960,21 @@ dependencies = [ "tokio", ] +[[package]] +name = "rama-socks5" +version = "0.3.0-alpha.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5468b263516daaf258de32542c1974b7cbe962363ad913dcb669f5d46db0ef3e" +dependencies = [ + "byteorder", + "rama-core", + "rama-net", + "rama-tcp", + "rama-udp", + "rama-utils", + "tokio", +] + [[package]] name = "rama-tcp" version = "0.3.0-alpha.4" @@ -5997,6 +6013,18 @@ dependencies = [ "tokio", ] +[[package]] +name = "rama-udp" +version = "0.3.0-alpha.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "36ed05e0ecac73e084e92a3a8b1fbf16fdae8958c506f0f0eada180a2d99eef4" +dependencies = [ + "rama-core", + "rama-net", + "tokio", + "tokio-util", +] + [[package]] name = "rama-unix" version = "0.3.0-alpha.4" diff --git a/codex-rs/network-proxy/Cargo.toml b/codex-rs/network-proxy/Cargo.toml index 2d303b1c95..959837d4e2 100644 --- a/codex-rs/network-proxy/Cargo.toml +++ b/codex-rs/network-proxy/Cargo.toml @@ -34,6 +34,7 @@ rama-core = { version = "=0.3.0-alpha.4" } rama-http = { version = "=0.3.0-alpha.4" } rama-http-backend = { version = "=0.3.0-alpha.4", features = ["tls"] } rama-net = { version = "=0.3.0-alpha.4", features = ["http", "tls"] } +rama-socks5 = { version = "=0.3.0-alpha.4" } rama-tcp = { version = "=0.3.0-alpha.4", features = ["http"] } rama-tls-boring = { version = "=0.3.0-alpha.4", features = ["http"] } diff --git a/codex-rs/network-proxy/README.md b/codex-rs/network-proxy/README.md index 1d19a92a68..3d8c20307d 100644 --- a/codex-rs/network-proxy/README.md +++ b/codex-rs/network-proxy/README.md @@ -3,6 +3,7 @@ `codex-network-proxy` is Codex's local network policy enforcement proxy. It runs: - an HTTP proxy (default `127.0.0.1:3128`) +- an optional SOCKS5 proxy (default `127.0.0.1:8081`, disabled by default) - an admin HTTP API (default `127.0.0.1:8080`) It enforces an allow/deny policy and a "limited" mode intended for read-only network access. @@ -20,6 +21,10 @@ Example config: enabled = true proxy_url = "http://127.0.0.1:3128" admin_url = "http://127.0.0.1:8080" +# Optional SOCKS5 listener (disabled by default). +enable_socks5 = false +socks_url = "http://127.0.0.1:8081" +enable_socks5_udp = false # When `enabled` is false, the proxy no-ops and does not bind listeners. # When true, respect HTTP(S)_PROXY/ALL_PROXY for upstream requests (HTTP(S) proxies only), # including CONNECT tunnels in full mode. @@ -28,7 +33,7 @@ allow_upstream_proxy = false # If you want to expose these listeners beyond localhost, you must opt in explicitly. dangerously_allow_non_loopback_proxy = false dangerously_allow_non_loopback_admin = false -mode = "limited" # or "full" +mode = "full" # default when unset; use "limited" for read-only mode [network_proxy.policy] # Hosts must match the allowlist (unless denied). @@ -60,6 +65,12 @@ export HTTP_PROXY="http://127.0.0.1:3128" export HTTPS_PROXY="http://127.0.0.1:3128" ``` +For SOCKS5 traffic (when `enable_socks5 = true`): + +```bash +export ALL_PROXY="socks5h://127.0.0.1:8081" +``` + ### 4) Understand blocks / debugging When a request is blocked, the proxy responds with `403` and includes: @@ -70,8 +81,8 @@ When a request is blocked, the proxy responds with `403` and includes: - `blocked-by-method-policy` - `blocked-by-policy` -In "limited" mode, only `GET`, `HEAD`, and `OPTIONS` are allowed for plain HTTP. HTTPS `CONNECT` -remains a transparent tunnel, so limited-mode method enforcement does not apply to HTTPS. +In "limited" mode, only `GET`, `HEAD`, and `OPTIONS` are allowed. HTTPS `CONNECT` and SOCKS5 are +blocked because they would bypass method enforcement. ## Library API diff --git a/codex-rs/network-proxy/src/config.rs b/codex-rs/network-proxy/src/config.rs index 7bd6957ac7..ae4a450179 100644 --- a/codex-rs/network-proxy/src/config.rs +++ b/codex-rs/network-proxy/src/config.rs @@ -23,6 +23,12 @@ pub struct NetworkProxySettings { #[serde(default = "default_admin_url")] pub admin_url: String, #[serde(default)] + pub enable_socks5: bool, + #[serde(default = "default_socks_url")] + pub socks_url: String, + #[serde(default)] + pub enable_socks5_udp: bool, + #[serde(default)] pub allow_upstream_proxy: bool, #[serde(default)] pub dangerously_allow_non_loopback_proxy: bool, @@ -40,6 +46,9 @@ impl Default for NetworkProxySettings { enabled: false, proxy_url: default_proxy_url(), admin_url: default_admin_url(), + enable_socks5: false, + socks_url: default_socks_url(), + enable_socks5_udp: false, allow_upstream_proxy: false, dangerously_allow_non_loopback_proxy: false, dangerously_allow_non_loopback_admin: false, @@ -90,6 +99,10 @@ fn default_admin_url() -> String { "http://127.0.0.1:8080".to_string() } +fn default_socks_url() -> String { + "http://127.0.0.1:8081".to_string() +} + /// Clamp non-loopback bind addresses to loopback unless explicitly allowed. fn clamp_non_loopback(addr: SocketAddr, allow_non_loopback: bool, name: &str) -> SocketAddr { if addr.ip().is_loopback() { @@ -110,21 +123,27 @@ fn clamp_non_loopback(addr: SocketAddr, allow_non_loopback: bool, name: &str) -> pub(crate) fn clamp_bind_addrs( http_addr: SocketAddr, + socks_addr: SocketAddr, admin_addr: SocketAddr, cfg: &NetworkProxySettings, -) -> (SocketAddr, SocketAddr) { +) -> (SocketAddr, SocketAddr, SocketAddr) { let http_addr = clamp_non_loopback( http_addr, cfg.dangerously_allow_non_loopback_proxy, "HTTP proxy", ); + let socks_addr = clamp_non_loopback( + socks_addr, + cfg.dangerously_allow_non_loopback_proxy, + "SOCKS5 proxy", + ); let admin_addr = clamp_non_loopback( admin_addr, cfg.dangerously_allow_non_loopback_admin, "admin API", ); if cfg.policy.allow_unix_sockets.is_empty() { - return (http_addr, admin_addr); + return (http_addr, socks_addr, admin_addr); } // `x-unix-socket` is intentionally a local escape hatch. If the proxy (or admin API) is @@ -136,6 +155,11 @@ pub(crate) fn clamp_bind_addrs( "unix socket proxying is enabled; ignoring dangerously_allow_non_loopback_proxy and clamping HTTP proxy to loopback" ); } + if cfg.dangerously_allow_non_loopback_proxy && !socks_addr.ip().is_loopback() { + warn!( + "unix socket proxying is enabled; ignoring dangerously_allow_non_loopback_proxy and clamping SOCKS5 proxy to loopback" + ); + } if cfg.dangerously_allow_non_loopback_admin && !admin_addr.ip().is_loopback() { warn!( "unix socket proxying is enabled; ignoring dangerously_allow_non_loopback_admin and clamping admin API to loopback" @@ -143,12 +167,14 @@ pub(crate) fn clamp_bind_addrs( } ( SocketAddr::from(([127, 0, 0, 1], http_addr.port())), + SocketAddr::from(([127, 0, 0, 1], socks_addr.port())), SocketAddr::from(([127, 0, 0, 1], admin_addr.port())), ) } pub struct RuntimeConfig { pub http_addr: SocketAddr, + pub socks_addr: SocketAddr, pub admin_addr: SocketAddr, } @@ -159,16 +185,24 @@ pub fn resolve_runtime(cfg: &NetworkProxyConfig) -> Result { cfg.network_proxy.proxy_url ) })?; + let socks_addr = resolve_addr(&cfg.network_proxy.socks_url, 8081).with_context(|| { + format!( + "invalid network_proxy.socks_url: {}", + cfg.network_proxy.socks_url + ) + })?; let admin_addr = resolve_addr(&cfg.network_proxy.admin_url, 8080).with_context(|| { format!( "invalid network_proxy.admin_url: {}", cfg.network_proxy.admin_url ) })?; - let (http_addr, admin_addr) = clamp_bind_addrs(http_addr, admin_addr, &cfg.network_proxy); + let (http_addr, socks_addr, admin_addr) = + clamp_bind_addrs(http_addr, socks_addr, admin_addr, &cfg.network_proxy); Ok(RuntimeConfig { http_addr, + socks_addr, admin_addr, }) } @@ -403,11 +437,14 @@ mod tests { ..Default::default() }; let http_addr = "0.0.0.0:3128".parse::().unwrap(); + let socks_addr = "0.0.0.0:8081".parse::().unwrap(); let admin_addr = "0.0.0.0:8080".parse::().unwrap(); - let (http_addr, admin_addr) = clamp_bind_addrs(http_addr, admin_addr, &cfg); + let (http_addr, socks_addr, admin_addr) = + clamp_bind_addrs(http_addr, socks_addr, admin_addr, &cfg); assert_eq!(http_addr, "0.0.0.0:3128".parse::().unwrap()); + assert_eq!(socks_addr, "0.0.0.0:8081".parse::().unwrap()); assert_eq!(admin_addr, "0.0.0.0:8080".parse::().unwrap()); } @@ -423,11 +460,14 @@ mod tests { ..Default::default() }; let http_addr = "0.0.0.0:3128".parse::().unwrap(); + let socks_addr = "0.0.0.0:8081".parse::().unwrap(); let admin_addr = "0.0.0.0:8080".parse::().unwrap(); - let (http_addr, admin_addr) = clamp_bind_addrs(http_addr, admin_addr, &cfg); + let (http_addr, socks_addr, admin_addr) = + clamp_bind_addrs(http_addr, socks_addr, admin_addr, &cfg); assert_eq!(http_addr, "127.0.0.1:3128".parse::().unwrap()); + assert_eq!(socks_addr, "127.0.0.1:8081".parse::().unwrap()); assert_eq!(admin_addr, "127.0.0.1:8080".parse::().unwrap()); } } diff --git a/codex-rs/network-proxy/src/lib.rs b/codex-rs/network-proxy/src/lib.rs index 71e4ec7356..f4bcb8c74b 100644 --- a/codex-rs/network-proxy/src/lib.rs +++ b/codex-rs/network-proxy/src/lib.rs @@ -9,6 +9,7 @@ mod proxy; mod reasons; mod responses; mod runtime; +mod socks5; mod state; mod upstream; diff --git a/codex-rs/network-proxy/src/proxy.rs b/codex-rs/network-proxy/src/proxy.rs index 480880e946..cb09b9959d 100644 --- a/codex-rs/network-proxy/src/proxy.rs +++ b/codex-rs/network-proxy/src/proxy.rs @@ -3,6 +3,7 @@ use crate::config; use crate::http_proxy; use crate::network_policy::NetworkPolicyDecider; use crate::runtime::unix_socket_permissions_supported; +use crate::socks5; use crate::state::NetworkProxyState; use anyhow::Context; use anyhow::Result; @@ -61,8 +62,9 @@ impl NetworkProxyBuilder { let current_cfg = state.current_cfg().await?; let runtime = config::resolve_runtime(¤t_cfg)?; // Reapply bind clamping for caller overrides so unix-socket proxying stays loopback-only. - let (http_addr, admin_addr) = config::clamp_bind_addrs( + let (http_addr, socks_addr, admin_addr) = config::clamp_bind_addrs( self.http_addr.unwrap_or(runtime.http_addr), + runtime.socks_addr, self.admin_addr.unwrap_or(runtime.admin_addr), ¤t_cfg.network_proxy, ); @@ -70,6 +72,7 @@ impl NetworkProxyBuilder { Ok(NetworkProxy { state, http_addr, + socks_addr, admin_addr, policy_decider: self.policy_decider, }) @@ -80,6 +83,7 @@ impl NetworkProxyBuilder { pub struct NetworkProxy { state: Arc, http_addr: SocketAddr, + socks_addr: SocketAddr, admin_addr: SocketAddr, policy_decider: Option>, } @@ -105,10 +109,21 @@ impl NetworkProxy { self.http_addr, self.policy_decider.clone(), )); + let socks_task = if current_cfg.network_proxy.enable_socks5 { + Some(tokio::spawn(socks5::run_socks5( + self.state.clone(), + self.socks_addr, + self.policy_decider.clone(), + current_cfg.network_proxy.enable_socks5_udp, + ))) + } else { + None + }; let admin_task = tokio::spawn(admin::run_admin_api(self.state.clone(), self.admin_addr)); Ok(NetworkProxyHandle { http_task: Some(http_task), + socks_task, admin_task: Some(admin_task), completed: false, }) @@ -117,6 +132,7 @@ impl NetworkProxy { pub struct NetworkProxyHandle { http_task: Option>>, + socks_task: Option>>, admin_task: Option>>, completed: bool, } @@ -125,6 +141,7 @@ impl NetworkProxyHandle { fn noop() -> Self { Self { http_task: Some(tokio::spawn(async { Ok(()) })), + socks_task: None, admin_task: Some(tokio::spawn(async { Ok(()) })), completed: true, } @@ -133,33 +150,49 @@ impl NetworkProxyHandle { pub async fn wait(mut self) -> Result<()> { let http_task = self.http_task.take().context("missing http proxy task")?; let admin_task = self.admin_task.take().context("missing admin proxy task")?; + let socks_task = self.socks_task.take(); let http_result = http_task.await; let admin_result = admin_task.await; + let socks_result = match socks_task { + Some(task) => Some(task.await), + None => None, + }; self.completed = true; http_result??; admin_result??; + if let Some(socks_result) = socks_result { + socks_result??; + } Ok(()) } pub async fn shutdown(mut self) -> Result<()> { - abort_tasks(self.http_task.take(), self.admin_task.take()).await; + abort_tasks( + self.http_task.take(), + self.socks_task.take(), + self.admin_task.take(), + ) + .await; self.completed = true; Ok(()) } } +async fn abort_task(task: Option>>) { + if let Some(task) = task { + task.abort(); + let _ = task.await; + } +} + async fn abort_tasks( http_task: Option>>, + socks_task: Option>>, admin_task: Option>>, ) { - if let Some(http_task) = http_task { - http_task.abort(); - let _ = http_task.await; - } - if let Some(admin_task) = admin_task { - admin_task.abort(); - let _ = admin_task.await; - } + abort_task(http_task).await; + abort_task(socks_task).await; + abort_task(admin_task).await; } impl Drop for NetworkProxyHandle { @@ -168,9 +201,10 @@ impl Drop for NetworkProxyHandle { return; } let http_task = self.http_task.take(); + let socks_task = self.socks_task.take(); let admin_task = self.admin_task.take(); tokio::spawn(async move { - abort_tasks(http_task, admin_task).await; + abort_tasks(http_task, socks_task, admin_task).await; }); } } diff --git a/codex-rs/network-proxy/src/socks5.rs b/codex-rs/network-proxy/src/socks5.rs new file mode 100644 index 0000000000..9426502544 --- /dev/null +++ b/codex-rs/network-proxy/src/socks5.rs @@ -0,0 +1,318 @@ +use crate::config::NetworkMode; +use crate::network_policy::NetworkDecision; +use crate::network_policy::NetworkPolicyDecider; +use crate::network_policy::NetworkPolicyRequest; +use crate::network_policy::NetworkProtocol; +use crate::network_policy::evaluate_host_policy; +use crate::policy::normalize_host; +use crate::reasons::REASON_METHOD_NOT_ALLOWED; +use crate::reasons::REASON_PROXY_DISABLED; +use crate::state::BlockedRequest; +use crate::state::NetworkProxyState; +use anyhow::Context as _; +use anyhow::Result; +use rama_core::Layer; +use rama_core::Service; +use rama_core::error::BoxError; +use rama_core::extensions::ExtensionsRef; +use rama_core::layer::AddInputExtensionLayer; +use rama_core::service::service_fn; +use rama_net::client::EstablishedClientConnection; +use rama_net::stream::SocketInfo; +use rama_socks5::Socks5Acceptor; +use rama_socks5::server::DefaultConnector; +use rama_socks5::server::DefaultUdpRelay; +use rama_socks5::server::udp::RelayRequest; +use rama_socks5::server::udp::RelayResponse; +use rama_tcp::TcpStream; +use rama_tcp::client::Request as TcpRequest; +use rama_tcp::client::service::TcpConnector; +use rama_tcp::server::TcpListener; +use std::io; +use std::net::SocketAddr; +use std::sync::Arc; +use tracing::error; +use tracing::info; +use tracing::warn; + +pub async fn run_socks5( + state: Arc, + addr: SocketAddr, + policy_decider: Option>, + enable_socks5_udp: bool, +) -> Result<()> { + let listener = TcpListener::build() + .bind(addr) + .await + // See `http_proxy.rs` for details on why we wrap `BoxError` before converting to anyhow. + .map_err(rama_core::error::OpaqueError::from) + .map_err(anyhow::Error::from) + .with_context(|| format!("bind SOCKS5 proxy: {addr}"))?; + + info!("SOCKS5 proxy listening on {addr}"); + + match state.network_mode().await { + Ok(NetworkMode::Limited) => { + info!("SOCKS5 is blocked in limited mode; set mode=\"full\" to allow SOCKS5"); + } + Ok(NetworkMode::Full) => {} + Err(err) => { + warn!("failed to read network mode: {err}"); + } + } + + let tcp_connector = TcpConnector::default(); + let policy_tcp_connector = service_fn({ + let policy_decider = policy_decider.clone(); + move |req: TcpRequest| { + let tcp_connector = tcp_connector.clone(); + let policy_decider = policy_decider.clone(); + async move { handle_socks5_tcp(req, tcp_connector, policy_decider).await } + } + }); + + let socks_connector = DefaultConnector::default().with_connector(policy_tcp_connector); + let base = Socks5Acceptor::new().with_connector(socks_connector); + + if enable_socks5_udp { + let udp_state = state.clone(); + let udp_decider = policy_decider.clone(); + let udp_relay = DefaultUdpRelay::default().with_async_inspector(service_fn({ + move |request: RelayRequest| { + let udp_state = udp_state.clone(); + let udp_decider = udp_decider.clone(); + async move { inspect_socks5_udp(request, udp_state, udp_decider).await } + } + })); + let socks_acceptor = base.with_udp_associator(udp_relay); + listener + .serve(AddInputExtensionLayer::new(state).into_layer(socks_acceptor)) + .await; + } else { + listener + .serve(AddInputExtensionLayer::new(state).into_layer(base)) + .await; + } + Ok(()) +} + +async fn handle_socks5_tcp( + req: TcpRequest, + tcp_connector: TcpConnector, + policy_decider: Option>, +) -> Result, BoxError> { + let app_state = req + .extensions() + .get::>() + .cloned() + .ok_or_else(|| io::Error::other("missing state"))?; + + let host = normalize_host(&req.authority.host.to_string()); + let port = req.authority.port; + if host.is_empty() { + return Err(io::Error::new(io::ErrorKind::InvalidInput, "invalid host").into()); + } + + let client = req + .extensions() + .get::() + .map(|info| info.peer_addr().to_string()); + + match app_state.enabled().await { + Ok(true) => {} + Ok(false) => { + let _ = app_state + .record_blocked(BlockedRequest::new( + host.clone(), + REASON_PROXY_DISABLED.to_string(), + client.clone(), + None, + None, + "socks5".to_string(), + )) + .await; + let client = client.as_deref().unwrap_or_default(); + warn!("SOCKS blocked; proxy disabled (client={client}, host={host})"); + return Err(io::Error::new(io::ErrorKind::PermissionDenied, "proxy disabled").into()); + } + Err(err) => { + error!("failed to read enabled state: {err}"); + return Err(io::Error::other("proxy error").into()); + } + } + + match app_state.network_mode().await { + Ok(NetworkMode::Limited) => { + let _ = app_state + .record_blocked(BlockedRequest::new( + host.clone(), + REASON_METHOD_NOT_ALLOWED.to_string(), + client.clone(), + None, + Some(NetworkMode::Limited), + "socks5".to_string(), + )) + .await; + let client = client.as_deref().unwrap_or_default(); + warn!( + "SOCKS blocked by method policy (client={client}, host={host}, mode=limited, allowed_methods=GET, HEAD, OPTIONS)" + ); + return Err(io::Error::new(io::ErrorKind::PermissionDenied, "blocked").into()); + } + Ok(NetworkMode::Full) => {} + Err(err) => { + error!("failed to evaluate method policy: {err}"); + return Err(io::Error::other("proxy error").into()); + } + } + + let request = NetworkPolicyRequest::new( + NetworkProtocol::Socks5Tcp, + host.clone(), + port, + client.clone(), + None, + None, + None, + ); + + match evaluate_host_policy(&app_state, policy_decider.as_ref(), &request).await { + Ok(NetworkDecision::Deny { reason }) => { + let _ = app_state + .record_blocked(BlockedRequest::new( + host.clone(), + reason.clone(), + client.clone(), + None, + None, + "socks5".to_string(), + )) + .await; + let client = client.as_deref().unwrap_or_default(); + warn!("SOCKS blocked (client={client}, host={host}, reason={reason})"); + return Err(io::Error::new(io::ErrorKind::PermissionDenied, "blocked").into()); + } + Ok(NetworkDecision::Allow) => { + let client = client.as_deref().unwrap_or_default(); + info!("SOCKS allowed (client={client}, host={host}, port={port})"); + } + Err(err) => { + error!("failed to evaluate host: {err}"); + return Err(io::Error::other("proxy error").into()); + } + } + + tcp_connector.serve(req).await +} + +async fn inspect_socks5_udp( + request: RelayRequest, + state: Arc, + policy_decider: Option>, +) -> io::Result { + let RelayRequest { + server_address, + payload, + extensions, + .. + } = request; + + let host = normalize_host(&server_address.ip_addr.to_string()); + let port = server_address.port; + if host.is_empty() { + return Err(io::Error::new(io::ErrorKind::InvalidInput, "invalid host")); + } + + let client = extensions + .get::() + .map(|info| info.peer_addr().to_string()); + + match state.enabled().await { + Ok(true) => {} + Ok(false) => { + let _ = state + .record_blocked(BlockedRequest::new( + host.clone(), + REASON_PROXY_DISABLED.to_string(), + client.clone(), + None, + None, + "socks5-udp".to_string(), + )) + .await; + let client = client.as_deref().unwrap_or_default(); + warn!("SOCKS UDP blocked; proxy disabled (client={client}, host={host})"); + return Ok(RelayResponse { + maybe_payload: None, + extensions, + }); + } + Err(err) => { + error!("failed to read enabled state: {err}"); + return Err(io::Error::other("proxy error")); + } + } + + match state.network_mode().await { + Ok(NetworkMode::Limited) => { + let _ = state + .record_blocked(BlockedRequest::new( + host.clone(), + REASON_METHOD_NOT_ALLOWED.to_string(), + client.clone(), + None, + Some(NetworkMode::Limited), + "socks5-udp".to_string(), + )) + .await; + return Ok(RelayResponse { + maybe_payload: None, + extensions, + }); + } + Ok(NetworkMode::Full) => {} + Err(err) => { + error!("failed to evaluate method policy: {err}"); + return Err(io::Error::other("proxy error")); + } + } + + let request = NetworkPolicyRequest::new( + NetworkProtocol::Socks5Udp, + host.clone(), + port, + client.clone(), + None, + None, + None, + ); + + match evaluate_host_policy(&state, policy_decider.as_ref(), &request).await { + Ok(NetworkDecision::Deny { reason }) => { + let _ = state + .record_blocked(BlockedRequest::new( + host.clone(), + reason.clone(), + client.clone(), + None, + None, + "socks5-udp".to_string(), + )) + .await; + let client = client.as_deref().unwrap_or_default(); + warn!("SOCKS UDP blocked (client={client}, host={host}, reason={reason})"); + Ok(RelayResponse { + maybe_payload: None, + extensions, + }) + } + Ok(NetworkDecision::Allow) => Ok(RelayResponse { + maybe_payload: Some(payload), + extensions, + }), + Err(err) => { + error!("failed to evaluate UDP host: {err}"); + Err(io::Error::other("proxy error")) + } + } +} From fc0fd85349429a90589e887095e853ace236a541 Mon Sep 17 00:00:00 2001 From: Owen Lin Date: Tue, 27 Jan 2026 10:41:54 -0800 Subject: [PATCH 5/7] fix(app-server, core): defer initial context write to rollout file until first turn (#9950) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Overview Currently calling `thread/resume` will always bump the thread's `updated_at` timestamp. This PR makes it the `updated_at` timestamp changes only if a turn is triggered. ### Additonal context What we typically do on resuming a thread is **always** writing “initial context” to the rollout file immediately. This initial context includes: - Developer instructions derived from sandbox/approval policy + cwd - Optional developer instructions (if provided) - Optional collaboration-mode instructions - Optional user instructions (if provided) - Environment context (cwd, shell, etc.) This PR defers writing the “initial context” to the rollout file until the first `turn/start`, so we don't inadvertently bump the thread's `updated_at` timestamp until a turn is actually triggered. This works even though both `thread/resume` and `turn/start` accept overrides (such as `model`, `cwd`, etc.) because the initial context is seeded from the effective `TurnContext` in memory, computed at `turn/start` time, after both sets of overrides have been applied. **NOTE**: This is a very short-lived solution until we introduce sqlite. Then we can remove this. --- .../tests/suite/v2/thread_resume.rs | 162 ++++++++++++++++++ codex-rs/core/src/codex.rs | 116 +++++++++++-- codex-rs/core/src/state/session.rs | 6 + codex-rs/core/src/tasks/mod.rs | 2 + 4 files changed, 269 insertions(+), 17 deletions(-) diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index 6129c0de2f..af746b37b7 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -2,7 +2,9 @@ use anyhow::Result; use app_test_support::McpProcess; use app_test_support::create_fake_rollout_with_text_elements; use app_test_support::create_mock_responses_server_repeating_assistant; +use app_test_support::rollout_path; use app_test_support::to_response; +use chrono::Utc; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::SessionSource; @@ -22,6 +24,8 @@ use codex_protocol::user_input::TextElement; use core_test_support::responses; use core_test_support::skip_if_no_network; use pretty_assertions::assert_eq; +use std::fs::FileTimes; +use std::path::Path; use std::path::PathBuf; use tempfile::TempDir; use tokio::time::timeout; @@ -147,6 +151,116 @@ async fn thread_resume_returns_rollout_history() -> Result<()> { Ok(()) } +#[tokio::test] +async fn thread_resume_without_overrides_does_not_change_updated_at_or_mtime() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + let codex_home = TempDir::new()?; + let rollout = setup_rollout_fixture(codex_home.path(), &server.uri())?; + let thread_id = rollout.conversation_id.clone(); + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let resume_id = mcp + .send_thread_resume_request(ThreadResumeParams { + thread_id: thread_id.clone(), + ..Default::default() + }) + .await?; + let resume_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(resume_id)), + ) + .await??; + let ThreadResumeResponse { thread, .. } = to_response::(resume_resp)?; + + assert_eq!(thread.updated_at, rollout.expected_updated_at); + + let after_modified = std::fs::metadata(&rollout.rollout_file_path)?.modified()?; + assert_eq!(after_modified, rollout.before_modified); + + let turn_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id, + input: vec![UserInput::Text { + text: "Hello".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), + ) + .await??; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + let after_turn_modified = std::fs::metadata(&rollout.rollout_file_path)?.modified()?; + assert!(after_turn_modified > rollout.before_modified); + + Ok(()) +} + +#[tokio::test] +async fn thread_resume_with_overrides_defers_updated_at_until_turn_start() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + let codex_home = TempDir::new()?; + let rollout = setup_rollout_fixture(codex_home.path(), &server.uri())?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let resume_id = mcp + .send_thread_resume_request(ThreadResumeParams { + thread_id: rollout.conversation_id.clone(), + model: Some("mock-model".to_string()), + ..Default::default() + }) + .await?; + let resume_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(resume_id)), + ) + .await??; + let ThreadResumeResponse { thread, .. } = to_response::(resume_resp)?; + + assert_eq!(thread.updated_at, rollout.expected_updated_at); + + let after_resume_modified = std::fs::metadata(&rollout.rollout_file_path)?.modified()?; + assert_eq!(after_resume_modified, rollout.before_modified); + + let turn_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: rollout.conversation_id, + input: vec![UserInput::Text { + text: "Hello".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), + ) + .await??; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + let after_turn_modified = std::fs::metadata(&rollout.rollout_file_path)?.modified()?; + assert!(after_turn_modified > rollout.before_modified); + + Ok(()) +} + #[tokio::test] async fn thread_resume_prefers_path_over_thread_id() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; @@ -364,3 +478,51 @@ stream_max_retries = 0 ), ) } + +fn set_rollout_mtime(path: &Path, updated_at_rfc3339: &str) -> Result<()> { + let parsed = chrono::DateTime::parse_from_rfc3339(updated_at_rfc3339)?.with_timezone(&Utc); + let times = FileTimes::new().set_modified(parsed.into()); + std::fs::OpenOptions::new() + .append(true) + .open(path)? + .set_times(times)?; + Ok(()) +} + +struct RolloutFixture { + conversation_id: String, + rollout_file_path: PathBuf, + before_modified: std::time::SystemTime, + expected_updated_at: i64, +} + +fn setup_rollout_fixture(codex_home: &Path, server_uri: &str) -> Result { + create_config_toml(codex_home, server_uri)?; + + let preview = "Saved user message"; + let filename_ts = "2025-01-05T12-00-00"; + let meta_rfc3339 = "2025-01-05T12:00:00Z"; + let expected_updated_at_rfc3339 = "2025-01-07T00:00:00Z"; + let conversation_id = create_fake_rollout_with_text_elements( + codex_home, + filename_ts, + meta_rfc3339, + preview, + Vec::new(), + Some("mock_provider"), + None, + )?; + let rollout_file_path = rollout_path(codex_home, filename_ts, &conversation_id); + set_rollout_mtime(rollout_file_path.as_path(), expected_updated_at_rfc3339)?; + let before_modified = std::fs::metadata(&rollout_file_path)?.modified()?; + let expected_updated_at = chrono::DateTime::parse_from_rfc3339(expected_updated_at_rfc3339)? + .with_timezone(&Utc) + .timestamp(); + + Ok(RolloutFixture { + conversation_id, + rollout_file_path, + before_modified, + expected_updated_at, + }) +} diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 41e148e113..26cfdefea4 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -933,23 +933,28 @@ impl Session { // Build and record initial items (user instructions + environment context) let items = self.build_initial_context(&turn_context).await; self.record_conversation_items(&turn_context, &items).await; + { + let mut state = self.state.lock().await; + state.initial_context_seeded = true; + } // Ensure initial items are visible to immediate readers (e.g., tests, forks). self.flush_rollout().await; } - InitialHistory::Resumed(_) | InitialHistory::Forked(_) => { - let rollout_items = conversation_history.get_rollout_items(); - let persist = matches!(conversation_history, InitialHistory::Forked(_)); + InitialHistory::Resumed(resumed_history) => { + let rollout_items = resumed_history.history; + { + let mut state = self.state.lock().await; + state.initial_context_seeded = false; + } // If resuming, warn when the last recorded model differs from the current one. - if let InitialHistory::Resumed(_) = conversation_history - && let Some(prev) = rollout_items.iter().rev().find_map(|it| { - if let RolloutItem::TurnContext(ctx) = it { - Some(ctx.model.as_str()) - } else { - None - } - }) - { + if let Some(prev) = rollout_items.iter().rev().find_map(|it| { + if let RolloutItem::TurnContext(ctx) = it { + Some(ctx.model.as_str()) + } else { + None + } + }) { let curr = turn_context.client.get_model(); if prev != curr { warn!( @@ -984,8 +989,29 @@ impl Session { state.set_token_info(Some(info)); } + // Defer seeding the session's initial context until the first turn starts so + // turn/start overrides can be merged before we write to the rollout. + self.flush_rollout().await; + } + InitialHistory::Forked(rollout_items) => { + // Always add response items to conversation history + let reconstructed_history = self + .reconstruct_history_from_rollout(&turn_context, &rollout_items) + .await; + if !reconstructed_history.is_empty() { + self.record_into_history(&reconstructed_history, &turn_context) + .await; + } + + // Seed usage info from the recorded rollout so UIs can show token counts + // immediately on resume/fork. + if let Some(info) = Self::last_token_info_from_rollout(&rollout_items) { + let mut state = self.state.lock().await; + state.set_token_info(Some(info)); + } + // If persisting, persist all rollout items as-is (recorder filters) - if persist && !rollout_items.is_empty() { + if !rollout_items.is_empty() { self.persist_rollout_items(&rollout_items).await; } @@ -993,6 +1019,10 @@ impl Session { let initial_context = self.build_initial_context(&turn_context).await; self.record_conversation_items(&turn_context, &initial_context) .await; + { + let mut state = self.state.lock().await; + state.initial_context_seeded = true; + } // Flush after seeding history and any persisted rollout copy. self.flush_rollout().await; } @@ -1643,6 +1673,21 @@ impl Session { state.replace_history(items); } + pub(crate) async fn seed_initial_context_if_needed(&self, turn_context: &TurnContext) { + { + let mut state = self.state.lock().await; + if state.initial_context_seeded { + return; + } + state.initial_context_seeded = true; + } + + let initial_context = self.build_initial_context(turn_context).await; + self.record_conversation_items(turn_context, &initial_context) + .await; + self.flush_rollout().await; + } + async fn persist_rollout_response_items(&self, items: &[ResponseItem]) { let rollout_items: Vec = items .iter() @@ -2332,6 +2377,11 @@ mod handlers { return; } + let initial_context_seeded = sess.state.lock().await.initial_context_seeded; + if !initial_context_seeded { + return; + } + let current_context = sess.new_default_turn_with_sub_id(sub_id).await; let update_items = sess.build_settings_update_items( Some(&previous_context), @@ -2419,6 +2469,7 @@ mod handlers { // Attempt to inject input into current task if let Err(items) = sess.inject_input(items).await { + sess.seed_initial_context_if_needed(¤t_context).await; let update_items = sess.build_settings_update_items( previous_context.as_ref(), ¤t_context, @@ -3719,6 +3770,23 @@ mod tests { #[tokio::test] async fn record_initial_history_reconstructs_resumed_transcript() { + let (session, turn_context) = make_session_and_context().await; + let (rollout_items, expected) = sample_rollout(&session, &turn_context).await; + + session + .record_initial_history(InitialHistory::Resumed(ResumedHistory { + conversation_id: ThreadId::default(), + history: rollout_items, + rollout_path: PathBuf::from("/tmp/resume.jsonl"), + })) + .await; + + let history = session.state.lock().await.clone_history(); + assert_eq!(expected, history.raw_items()); + } + + #[tokio::test] + async fn resumed_history_seeds_initial_context_on_first_turn_only() { let (session, turn_context) = make_session_and_context().await; let (rollout_items, mut expected) = sample_rollout(&session, &turn_context).await; @@ -3730,9 +3798,17 @@ mod tests { })) .await; + let history_before_seed = session.state.lock().await.clone_history(); + assert_eq!(expected, history_before_seed.raw_items()); + + session.seed_initial_context_if_needed(&turn_context).await; expected.extend(session.build_initial_context(&turn_context).await); - let history = session.state.lock().await.clone_history(); - assert_eq!(expected, history.raw_items()); + let history_after_seed = session.clone_history().await; + assert_eq!(expected, history_after_seed.raw_items()); + + session.seed_initial_context_if_needed(&turn_context).await; + let history_after_second_seed = session.clone_history().await; + assert_eq!(expected, history_after_second_seed.raw_items()); } #[tokio::test] @@ -4347,7 +4423,8 @@ mod tests { session_configuration.session_source.clone(), ); - let state = SessionState::new(session_configuration.clone()); + let mut state = SessionState::new(session_configuration.clone()); + mark_state_initial_context_seeded(&mut state); let skills_manager = Arc::new(SkillsManager::new(config.codex_home.clone())); let services = SessionServices { @@ -4456,7 +4533,8 @@ mod tests { session_configuration.session_source.clone(), ); - let state = SessionState::new(session_configuration.clone()); + let mut state = SessionState::new(session_configuration.clone()); + mark_state_initial_context_seeded(&mut state); let skills_manager = Arc::new(SkillsManager::new(config.codex_home.clone())); let services = SessionServices { @@ -4502,6 +4580,10 @@ mod tests { (session, turn_context, rx_event) } + fn mark_state_initial_context_seeded(state: &mut SessionState) { + state.initial_context_seeded = true; + } + #[tokio::test] async fn refresh_mcp_servers_is_deferred_until_next_turn() { let (session, turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/state/session.rs b/codex-rs/core/src/state/session.rs index 746396949e..067f7b3378 100644 --- a/codex-rs/core/src/state/session.rs +++ b/codex-rs/core/src/state/session.rs @@ -15,6 +15,11 @@ pub(crate) struct SessionState { pub(crate) history: ContextManager, pub(crate) latest_rate_limits: Option, pub(crate) server_reasoning_included: bool, + /// Whether the session's initial context has been seeded into history. + /// + /// TODO(owen): This is a temporary solution to avoid updating a thread's updated_at + /// timestamp when resuming a session. Remove this once SQLite is in place. + pub(crate) initial_context_seeded: bool, } impl SessionState { @@ -26,6 +31,7 @@ impl SessionState { history, latest_rate_limits: None, server_reasoning_included: false, + initial_context_seeded: false, } } diff --git a/codex-rs/core/src/tasks/mod.rs b/codex-rs/core/src/tasks/mod.rs index bbf9d9c276..08d23f7987 100644 --- a/codex-rs/core/src/tasks/mod.rs +++ b/codex-rs/core/src/tasks/mod.rs @@ -115,6 +115,8 @@ impl Session { task: T, ) { self.abort_all_tasks(TurnAbortReason::Replaced).await; + self.seed_initial_context_if_needed(turn_context.as_ref()) + .await; let task: Arc = Arc::new(task); let task_kind = task.kind(); From 894923ed5d6d659987396bbead8db1cb71f76225 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 27 Jan 2026 10:47:07 -0800 Subject: [PATCH 6/7] feat: make it possible to specify --config flags in the SDK (#10003) Updates the `CodexOptions` passed to the `Codex()` constructor in the SDK to support a `config` property that is a map of configuration data that will be transformed into `--config` flags passed to the invocation of `codex`. Therefore, something like this: ```typescript const codex = new Codex({ config: { show_raw_agent_reasoning: true, sandbox_workspace_write: { network_access: true }, }, }); ``` would result in the following args being added to the invocation of `codex`: ```shell --config show_raw_agent_reasoning=true --config sandbox_workspace_write.network_access=true ``` --- sdk/typescript/README.md | 16 +++++ sdk/typescript/src/codex.ts | 3 +- sdk/typescript/src/codexOptions.ts | 12 ++++ sdk/typescript/src/exec.ts | 110 +++++++++++++++++++++++++++-- sdk/typescript/tests/run.test.ts | 110 ++++++++++++++++++++++++++++- 5 files changed, 240 insertions(+), 11 deletions(-) diff --git a/sdk/typescript/README.md b/sdk/typescript/README.md index 09e8a513d3..ae87b63fab 100644 --- a/sdk/typescript/README.md +++ b/sdk/typescript/README.md @@ -131,3 +131,19 @@ const codex = new Codex({ The SDK still injects its required variables (such as `OPENAI_BASE_URL` and `CODEX_API_KEY`) on top of the environment you provide. + +### Passing `--config` overrides + +Use the `config` option to provide additional Codex CLI configuration overrides. The SDK accepts a JSON object, flattens it +into dotted paths, and serializes values as TOML literals before passing them as repeated `--config key=value` flags. + +```typescript +const codex = new Codex({ + config: { + show_raw_agent_reasoning: true, + sandbox_workspace_write: { network_access: true }, + }, +}); +``` + +Thread options still take precedence for overlapping settings because they are emitted after these global overrides. diff --git a/sdk/typescript/src/codex.ts b/sdk/typescript/src/codex.ts index a42159232e..e3ce4aa0de 100644 --- a/sdk/typescript/src/codex.ts +++ b/sdk/typescript/src/codex.ts @@ -13,7 +13,8 @@ export class Codex { private options: CodexOptions; constructor(options: CodexOptions = {}) { - this.exec = new CodexExec(options.codexPathOverride, options.env); + const { codexPathOverride, env, config } = options; + this.exec = new CodexExec(codexPathOverride, env, config); this.options = options; } diff --git a/sdk/typescript/src/codexOptions.ts b/sdk/typescript/src/codexOptions.ts index 31fb637d4c..b6abb94b79 100644 --- a/sdk/typescript/src/codexOptions.ts +++ b/sdk/typescript/src/codexOptions.ts @@ -1,7 +1,19 @@ +export type CodexConfigValue = string | number | boolean | CodexConfigValue[] | CodexConfigObject; + +export type CodexConfigObject = { [key: string]: CodexConfigValue }; + export type CodexOptions = { codexPathOverride?: string; baseUrl?: string; apiKey?: string; + /** + * Additional `--config key=value` overrides to pass to the Codex CLI. + * + * Provide a JSON object and the SDK will flatten it into dotted paths and + * serialize values as TOML literals so they are compatible with the CLI's + * `--config` parsing. + */ + config?: CodexConfigObject; /** * Environment variables passed to the Codex CLI process. When provided, the SDK * will not inherit variables from `process.env`. diff --git a/sdk/typescript/src/exec.ts b/sdk/typescript/src/exec.ts index c8e957e1f2..d569106c84 100644 --- a/sdk/typescript/src/exec.ts +++ b/sdk/typescript/src/exec.ts @@ -3,12 +3,8 @@ import path from "node:path"; import readline from "node:readline"; import { fileURLToPath } from "node:url"; -import { - SandboxMode, - ModelReasoningEffort, - ApprovalMode, - WebSearchMode, -} from "./threadOptions"; +import type { CodexConfigObject, CodexConfigValue } from "./codexOptions"; +import { SandboxMode, ModelReasoningEffort, ApprovalMode, WebSearchMode } from "./threadOptions"; export type CodexExecArgs = { input: string; @@ -49,15 +45,27 @@ const TYPESCRIPT_SDK_ORIGINATOR = "codex_sdk_ts"; export class CodexExec { private executablePath: string; private envOverride?: Record; + private configOverrides?: CodexConfigObject; - constructor(executablePath: string | null = null, env?: Record) { + constructor( + executablePath: string | null = null, + env?: Record, + configOverrides?: CodexConfigObject, + ) { this.executablePath = executablePath || findCodexPath(); this.envOverride = env; + this.configOverrides = configOverrides; } async *run(args: CodexExecArgs): AsyncGenerator { const commandArgs: string[] = ["exec", "--experimental-json"]; + if (this.configOverrides) { + for (const override of serializeConfigOverrides(this.configOverrides)) { + commandArgs.push("--config", override); + } + } + if (args.model) { commandArgs.push("--model", args.model); } @@ -202,6 +210,94 @@ export class CodexExec { } } +function serializeConfigOverrides(configOverrides: CodexConfigObject): string[] { + const overrides: string[] = []; + flattenConfigOverrides(configOverrides, "", overrides); + return overrides; +} + +function flattenConfigOverrides( + value: CodexConfigValue, + prefix: string, + overrides: string[], +): void { + if (!isPlainObject(value)) { + if (prefix) { + overrides.push(`${prefix}=${toTomlValue(value, prefix)}`); + return; + } else { + throw new Error("Codex config overrides must be a plain object"); + } + } + + const entries = Object.entries(value); + if (!prefix && entries.length === 0) { + return; + } + + if (prefix && entries.length === 0) { + overrides.push(`${prefix}={}`); + return; + } + + for (const [key, child] of entries) { + if (!key) { + throw new Error("Codex config override keys must be non-empty strings"); + } + if (child === undefined) { + continue; + } + const path = prefix ? `${prefix}.${key}` : key; + if (isPlainObject(child)) { + flattenConfigOverrides(child, path, overrides); + } else { + overrides.push(`${path}=${toTomlValue(child, path)}`); + } + } +} + +function toTomlValue(value: CodexConfigValue, path: string): string { + if (typeof value === "string") { + return JSON.stringify(value); + } else if (typeof value === "number") { + if (!Number.isFinite(value)) { + throw new Error(`Codex config override at ${path} must be a finite number`); + } + return `${value}`; + } else if (typeof value === "boolean") { + return value ? "true" : "false"; + } else if (Array.isArray(value)) { + const rendered = value.map((item, index) => toTomlValue(item, `${path}[${index}]`)); + return `[${rendered.join(", ")}]`; + } else if (isPlainObject(value)) { + const parts: string[] = []; + for (const [key, child] of Object.entries(value)) { + if (!key) { + throw new Error("Codex config override keys must be non-empty strings"); + } + if (child === undefined) { + continue; + } + parts.push(`${formatTomlKey(key)} = ${toTomlValue(child, `${path}.${key}`)}`); + } + return `{${parts.join(", ")}}`; + } else if (value === null) { + throw new Error(`Codex config override at ${path} cannot be null`); + } else { + const typeName = typeof value; + throw new Error(`Unsupported Codex config override value at ${path}: ${typeName}`); + } +} + +const TOML_BARE_KEY = /^[A-Za-z0-9_-]+$/; +function formatTomlKey(key: string): string { + return TOML_BARE_KEY.test(key) ? key : JSON.stringify(key); +} + +function isPlainObject(value: unknown): value is CodexConfigObject { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + const scriptFileName = fileURLToPath(import.meta.url); const scriptDirName = path.dirname(scriptFileName); diff --git a/sdk/typescript/tests/run.test.ts b/sdk/typescript/tests/run.test.ts index 993b2304d5..410bf50267 100644 --- a/sdk/typescript/tests/run.test.ts +++ b/sdk/typescript/tests/run.test.ts @@ -410,6 +410,86 @@ describe("Codex", () => { } }); + it("passes CodexOptions config overrides as TOML --config flags", async () => { + const { url, close } = await startResponsesTestProxy({ + statusCode: 200, + responseBodies: [ + sse( + responseStarted("response_1"), + assistantMessage("Config overrides applied", "item_1"), + responseCompleted("response_1"), + ), + ], + }); + + const { args: spawnArgs, restore } = codexExecSpy(); + + try { + const client = new Codex({ + codexPathOverride: codexExecPath, + baseUrl: url, + apiKey: "test", + config: { + approval_policy: "never", + sandbox_workspace_write: { network_access: true }, + retry_budget: 3, + tool_rules: { allow: ["git status", "git diff"] }, + }, + }); + + const thread = client.startThread(); + await thread.run("apply config overrides"); + + const commandArgs = spawnArgs[0]; + expect(commandArgs).toBeDefined(); + expectPair(commandArgs, ["--config", 'approval_policy="never"']); + expectPair(commandArgs, ["--config", "sandbox_workspace_write.network_access=true"]); + expectPair(commandArgs, ["--config", "retry_budget=3"]); + expectPair(commandArgs, ["--config", 'tool_rules.allow=["git status", "git diff"]']); + } finally { + restore(); + await close(); + } + }); + + it("lets thread options override CodexOptions config overrides", async () => { + const { url, close } = await startResponsesTestProxy({ + statusCode: 200, + responseBodies: [ + sse( + responseStarted("response_1"), + assistantMessage("Thread overrides applied", "item_1"), + responseCompleted("response_1"), + ), + ], + }); + + const { args: spawnArgs, restore } = codexExecSpy(); + + try { + const client = new Codex({ + codexPathOverride: codexExecPath, + baseUrl: url, + apiKey: "test", + config: { approval_policy: "never" }, + }); + + const thread = client.startThread({ approvalPolicy: "on-request" }); + await thread.run("override approval policy"); + + const commandArgs = spawnArgs[0]; + const approvalPolicyOverrides = collectConfigValues(commandArgs, "approval_policy"); + expect(approvalPolicyOverrides).toEqual([ + 'approval_policy="never"', + 'approval_policy="on-request"', + ]); + expect(approvalPolicyOverrides.at(-1)).toBe('approval_policy="on-request"'); + } finally { + restore(); + await close(); + } + }); + it("allows overriding the env passed to the Codex CLI", async () => { const { url, close } = await startResponsesTestProxy({ statusCode: 200, @@ -737,13 +817,37 @@ describe("Codex", () => { } }, 10000); // TODO(pakrym): remove timeout }); + +/** + * Given a list of args to `codex` and a `key`, collects all `--config` + * overrides for that key. + */ +function collectConfigValues(args: string[] | undefined, key: string): string[] { + if (!args) { + throw new Error("args is undefined"); + } + + const values: string[] = []; + for (let i = 0; i < args.length; i += 1) { + if (args[i] !== "--config") { + continue; + } + + const override = args[i + 1]; + if (override?.startsWith(`${key}=`)) { + values.push(override); + } + } + return values; +} + function expectPair(args: string[] | undefined, pair: [string, string]) { if (!args) { - throw new Error("Args is undefined"); + throw new Error("args is undefined"); } - const index = args.indexOf(pair[0]); + const index = args.findIndex((arg, i) => arg === pair[0] && args[i + 1] === pair[1]); if (index === -1) { - throw new Error(`Pair ${pair[0]} not found in args`); + throw new Error(`Pair ${pair[0]} ${pair[1]} not found in args`); } expect(args[index + 1]).toBe(pair[1]); } From ccc4ec9a11397bf5a12a816cce6082ad52669c38 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 27 Jan 2026 10:47:31 -0800 Subject: [PATCH 7/7] chore: introduce *Args types for new() methods --- codex-rs/network-proxy/src/http_proxy.rs | 110 ++++++++-------- codex-rs/network-proxy/src/lib.rs | 1 + codex-rs/network-proxy/src/network_policy.rs | 83 +++++++----- codex-rs/network-proxy/src/runtime.rs | 26 ++-- codex-rs/network-proxy/src/socks5.rs | 130 ++++++++++--------- codex-rs/network-proxy/src/state.rs | 1 + 6 files changed, 189 insertions(+), 162 deletions(-) diff --git a/codex-rs/network-proxy/src/http_proxy.rs b/codex-rs/network-proxy/src/http_proxy.rs index abe6b30144..b2856ac601 100644 --- a/codex-rs/network-proxy/src/http_proxy.rs +++ b/codex-rs/network-proxy/src/http_proxy.rs @@ -2,6 +2,7 @@ use crate::config::NetworkMode; use crate::network_policy::NetworkDecision; use crate::network_policy::NetworkPolicyDecider; use crate::network_policy::NetworkPolicyRequest; +use crate::network_policy::NetworkPolicyRequestArgs; use crate::network_policy::NetworkProtocol; use crate::network_policy::evaluate_host_policy; use crate::policy::normalize_host; @@ -12,6 +13,7 @@ use crate::responses::blocked_header_value; use crate::responses::json_response; use crate::runtime::unix_socket_permissions_supported; use crate::state::BlockedRequest; +use crate::state::BlockedRequestArgs; use crate::state::NetworkProxyState; use crate::upstream::UpstreamClient; use crate::upstream::proxy_for_connect; @@ -146,27 +148,27 @@ async fn http_connect_accept( .await); } - let request = NetworkPolicyRequest::new( - NetworkProtocol::HttpsConnect, - host.clone(), - authority.port, - client.clone(), - Some("CONNECT".to_string()), - None, - None, - ); + let request = NetworkPolicyRequest::new(NetworkPolicyRequestArgs { + protocol: NetworkProtocol::HttpsConnect, + host: host.clone(), + port: authority.port, + client_addr: client.clone(), + method: Some("CONNECT".to_string()), + command: None, + exec_policy_hint: None, + }); match evaluate_host_policy(&app_state, policy_decider.as_ref(), &request).await { Ok(NetworkDecision::Deny { reason }) => { let _ = app_state - .record_blocked(BlockedRequest::new( - host.clone(), - reason.clone(), - client.clone(), - Some("CONNECT".to_string()), - None, - "http-connect".to_string(), - )) + .record_blocked(BlockedRequest::new(BlockedRequestArgs { + host: host.clone(), + reason: reason.clone(), + client: client.clone(), + method: Some("CONNECT".to_string()), + mode: None, + protocol: "http-connect".to_string(), + })) .await; let client = client.as_deref().unwrap_or_default(); warn!("CONNECT blocked (client={client}, host={host}, reason={reason})"); @@ -189,14 +191,14 @@ async fn http_connect_accept( if mode == NetworkMode::Limited { let _ = app_state - .record_blocked(BlockedRequest::new( - host.clone(), - REASON_METHOD_NOT_ALLOWED.to_string(), - client.clone(), - Some("CONNECT".to_string()), - Some(NetworkMode::Limited), - "http-connect".to_string(), - )) + .record_blocked(BlockedRequest::new(BlockedRequestArgs { + host: host.clone(), + reason: REASON_METHOD_NOT_ALLOWED.to_string(), + client: client.clone(), + method: Some("CONNECT".to_string()), + mode: Some(NetworkMode::Limited), + protocol: "http-connect".to_string(), + })) .await; let client = client.as_deref().unwrap_or_default(); warn!("CONNECT blocked by method policy (client={client}, host={host}, mode=limited)"); @@ -425,27 +427,27 @@ async fn http_plain_proxy( .await); } - let request = NetworkPolicyRequest::new( - NetworkProtocol::Http, - host.clone(), + let request = NetworkPolicyRequest::new(NetworkPolicyRequestArgs { + protocol: NetworkProtocol::Http, + host: host.clone(), port, - client.clone(), - Some(req.method().as_str().to_string()), - None, - None, - ); + client_addr: client.clone(), + method: Some(req.method().as_str().to_string()), + command: None, + exec_policy_hint: None, + }); match evaluate_host_policy(&app_state, policy_decider.as_ref(), &request).await { Ok(NetworkDecision::Deny { reason }) => { let _ = app_state - .record_blocked(BlockedRequest::new( - host.clone(), - reason.clone(), - client.clone(), - Some(req.method().as_str().to_string()), - None, - "http".to_string(), - )) + .record_blocked(BlockedRequest::new(BlockedRequestArgs { + host: host.clone(), + reason: reason.clone(), + client: client.clone(), + method: Some(req.method().as_str().to_string()), + mode: None, + protocol: "http".to_string(), + })) .await; let client = client.as_deref().unwrap_or_default(); warn!("request blocked (client={client}, host={host}, reason={reason})"); @@ -460,14 +462,14 @@ async fn http_plain_proxy( if !method_allowed { let _ = app_state - .record_blocked(BlockedRequest::new( - host.clone(), - REASON_METHOD_NOT_ALLOWED.to_string(), - client.clone(), - Some(req.method().as_str().to_string()), - Some(NetworkMode::Limited), - "http".to_string(), - )) + .record_blocked(BlockedRequest::new(BlockedRequestArgs { + host: host.clone(), + reason: REASON_METHOD_NOT_ALLOWED.to_string(), + client: client.clone(), + method: Some(req.method().as_str().to_string()), + mode: Some(NetworkMode::Limited), + protocol: "http".to_string(), + })) .await; let client = client.as_deref().unwrap_or_default(); let method = req.method(); @@ -565,14 +567,14 @@ async fn proxy_disabled_response( protocol: &str, ) -> Response { let _ = app_state - .record_blocked(BlockedRequest::new( + .record_blocked(BlockedRequest::new(BlockedRequestArgs { host, - REASON_PROXY_DISABLED.to_string(), + reason: REASON_PROXY_DISABLED.to_string(), client, method, - None, - protocol.to_string(), - )) + mode: None, + protocol: protocol.to_string(), + })) .await; text_response(StatusCode::SERVICE_UNAVAILABLE, "proxy disabled") } diff --git a/codex-rs/network-proxy/src/lib.rs b/codex-rs/network-proxy/src/lib.rs index f4bcb8c74b..e636273128 100644 --- a/codex-rs/network-proxy/src/lib.rs +++ b/codex-rs/network-proxy/src/lib.rs @@ -17,6 +17,7 @@ use anyhow::Result; pub use network_policy::NetworkDecision; pub use network_policy::NetworkPolicyDecider; pub use network_policy::NetworkPolicyRequest; +pub use network_policy::NetworkPolicyRequestArgs; pub use network_policy::NetworkProtocol; pub use proxy::Args; pub use proxy::NetworkProxy; diff --git a/codex-rs/network-proxy/src/network_policy.rs b/codex-rs/network-proxy/src/network_policy.rs index 6f410b3e4b..f14202510c 100644 --- a/codex-rs/network-proxy/src/network_policy.rs +++ b/codex-rs/network-proxy/src/network_policy.rs @@ -26,16 +26,27 @@ pub struct NetworkPolicyRequest { pub exec_policy_hint: Option, } +pub struct NetworkPolicyRequestArgs { + pub protocol: NetworkProtocol, + pub host: String, + pub port: u16, + pub client_addr: Option, + pub method: Option, + pub command: Option, + pub exec_policy_hint: Option, +} + impl NetworkPolicyRequest { - pub fn new( - protocol: NetworkProtocol, - host: String, - port: u16, - client_addr: Option, - method: Option, - command: Option, - exec_policy_hint: Option, - ) -> Self { + pub fn new(args: NetworkPolicyRequestArgs) -> Self { + let NetworkPolicyRequestArgs { + protocol, + host, + port, + client_addr, + method, + command, + exec_policy_hint, + } = args; Self { protocol, host, @@ -139,15 +150,15 @@ mod tests { } }); - let request = NetworkPolicyRequest::new( - NetworkProtocol::Http, - "example.com".to_string(), - 80, - None, - Some("GET".to_string()), - None, - None, - ); + let request = NetworkPolicyRequest::new(NetworkPolicyRequestArgs { + protocol: NetworkProtocol::Http, + host: "example.com".to_string(), + port: 80, + client_addr: None, + method: Some("GET".to_string()), + command: None, + exec_policy_hint: None, + }); let decision = evaluate_host_policy(&state, Some(&decider), &request) .await @@ -172,15 +183,15 @@ mod tests { } }); - let request = NetworkPolicyRequest::new( - NetworkProtocol::Http, - "blocked.com".to_string(), - 80, - None, - Some("GET".to_string()), - None, - None, - ); + let request = NetworkPolicyRequest::new(NetworkPolicyRequestArgs { + protocol: NetworkProtocol::Http, + host: "blocked.com".to_string(), + port: 80, + client_addr: None, + method: Some("GET".to_string()), + command: None, + exec_policy_hint: None, + }); let decision = evaluate_host_policy(&state, Some(&decider), &request) .await @@ -210,15 +221,15 @@ mod tests { } }); - let request = NetworkPolicyRequest::new( - NetworkProtocol::Http, - "127.0.0.1".to_string(), - 80, - None, - Some("GET".to_string()), - None, - None, - ); + let request = NetworkPolicyRequest::new(NetworkPolicyRequestArgs { + protocol: NetworkProtocol::Http, + host: "127.0.0.1".to_string(), + port: 80, + client_addr: None, + method: Some("GET".to_string()), + command: None, + exec_policy_hint: None, + }); let decision = evaluate_host_policy(&state, Some(&decider), &request) .await diff --git a/codex-rs/network-proxy/src/runtime.rs b/codex-rs/network-proxy/src/runtime.rs index 49e34d5c0a..1c29672cfa 100644 --- a/codex-rs/network-proxy/src/runtime.rs +++ b/codex-rs/network-proxy/src/runtime.rs @@ -73,15 +73,25 @@ pub struct BlockedRequest { pub timestamp: i64, } +pub struct BlockedRequestArgs { + pub host: String, + pub reason: String, + pub client: Option, + pub method: Option, + pub mode: Option, + pub protocol: String, +} + impl BlockedRequest { - pub fn new( - host: String, - reason: String, - client: Option, - method: Option, - mode: Option, - protocol: String, - ) -> Self { + pub fn new(args: BlockedRequestArgs) -> Self { + let BlockedRequestArgs { + host, + reason, + client, + method, + mode, + protocol, + } = args; Self { host, reason, diff --git a/codex-rs/network-proxy/src/socks5.rs b/codex-rs/network-proxy/src/socks5.rs index 9426502544..44be9060be 100644 --- a/codex-rs/network-proxy/src/socks5.rs +++ b/codex-rs/network-proxy/src/socks5.rs @@ -2,12 +2,14 @@ use crate::config::NetworkMode; use crate::network_policy::NetworkDecision; use crate::network_policy::NetworkPolicyDecider; use crate::network_policy::NetworkPolicyRequest; +use crate::network_policy::NetworkPolicyRequestArgs; use crate::network_policy::NetworkProtocol; use crate::network_policy::evaluate_host_policy; use crate::policy::normalize_host; use crate::reasons::REASON_METHOD_NOT_ALLOWED; use crate::reasons::REASON_PROXY_DISABLED; use crate::state::BlockedRequest; +use crate::state::BlockedRequestArgs; use crate::state::NetworkProxyState; use anyhow::Context as _; use anyhow::Result; @@ -122,14 +124,14 @@ async fn handle_socks5_tcp( Ok(true) => {} Ok(false) => { let _ = app_state - .record_blocked(BlockedRequest::new( - host.clone(), - REASON_PROXY_DISABLED.to_string(), - client.clone(), - None, - None, - "socks5".to_string(), - )) + .record_blocked(BlockedRequest::new(BlockedRequestArgs { + host: host.clone(), + reason: REASON_PROXY_DISABLED.to_string(), + client: client.clone(), + method: None, + mode: None, + protocol: "socks5".to_string(), + })) .await; let client = client.as_deref().unwrap_or_default(); warn!("SOCKS blocked; proxy disabled (client={client}, host={host})"); @@ -144,14 +146,14 @@ async fn handle_socks5_tcp( match app_state.network_mode().await { Ok(NetworkMode::Limited) => { let _ = app_state - .record_blocked(BlockedRequest::new( - host.clone(), - REASON_METHOD_NOT_ALLOWED.to_string(), - client.clone(), - None, - Some(NetworkMode::Limited), - "socks5".to_string(), - )) + .record_blocked(BlockedRequest::new(BlockedRequestArgs { + host: host.clone(), + reason: REASON_METHOD_NOT_ALLOWED.to_string(), + client: client.clone(), + method: None, + mode: Some(NetworkMode::Limited), + protocol: "socks5".to_string(), + })) .await; let client = client.as_deref().unwrap_or_default(); warn!( @@ -166,27 +168,27 @@ async fn handle_socks5_tcp( } } - let request = NetworkPolicyRequest::new( - NetworkProtocol::Socks5Tcp, - host.clone(), + let request = NetworkPolicyRequest::new(NetworkPolicyRequestArgs { + protocol: NetworkProtocol::Socks5Tcp, + host: host.clone(), port, - client.clone(), - None, - None, - None, - ); + client_addr: client.clone(), + method: None, + command: None, + exec_policy_hint: None, + }); match evaluate_host_policy(&app_state, policy_decider.as_ref(), &request).await { Ok(NetworkDecision::Deny { reason }) => { let _ = app_state - .record_blocked(BlockedRequest::new( - host.clone(), - reason.clone(), - client.clone(), - None, - None, - "socks5".to_string(), - )) + .record_blocked(BlockedRequest::new(BlockedRequestArgs { + host: host.clone(), + reason: reason.clone(), + client: client.clone(), + method: None, + mode: None, + protocol: "socks5".to_string(), + })) .await; let client = client.as_deref().unwrap_or_default(); warn!("SOCKS blocked (client={client}, host={host}, reason={reason})"); @@ -231,14 +233,14 @@ async fn inspect_socks5_udp( Ok(true) => {} Ok(false) => { let _ = state - .record_blocked(BlockedRequest::new( - host.clone(), - REASON_PROXY_DISABLED.to_string(), - client.clone(), - None, - None, - "socks5-udp".to_string(), - )) + .record_blocked(BlockedRequest::new(BlockedRequestArgs { + host: host.clone(), + reason: REASON_PROXY_DISABLED.to_string(), + client: client.clone(), + method: None, + mode: None, + protocol: "socks5-udp".to_string(), + })) .await; let client = client.as_deref().unwrap_or_default(); warn!("SOCKS UDP blocked; proxy disabled (client={client}, host={host})"); @@ -256,14 +258,14 @@ async fn inspect_socks5_udp( match state.network_mode().await { Ok(NetworkMode::Limited) => { let _ = state - .record_blocked(BlockedRequest::new( - host.clone(), - REASON_METHOD_NOT_ALLOWED.to_string(), - client.clone(), - None, - Some(NetworkMode::Limited), - "socks5-udp".to_string(), - )) + .record_blocked(BlockedRequest::new(BlockedRequestArgs { + host: host.clone(), + reason: REASON_METHOD_NOT_ALLOWED.to_string(), + client: client.clone(), + method: None, + mode: Some(NetworkMode::Limited), + protocol: "socks5-udp".to_string(), + })) .await; return Ok(RelayResponse { maybe_payload: None, @@ -277,27 +279,27 @@ async fn inspect_socks5_udp( } } - let request = NetworkPolicyRequest::new( - NetworkProtocol::Socks5Udp, - host.clone(), + let request = NetworkPolicyRequest::new(NetworkPolicyRequestArgs { + protocol: NetworkProtocol::Socks5Udp, + host: host.clone(), port, - client.clone(), - None, - None, - None, - ); + client_addr: client.clone(), + method: None, + command: None, + exec_policy_hint: None, + }); match evaluate_host_policy(&state, policy_decider.as_ref(), &request).await { Ok(NetworkDecision::Deny { reason }) => { let _ = state - .record_blocked(BlockedRequest::new( - host.clone(), - reason.clone(), - client.clone(), - None, - None, - "socks5-udp".to_string(), - )) + .record_blocked(BlockedRequest::new(BlockedRequestArgs { + host: host.clone(), + reason: reason.clone(), + client: client.clone(), + method: None, + mode: None, + protocol: "socks5-udp".to_string(), + })) .await; let client = client.as_deref().unwrap_or_default(); warn!("SOCKS UDP blocked (client={client}, host={host}, reason={reason})"); diff --git a/codex-rs/network-proxy/src/state.rs b/codex-rs/network-proxy/src/state.rs index e1e8a1e4e6..552455f510 100644 --- a/codex-rs/network-proxy/src/state.rs +++ b/codex-rs/network-proxy/src/state.rs @@ -20,6 +20,7 @@ use serde::Deserialize; use std::collections::HashSet; pub use crate::runtime::BlockedRequest; +pub use crate::runtime::BlockedRequestArgs; pub use crate::runtime::NetworkProxyState; #[cfg(test)] pub(crate) use crate::runtime::network_proxy_state_for_policy;