From 45bccd36b038a28b23663189cc6f7557e49e06d0 Mon Sep 17 00:00:00 2001 From: easong-openai Date: Mon, 15 Sep 2025 17:34:04 -0700 Subject: [PATCH 01/21] fix permissions alignment --- codex-rs/tui/src/bottom_pane/chat_composer.rs | 11 +- codex-rs/tui/src/bottom_pane/command_popup.rs | 33 +- .../src/bottom_pane/list_selection_view.rs | 34 ++- .../src/bottom_pane/selection_popup_common.rs | 282 +++++++++++++----- ..._chat_composer__tests__slash_popup_mo.snap | 2 +- ..._list_selection_spacing_with_subtitle.snap | 2 +- ...st_selection_spacing_without_subtitle.snap | 2 +- 7 files changed, 280 insertions(+), 86 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 2138d5409895..bf11ff571dca 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -142,14 +142,16 @@ impl ChatComposer { .desired_height(width.saturating_sub(LIVE_PREFIX_COLS)) + match &self.active_popup { ActivePopup::None => FOOTER_HEIGHT_WITH_HINT, - ActivePopup::Command(c) => c.calculate_required_height(), + ActivePopup::Command(c) => c.calculate_required_height(width), ActivePopup::File(c) => c.calculate_required_height(), } } pub fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> { let popup_constraint = match &self.active_popup { - ActivePopup::Command(popup) => Constraint::Max(popup.calculate_required_height()), + ActivePopup::Command(popup) => { + Constraint::Max(popup.calculate_required_height(area.width)) + } ActivePopup::File(popup) => Constraint::Max(popup.calculate_required_height()), ActivePopup::None => Constraint::Max(FOOTER_HEIGHT_WITH_HINT), }; @@ -1232,7 +1234,10 @@ impl ChatComposer { impl WidgetRef for ChatComposer { fn render_ref(&self, area: Rect, buf: &mut Buffer) { let (popup_constraint, hint_spacing) = match &self.active_popup { - ActivePopup::Command(popup) => (Constraint::Max(popup.calculate_required_height()), 0), + ActivePopup::Command(popup) => ( + Constraint::Max(popup.calculate_required_height(area.width)), + 0, + ), ActivePopup::File(popup) => (Constraint::Max(popup.calculate_required_height()), 0), ActivePopup::None => ( Constraint::Length(FOOTER_HEIGHT_WITH_HINT), diff --git a/codex-rs/tui/src/bottom_pane/command_popup.rs b/codex-rs/tui/src/bottom_pane/command_popup.rs index a5961e31886f..8de266f81904 100644 --- a/codex-rs/tui/src/bottom_pane/command_popup.rs +++ b/codex-rs/tui/src/bottom_pane/command_popup.rs @@ -92,10 +92,35 @@ impl CommandPopup { .ensure_visible(matches_len, MAX_POPUP_ROWS.min(matches_len)); } - /// Determine the preferred height of the popup. This is the number of - /// rows required to show at most MAX_POPUP_ROWS commands. - pub(crate) fn calculate_required_height(&self) -> u16 { - self.filtered_items().len().clamp(1, MAX_POPUP_ROWS) as u16 + /// Determine the preferred height of the popup for a given width. + /// Accounts for wrapped descriptions so that long tooltips don't overflow. + pub(crate) fn calculate_required_height(&self, width: u16) -> u16 { + use super::selection_popup_common::GenericDisplayRow; + use super::selection_popup_common::measure_rows_height; + let matches = self.filtered(); + let rows_all: Vec = if matches.is_empty() { + Vec::new() + } else { + matches + .into_iter() + .map(|(item, indices, _)| match item { + CommandItem::Builtin(cmd) => GenericDisplayRow { + name: format!("/{}", cmd.command()), + match_indices: indices.map(|v| v.into_iter().map(|i| i + 1).collect()), + is_current: false, + description: Some(cmd.description().to_string()), + }, + CommandItem::UserPrompt(i) => GenericDisplayRow { + name: format!("/{}", self.prompts[i].name), + match_indices: indices.map(|v| v.into_iter().map(|i| i + 1).collect()), + is_current: false, + description: Some("send saved prompt".to_string()), + }, + }) + .collect() + }; + + measure_rows_height(&rows_all, &self.state, MAX_POPUP_ROWS, width) } /// Compute fuzzy-filtered matches over built-in commands and user prompts, diff --git a/codex-rs/tui/src/bottom_pane/list_selection_view.rs b/codex-rs/tui/src/bottom_pane/list_selection_view.rs index 7be91db202be..5d5dbf0f330d 100644 --- a/codex-rs/tui/src/bottom_pane/list_selection_view.rs +++ b/codex-rs/tui/src/bottom_pane/list_selection_view.rs @@ -17,6 +17,7 @@ use super::bottom_pane_view::BottomPaneView; use super::popup_consts::MAX_POPUP_ROWS; use super::scroll_state::ScrollState; use super::selection_popup_common::GenericDisplayRow; +use super::selection_popup_common::measure_rows_height; use super::selection_popup_common::render_rows; /// One selectable item in the generic selection list. @@ -135,11 +136,36 @@ impl BottomPaneView for ListSelectionView { CancellationEvent::Handled } - fn desired_height(&self, _width: u16) -> u16 { - let rows = (self.items.len()).clamp(1, MAX_POPUP_ROWS); + fn desired_height(&self, width: u16) -> u16 { + // Measure wrapped height for up to MAX_POPUP_ROWS items at the given width. + // Build the same display rows used by the renderer so wrapping math matches. + let rows: Vec = self + .items + .iter() + .enumerate() + .map(|(i, it)| { + let is_selected = self.state.selected_idx == Some(i); + let prefix = if is_selected { '>' } else { ' ' }; + let name_with_marker = if it.is_current { + format!("{} (current)", it.name) + } else { + it.name.clone() + }; + let display_name = format!("{} {}. {}", prefix, i + 1, name_with_marker); + GenericDisplayRow { + name: display_name, + match_indices: None, + is_current: it.is_current, + description: it.description.clone(), + } + }) + .collect(); + + let rows_height = measure_rows_height(&rows, &self.state, MAX_POPUP_ROWS, width); + // +1 for the title row, +1 for a spacer line beneath the header, - // +1 for optional subtitle, +1 for optional footer - let mut height = rows as u16 + 2; + // +1 for optional subtitle, +1 for optional footer (2 lines incl. spacing) + let mut height = rows_height + 2; if self.subtitle.is_some() { // +1 for subtitle (the spacer is accounted for above) height = height.saturating_add(1); diff --git a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs index 3852d10bc11b..61a26f93418f 100644 --- a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs +++ b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs @@ -1,6 +1,7 @@ use ratatui::buffer::Buffer; use ratatui::layout::Rect; -use ratatui::prelude::Constraint; +// Note: Table-based layout previously used Constraint; the manual renderer +// below no longer requires it. use ratatui::style::Color; use ratatui::style::Modifier; use ratatui::style::Style; @@ -10,9 +11,7 @@ use ratatui::text::Span; use ratatui::widgets::Block; use ratatui::widgets::BorderType; use ratatui::widgets::Borders; -use ratatui::widgets::Cell; -use ratatui::widgets::Row; -use ratatui::widgets::Table; +use ratatui::widgets::Paragraph; use ratatui::widgets::Widget; use super::scroll_state::ScrollState; @@ -27,6 +26,61 @@ pub(crate) struct GenericDisplayRow { impl GenericDisplayRow {} +/// Compute a shared description-column start based on the widest visible name +/// plus two spaces of padding. Ensures at least one column is left for the +/// description. +fn compute_desc_col( + rows_all: &[GenericDisplayRow], + start_idx: usize, + visible_items: usize, + content_width: u16, +) -> usize { + let visible_range = start_idx..(start_idx + visible_items); + let max_name_width = rows_all + .iter() + .enumerate() + .filter(|(i, _)| visible_range.contains(i)) + .map(|(_, r)| Line::from(r.name.clone()).width()) + .max() + .unwrap_or(0); + let mut desc_col = max_name_width.saturating_add(2); + if (desc_col as u16) >= content_width { + desc_col = content_width.saturating_sub(1) as usize; + } + desc_col +} + +/// Build the full display line for a row with the description padded to start +/// at `desc_col`. Applies fuzzy-match bolding when indices are present and +/// dims the description. +fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> { + let mut name_spans: Vec = Vec::with_capacity(row.name.len()); + if let Some(idxs) = row.match_indices.as_ref() { + let mut idx_iter = idxs.iter().peekable(); + for (char_idx, ch) in row.name.chars().enumerate() { + if idx_iter.peek().is_some_and(|next| **next == char_idx) { + idx_iter.next(); + name_spans.push(ch.to_string().bold()); + } else { + name_spans.push(ch.to_string().into()); + } + } + } else { + name_spans.push(row.name.clone().into()); + } + + let this_name_width = Line::from(name_spans.clone()).width(); + let mut full_spans: Vec = name_spans; + if let Some(desc) = row.description.as_ref() { + let gap = desc_col.saturating_sub(this_name_width); + if gap > 0 { + full_spans.push(" ".repeat(gap).into()); + } + full_spans.push(desc.clone().dim()); + } + Line::from(full_spans) +} + /// Render a list of rows using the provided ScrollState, with shared styling /// and behavior for selection popups. pub(crate) fn render_rows( @@ -38,84 +92,168 @@ pub(crate) fn render_rows( _dim_non_selected: bool, empty_message: &str, ) { - let mut rows: Vec = Vec::new(); + // Always draw a dim left border to match other popups. + let block = Block::default() + .borders(Borders::LEFT) + .border_type(BorderType::QuadrantOutside) + .border_style(Style::default().add_modifier(Modifier::DIM)); + block.render(area, buf); + + // Content renders to the right of the border. + let content_area = Rect { + x: area.x.saturating_add(1), + y: area.y, + width: area.width.saturating_sub(1), + height: area.height, + }; + if rows_all.is_empty() { - rows.push(Row::new(vec![Cell::from(Line::from( - empty_message.dim().italic(), - ))])); - } else { - let max_rows_from_area = area.height as usize; - let visible_rows = max_results - .min(rows_all.len()) - .min(max_rows_from_area.max(1)); - - // Compute starting index based on scroll state and selection. - let mut start_idx = state.scroll_top.min(rows_all.len().saturating_sub(1)); - if let Some(sel) = state.selected_idx { - if sel < start_idx { - start_idx = sel; - } else if visible_rows > 0 { - let bottom = start_idx + visible_rows - 1; - if sel > bottom { - start_idx = sel + 1 - visible_rows; - } - } + if content_area.height > 0 { + let para = Paragraph::new(Line::from(empty_message.dim().italic())); + para.render( + Rect { + x: content_area.x, + y: content_area.y, + width: content_area.width, + height: 1, + }, + buf, + ); } + return; + } - for (i, row) in rows_all - .iter() - .enumerate() - .skip(start_idx) - .take(visible_rows) - { - let GenericDisplayRow { - name, - match_indices, - is_current: _is_current, - description, - } = row; - - // Highlight fuzzy indices when present. - let mut spans: Vec = Vec::with_capacity(name.len()); - if let Some(idxs) = match_indices.as_ref() { - let mut idx_iter = idxs.iter().peekable(); - for (char_idx, ch) in name.chars().enumerate() { - if idx_iter.peek().is_some_and(|next| **next == char_idx) { - idx_iter.next(); - spans.push(ch.to_string().bold()); - } else { - spans.push(ch.to_string().into()); - } - } - } else { - spans.push(name.clone().into()); - } + // Determine which logical rows (items) are visible given the selection and + // the max_results clamp. Scrolling is still item-based for simplicity. + let max_rows_from_area = content_area.height as usize; + let visible_items = max_results + .min(rows_all.len()) + .min(max_rows_from_area.max(1)); - if let Some(desc) = description.as_ref() { - spans.push(" ".into()); - spans.push(desc.clone().dim()); + let mut start_idx = state.scroll_top.min(rows_all.len().saturating_sub(1)); + if let Some(sel) = state.selected_idx { + if sel < start_idx { + start_idx = sel; + } else if visible_items > 0 { + let bottom = start_idx + visible_items - 1; + if sel > bottom { + start_idx = sel + 1 - visible_items; } + } + } + + let desc_col = compute_desc_col(rows_all, start_idx, visible_items, content_area.width); + + // Render items, wrapping descriptions and aligning wrapped lines under the + // shared description column. Stop when we run out of vertical space. + let mut cur_y = content_area.y; + for (i, row) in rows_all + .iter() + .enumerate() + .skip(start_idx) + .take(visible_items) + { + if cur_y >= content_area.y + content_area.height { + break; + } + + let GenericDisplayRow { + name, + match_indices, + is_current: _is_current, + description, + } = row; + + let full_line = build_full_line( + &GenericDisplayRow { + name: name.clone(), + match_indices: match_indices.clone(), + is_current: *_is_current, + description: description.clone(), + }, + desc_col, + ); - let mut cell = Cell::from(Line::from(spans)); + // Wrap with subsequent indent aligned to the description column. + use crate::wrapping::RtOptions; + use crate::wrapping::word_wrap_line; + let options = RtOptions::new(content_area.width as usize) + .initial_indent(Line::from("")) + .subsequent_indent(Line::from(" ".repeat(desc_col))); + let wrapped = word_wrap_line(&full_line, options); + + // Render the wrapped lines. + for mut line in wrapped { + if cur_y >= content_area.y + content_area.height { + break; + } if Some(i) == state.selected_idx { - cell = cell.style( - Style::default() - .fg(Color::Cyan) - .add_modifier(Modifier::BOLD), - ); + // Match previous behavior: cyan + bold for the selected row. + line.style = Style::default() + .fg(Color::Cyan) + .add_modifier(Modifier::BOLD); } - rows.push(Row::new(vec![cell])); + let para = Paragraph::new(line); + para.render( + Rect { + x: content_area.x, + y: cur_y, + width: content_area.width, + height: 1, + }, + buf, + ); + cur_y = cur_y.saturating_add(1); } } +} - let table = Table::new(rows, vec![Constraint::Percentage(100)]) - .block( - Block::default() - .borders(Borders::LEFT) - .border_type(BorderType::QuadrantOutside) - .border_style(Style::default().add_modifier(Modifier::DIM)), - ) - .widths([Constraint::Percentage(100)]); +/// Compute the number of terminal rows required to render up to `max_results` +/// items from `rows_all` given the current scroll/selection state and the +/// available `width`. Accounts for description wrapping and alignment so the +/// caller can allocate sufficient vertical space. +pub(crate) fn measure_rows_height( + rows_all: &[GenericDisplayRow], + state: &ScrollState, + max_results: usize, + width: u16, +) -> u16 { + if rows_all.is_empty() { + return 1; // placeholder "no matches" line + } + + let content_width = width.saturating_sub(1).max(1); + + let visible_items = max_results.min(rows_all.len()); + let mut start_idx = state.scroll_top.min(rows_all.len().saturating_sub(1)); + if let Some(sel) = state.selected_idx { + if sel < start_idx { + start_idx = sel; + } else if visible_items > 0 { + let bottom = start_idx + visible_items - 1; + if sel > bottom { + start_idx = sel + 1 - visible_items; + } + } + } - table.render(area, buf); + let desc_col = compute_desc_col(rows_all, start_idx, visible_items, content_width); + + use crate::wrapping::RtOptions; + use crate::wrapping::word_wrap_line; + let mut total: u16 = 0; + for row in rows_all + .iter() + .enumerate() + .skip(start_idx) + .take(visible_items) + .map(|(_, r)| r) + { + let full_line = build_full_line(row, desc_col); + let opts = RtOptions::new(content_width as usize) + .initial_indent(Line::from("")) + .subsequent_indent(Line::from(" ".repeat(desc_col))); + total = total.saturating_add(word_wrap_line(&full_line, opts).len() as u16); + } + total.max(1) } diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__slash_popup_mo.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__slash_popup_mo.snap index b626f80ca177..f908fb6144cb 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__slash_popup_mo.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__slash_popup_mo.snap @@ -4,5 +4,5 @@ expression: terminal.backend() --- "▌ /mo " "▌ " -"▌/model choose what model and reasoning effort to use " +"▌/model choose what model and reasoning effort to use " "▌/mention mention a file " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_spacing_with_subtitle.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_spacing_with_subtitle.snap index aab7e7c9b697..65606ed7d06c 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_spacing_with_subtitle.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_spacing_with_subtitle.snap @@ -6,6 +6,6 @@ expression: render_lines(&view) ▌ Switch between Codex approval presets ▌ ▌> 1. Read Only (current) Codex can read files -▌ 2. Full Access Codex can edit files +▌ 2. Full Access Codex can edit files Press Enter to confirm or Esc to go back diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_spacing_without_subtitle.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_spacing_without_subtitle.snap index 77552f2b93c7..b42a5f8c6b03 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_spacing_without_subtitle.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_spacing_without_subtitle.snap @@ -5,6 +5,6 @@ expression: render_lines(&view) ▌ Select Approval Mode ▌ ▌> 1. Read Only (current) Codex can read files -▌ 2. Full Access Codex can edit files +▌ 2. Full Access Codex can edit files Press Enter to confirm or Esc to go back From a8026d3846486e790827b26e9abf13e7f96e48bf Mon Sep 17 00:00:00 2001 From: Dylan Date: Mon, 15 Sep 2025 19:01:10 -0700 Subject: [PATCH 02/21] fix: read-only escalations (#3673) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Splitting out this smaller fix from #2694 - fixes the sandbox permissions so Chat / read-only mode tool definition matches expectations ## Testing - [x] Tested locally Screenshot 2025-09-15 at 2 51 19 PM --- codex-rs/core/src/openai_tools.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/core/src/openai_tools.rs b/codex-rs/core/src/openai_tools.rs index a511d6dd1639..f4d724815e22 100644 --- a/codex-rs/core/src/openai_tools.rs +++ b/codex-rs/core/src/openai_tools.rs @@ -273,7 +273,7 @@ fn create_shell_tool_for_sandbox(sandbox_policy: &SandboxPolicy) -> OpenAiTool { }, ); - if matches!(sandbox_policy, SandboxPolicy::WorkspaceWrite { .. }) { + if !matches!(sandbox_policy, SandboxPolicy::DangerFullAccess) { properties.insert( "with_escalated_permissions".to_string(), JsonSchema::Boolean { From 5e2c4f7e357e3b1a63d8a4fc79be245975f5af8a Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Tue, 16 Sep 2025 08:43:29 -0700 Subject: [PATCH 03/21] Update azure model provider example (#3680) Make the section linkable. --- docs/config.md | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/docs/config.md b/docs/config.md index 01b3efe0a170..4f287b824e96 100644 --- a/docs/config.md +++ b/docs/config.md @@ -69,17 +69,6 @@ base_url = "https://api.mistral.ai/v1" env_key = "MISTRAL_API_KEY" ``` -Note that Azure requires `api-version` to be passed as a query parameter, so be sure to specify it as part of `query_params` when defining the Azure provider: - -```toml -[model_providers.azure] -name = "Azure" -# Make sure you set the appropriate subdomain for this URL. -base_url = "https://YOUR_PROJECT_NAME.openai.azure.com/openai" -env_key = "AZURE_OPENAI_API_KEY" # Or "OPENAI_API_KEY", whichever you use. -query_params = { api-version = "2025-04-01-preview" } -``` - It is also possible to configure a provider to include extra HTTP headers with a request. These can be hardcoded values (`http_headers`) or values read from environment variables (`env_http_headers`): ```toml @@ -96,6 +85,22 @@ http_headers = { "X-Example-Header" = "example-value" } env_http_headers = { "X-Example-Features" = "EXAMPLE_FEATURES" } ``` +### Azure model provider example + +Note that Azure requires `api-version` to be passed as a query parameter, so be sure to specify it as part of `query_params` when defining the Azure provider: + +```toml +[model_providers.azure] +name = "Azure" +# Make sure you set the appropriate subdomain for this URL. +base_url = "https://YOUR_PROJECT_NAME.openai.azure.com/openai" +env_key = "AZURE_OPENAI_API_KEY" # Or "OPENAI_API_KEY", whichever you use. +query_params = { api-version = "2025-04-01-preview" } +wire_api = "responses" +``` + +Export your key before launching Codex: `export AZURE_OPENAI_API_KEY=…` + ### Per-provider network tuning The following optional settings control retry behaviour and streaming idle timeouts **per model provider**. They must be specified inside the corresponding `[model_providers.]` block in `config.toml`. (Older releases accepted top‑level keys; those are now ignored.) From 116c497948a7cee9a6d130dc5426a2271b7984ea Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 16 Sep 2025 16:42:58 +0000 Subject: [PATCH 04/21] chore(merge): enforce policy removals (images + cargo caches) --- .github/auto/MERGE_PLAN.md | 36 ------------------------------------ .github/auto/MERGE_REPORT.md | 27 --------------------------- 2 files changed, 63 deletions(-) delete mode 100644 .github/auto/MERGE_PLAN.md delete mode 100644 .github/auto/MERGE_REPORT.md diff --git a/.github/auto/MERGE_PLAN.md b/.github/auto/MERGE_PLAN.md deleted file mode 100644 index 6154ee225196..000000000000 --- a/.github/auto/MERGE_PLAN.md +++ /dev/null @@ -1,36 +0,0 @@ -Upstream merge plan - -- Mode: by-bucket (per task input MERGE_MODE) -- Upstream: openai/codex@main -> branch: upstream-merge (no fast-forward) - -Strategy - -- Buckets - - Prefer ours: codex-rs/tui/**, codex-cli/**, codex-rs/core/src/{openai_tools.rs,codex.rs,agent_tool.rs,default_client.rs}, codex-rs/protocol/src/models.rs, .github/workflows/**, docs/**, AGENTS.md, README.md, CHANGELOG.md - - Prefer theirs: codex-rs/common/**, codex-rs/exec/**, codex-rs/file-search/** - - Purge: .github/codex-cli-*.png|jpg|jpeg|webp (keep removed if reintroduced) - -- Default rule - - Outside prefer_ours_globs, adopt upstream unless it breaks our build or documented behavior. - - In protected areas, keep fork behavior unless an upstream change is clearly compatible and beneficial. - -- Invariants to preserve - - Tool families: keep browser_*, agent_* and web_fetch tool schema registration in openai_tools and parity with handlers. - - Gating: retain browser tool exposure gating logic. - - Screenshot UX: keep pending-queue semantics and TUI consumer behavior. - - Version/UA: continue using codex_version::version() and get_codex_user_agent_default() where applicable. - - Public API: keep codex-core re-exports (ModelClient, Prompt, ResponseEvent, ResponseStream) and models alias. - -Execution steps - -1) Ensure upstream remote and fetch both remotes -2) Merge upstream/main into upstream-merge with --no-commit -3) Reconcile diffs using bucket rules and purge list -4) Run scripts/upstream-merge/verify.sh (includes build-fast.sh and guards) -5) Commit with a clear merge message and short status -6) Write MERGE_REPORT.md and push upstream-merge - -Notes - -- No unrelated refactors. Avoid re-introducing previously removed images and branding assets. -- If histories diverge with no merge-base, re-run merge with --allow-unrelated-histories. diff --git a/.github/auto/MERGE_REPORT.md b/.github/auto/MERGE_REPORT.md deleted file mode 100644 index 844de70ff026..000000000000 --- a/.github/auto/MERGE_REPORT.md +++ /dev/null @@ -1,27 +0,0 @@ -# Upstream Merge Report - -- Source: openai/codex@main (5e2c4f7e3) -- Target branch: upstream-merge -- Mode: by-bucket - -## Incorporated -- docs/config.md: Adopt upstream update to Azure model provider example (clarifies configuration). No conflicts. - -## Dropped / Purged -- No new files matched purge globs. Confirmed no reintroduction of `.github/codex-cli-*` images. - -## Preserved (fork invariants) -- Tool families and gating: browser_*, agent_*, and web_fetch schemas present and parity with handlers verified by guards. -- Screenshot UX semantics unchanged. -- Version/UA: `codex_version::version()` usage intact in default_client. -- Public API re-exports preserved (ModelClient, Prompt, ResponseEvent, ResponseStream) and models alias. - -## Verify Summary -- scripts/upstream-merge/verify.sh: PASSED - - build-fast.sh: ok - - codex-core api_surface tests compile: ok - - static guards (tools + UA/version): ok - -## Notes -- No TUI/CLI user-visible branding changes included in this bucket. -- No conflicts encountered; merge committed as a simple upstream sync. From 244687303b14a89b9bc998adabce7a7c9e1cc396 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Tue, 16 Sep 2025 14:02:15 -0400 Subject: [PATCH 05/21] Persist search items (#3745) Let's record the search items because they are part of the history. --- codex-rs/core/src/conversation_history.rs | 5 +++-- codex-rs/core/src/rollout/policy.rs | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/codex-rs/core/src/conversation_history.rs b/codex-rs/core/src/conversation_history.rs index 891a2ea4726b..7c23e4dc297c 100644 --- a/codex-rs/core/src/conversation_history.rs +++ b/codex-rs/core/src/conversation_history.rs @@ -47,8 +47,9 @@ fn is_api_message(message: &ResponseItem) -> bool { | ResponseItem::CustomToolCall { .. } | ResponseItem::CustomToolCallOutput { .. } | ResponseItem::LocalShellCall { .. } - | ResponseItem::Reasoning { .. } => true, - ResponseItem::WebSearchCall { .. } | ResponseItem::Other => false, + | ResponseItem::Reasoning { .. } + | ResponseItem::WebSearchCall { .. } => true, + ResponseItem::Other => false, } } diff --git a/codex-rs/core/src/rollout/policy.rs b/codex-rs/core/src/rollout/policy.rs index 0d8fee92b03d..2fd0efb0dc10 100644 --- a/codex-rs/core/src/rollout/policy.rs +++ b/codex-rs/core/src/rollout/policy.rs @@ -25,8 +25,9 @@ pub(crate) fn should_persist_response_item(item: &ResponseItem) -> bool { | ResponseItem::FunctionCall { .. } | ResponseItem::FunctionCallOutput { .. } | ResponseItem::CustomToolCall { .. } - | ResponseItem::CustomToolCallOutput { .. } => true, - ResponseItem::WebSearchCall { .. } | ResponseItem::Other => false, + | ResponseItem::CustomToolCallOutput { .. } + | ResponseItem::WebSearchCall { .. } => true, + ResponseItem::Other => false, } } From 11285655c4dd02937b48f0bdb97c7ec5078b0714 Mon Sep 17 00:00:00 2001 From: Dylan Date: Tue, 16 Sep 2025 11:32:20 -0700 Subject: [PATCH 06/21] fix: Record EnvironmentContext in SendUserTurn (#3678) ## Summary SendUserTurn has not been correctly handling updates to policies. While the tui protocol handles this in `Op::OverrideTurnContext`, the SendUserTurn should be appending `EnvironmentContext` messages when the sandbox settings change. MCP client behavior should match the cli behavior, so we update `SendUserTurn` message to match. ## Testing - [x] Added prompt caching tests --- codex-rs/core/src/codex.rs | 15 +- codex-rs/core/src/environment_context.rs | 115 ++++++++ codex-rs/core/src/shell.rs | 12 +- codex-rs/core/tests/suite/prompt_caching.rs | 277 +++++++++++++++++++- 4 files changed, 410 insertions(+), 9 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 375183ab82df..b170076d2260 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1348,10 +1348,21 @@ async fn submission_loop( cwd, is_review_mode: false, }; - // TODO: record the new environment context in the conversation history + + // if the environment context has changed, record it in the conversation history + let previous_env_context = EnvironmentContext::from(turn_context.as_ref()); + let new_env_context = EnvironmentContext::from(&fresh_turn_context); + if !new_env_context.equals_except_shell(&previous_env_context) { + sess.record_conversation_items(&[ResponseItem::from(new_env_context)]) + .await; + } + + // Install the new persistent context for subsequent tasks/turns. + turn_context = Arc::new(fresh_turn_context); + // no current task, spawn a new one with the per‑turn context let task = - AgentTask::spawn(sess.clone(), Arc::new(fresh_turn_context), sub.id, items); + AgentTask::spawn(sess.clone(), Arc::clone(&turn_context), sub.id, items); sess.set_task(task); } } diff --git a/codex-rs/core/src/environment_context.rs b/codex-rs/core/src/environment_context.rs index 89af9e1c917b..8f3292a22651 100644 --- a/codex-rs/core/src/environment_context.rs +++ b/codex-rs/core/src/environment_context.rs @@ -2,6 +2,7 @@ use serde::Deserialize; use serde::Serialize; use strum_macros::Display as DeriveDisplay; +use crate::codex::TurnContext; use crate::protocol::AskForApproval; use crate::protocol::SandboxPolicy; use crate::shell::Shell; @@ -71,6 +72,39 @@ impl EnvironmentContext { shell, } } + + /// Compares two environment contexts, ignoring the shell. Useful when + /// comparing turn to turn, since the initial environment_context will + /// include the shell, and then it is not configurable from turn to turn. + pub fn equals_except_shell(&self, other: &EnvironmentContext) -> bool { + let EnvironmentContext { + cwd, + approval_policy, + sandbox_mode, + network_access, + writable_roots, + // should compare all fields except shell + shell: _, + } = other; + + self.cwd == *cwd + && self.approval_policy == *approval_policy + && self.sandbox_mode == *sandbox_mode + && self.network_access == *network_access + && self.writable_roots == *writable_roots + } +} + +impl From<&TurnContext> for EnvironmentContext { + fn from(turn_context: &TurnContext) -> Self { + Self::new( + Some(turn_context.cwd.clone()), + Some(turn_context.approval_policy), + Some(turn_context.sandbox_policy.clone()), + // Shell is not configurable from turn to turn + None, + ) + } } impl EnvironmentContext { @@ -140,6 +174,9 @@ impl From for ResponseItem { #[cfg(test)] mod tests { + use crate::shell::BashShell; + use crate::shell::ZshShell; + use super::*; use pretty_assertions::assert_eq; @@ -210,4 +247,82 @@ mod tests { assert_eq!(context.serialize_to_xml(), expected); } + + #[test] + fn equals_except_shell_compares_approval_policy() { + // Approval policy + let context1 = EnvironmentContext::new( + Some(PathBuf::from("/repo")), + Some(AskForApproval::OnRequest), + Some(workspace_write_policy(vec!["/repo"], false)), + None, + ); + let context2 = EnvironmentContext::new( + Some(PathBuf::from("/repo")), + Some(AskForApproval::Never), + Some(workspace_write_policy(vec!["/repo"], true)), + None, + ); + assert!(!context1.equals_except_shell(&context2)); + } + + #[test] + fn equals_except_shell_compares_sandbox_policy() { + let context1 = EnvironmentContext::new( + Some(PathBuf::from("/repo")), + Some(AskForApproval::OnRequest), + Some(SandboxPolicy::new_read_only_policy()), + None, + ); + let context2 = EnvironmentContext::new( + Some(PathBuf::from("/repo")), + Some(AskForApproval::OnRequest), + Some(SandboxPolicy::new_workspace_write_policy()), + None, + ); + + assert!(!context1.equals_except_shell(&context2)); + } + + #[test] + fn equals_except_shell_compares_workspace_write_policy() { + let context1 = EnvironmentContext::new( + Some(PathBuf::from("/repo")), + Some(AskForApproval::OnRequest), + Some(workspace_write_policy(vec!["/repo", "/tmp", "/var"], false)), + None, + ); + let context2 = EnvironmentContext::new( + Some(PathBuf::from("/repo")), + Some(AskForApproval::OnRequest), + Some(workspace_write_policy(vec!["/repo", "/tmp"], true)), + None, + ); + + assert!(!context1.equals_except_shell(&context2)); + } + + #[test] + fn equals_except_shell_ignores_shell() { + let context1 = EnvironmentContext::new( + Some(PathBuf::from("/repo")), + Some(AskForApproval::OnRequest), + Some(workspace_write_policy(vec!["/repo"], false)), + Some(Shell::Bash(BashShell { + shell_path: "/bin/bash".into(), + bashrc_path: "/home/user/.bashrc".into(), + })), + ); + let context2 = EnvironmentContext::new( + Some(PathBuf::from("/repo")), + Some(AskForApproval::OnRequest), + Some(workspace_write_policy(vec!["/repo"], false)), + Some(Shell::Zsh(ZshShell { + shell_path: "/bin/zsh".into(), + zshrc_path: "/home/user/.zshrc".into(), + })), + ); + + assert!(context1.equals_except_shell(&context2)); + } } diff --git a/codex-rs/core/src/shell.rs b/codex-rs/core/src/shell.rs index 1734d5de65cb..cb278fdd468c 100644 --- a/codex-rs/core/src/shell.rs +++ b/codex-rs/core/src/shell.rs @@ -5,20 +5,20 @@ use std::path::PathBuf; #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] pub struct ZshShell { - shell_path: String, - zshrc_path: String, + pub(crate) shell_path: String, + pub(crate) zshrc_path: String, } #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] pub struct BashShell { - shell_path: String, - bashrc_path: String, + pub(crate) shell_path: String, + pub(crate) bashrc_path: String, } #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] pub struct PowerShellConfig { - exe: String, // Executable name or path, e.g. "pwsh" or "powershell.exe". - bash_exe_fallback: Option, // In case the model generates a bash command. + pub(crate) exe: String, // Executable name or path, e.g. "pwsh" or "powershell.exe". + pub(crate) bash_exe_fallback: Option, // In case the model generates a bash command. } #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index c6731fee342d..a69f57a2014f 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -12,6 +12,7 @@ use codex_core::protocol::Op; use codex_core::protocol::SandboxPolicy; use codex_core::protocol_config_types::ReasoningEffort; use codex_core::protocol_config_types::ReasoningSummary; +use codex_core::shell::Shell; use codex_core::shell::default_user_shell; use core_test_support::load_default_config_for_test; use core_test_support::load_sse_fixture_with_id; @@ -23,6 +24,30 @@ use wiremock::ResponseTemplate; use wiremock::matchers::method; use wiremock::matchers::path; +fn text_user_input(text: String) -> serde_json::Value { + serde_json::json!({ + "type": "message", + "role": "user", + "content": [ { "type": "input_text", "text": text } ] + }) +} + +fn default_env_context_str(cwd: &str, shell: &Shell) -> String { + format!( + r#" + {} + on-request + read-only + restricted +{}"#, + cwd, + match shell.name() { + Some(name) => format!(" {name}\n"), + None => String::new(), + } + ) +} + /// Build minimal SSE stream with completed marker using the JSON fixture. fn sse_completed(id: &str) -> String { load_sse_fixture_with_id("tests/fixtures/completed_template.json", id) @@ -546,12 +571,262 @@ async fn per_turn_overrides_keep_cached_prefix_and_key_constant() { "role": "user", "content": [ { "type": "input_text", "text": "hello 2" } ] }); + let expected_env_text_2 = format!( + r#" + {} + never + workspace-write + enabled + + {} + +"#, + new_cwd.path().to_string_lossy(), + writable.path().to_string_lossy(), + ); + let expected_env_msg_2 = serde_json::json!({ + "type": "message", + "role": "user", + "content": [ { "type": "input_text", "text": expected_env_text_2 } ] + }); let expected_body2 = serde_json::json!( [ body1["input"].as_array().unwrap().as_slice(), - [expected_user_message_2].as_slice(), + [expected_env_msg_2, expected_user_message_2].as_slice(), ] .concat() ); assert_eq!(body2["input"], expected_body2); } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn send_user_turn_with_no_changes_does_not_send_environment_context() { + use pretty_assertions::assert_eq; + + let server = MockServer::start().await; + + let sse = sse_completed("resp"); + let template = ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .set_body_raw(sse, "text/event-stream"); + + Mock::given(method("POST")) + .and(path("/v1/responses")) + .respond_with(template) + .expect(2) + .mount(&server) + .await; + + let model_provider = ModelProviderInfo { + base_url: Some(format!("{}/v1", server.uri())), + ..built_in_model_providers()["openai"].clone() + }; + + let cwd = TempDir::new().unwrap(); + let codex_home = TempDir::new().unwrap(); + let mut config = load_default_config_for_test(&codex_home); + config.cwd = cwd.path().to_path_buf(); + config.model_provider = model_provider; + config.user_instructions = Some("be consistent and helpful".to_string()); + + let default_cwd = config.cwd.clone(); + let default_approval_policy = config.approval_policy; + let default_sandbox_policy = config.sandbox_policy.clone(); + let default_model = config.model.clone(); + let default_effort = config.model_reasoning_effort; + let default_summary = config.model_reasoning_summary; + + let conversation_manager = + ConversationManager::with_auth(CodexAuth::from_api_key("Test API Key")); + let codex = conversation_manager + .new_conversation(config) + .await + .expect("create new conversation") + .conversation; + + codex + .submit(Op::UserTurn { + items: vec![InputItem::Text { + text: "hello 1".into(), + }], + cwd: default_cwd.clone(), + approval_policy: default_approval_policy, + sandbox_policy: default_sandbox_policy.clone(), + model: default_model.clone(), + effort: default_effort, + summary: default_summary, + }) + .await + .unwrap(); + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + + codex + .submit(Op::UserTurn { + items: vec![InputItem::Text { + text: "hello 2".into(), + }], + cwd: default_cwd.clone(), + approval_policy: default_approval_policy, + sandbox_policy: default_sandbox_policy.clone(), + model: default_model.clone(), + effort: default_effort, + summary: default_summary, + }) + .await + .unwrap(); + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + + let requests = server.received_requests().await.unwrap(); + assert_eq!(requests.len(), 2, "expected two POST requests"); + + let body1 = requests[0].body_json::().unwrap(); + let body2 = requests[1].body_json::().unwrap(); + + let shell = default_user_shell().await; + let expected_ui_text = + "\n\nbe consistent and helpful\n\n"; + let expected_ui_msg = text_user_input(expected_ui_text.to_string()); + + let expected_env_msg_1 = text_user_input(default_env_context_str( + &cwd.path().to_string_lossy(), + &shell, + )); + let expected_user_message_1 = text_user_input("hello 1".to_string()); + + let expected_input_1 = serde_json::Value::Array(vec![ + expected_ui_msg.clone(), + expected_env_msg_1.clone(), + expected_user_message_1.clone(), + ]); + assert_eq!(body1["input"], expected_input_1); + + let expected_user_message_2 = text_user_input("hello 2".to_string()); + let expected_input_2 = serde_json::Value::Array(vec![ + expected_ui_msg, + expected_env_msg_1, + expected_user_message_1, + expected_user_message_2, + ]); + assert_eq!(body2["input"], expected_input_2); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn send_user_turn_with_changes_sends_environment_context() { + use pretty_assertions::assert_eq; + + let server = MockServer::start().await; + + let sse = sse_completed("resp"); + let template = ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .set_body_raw(sse, "text/event-stream"); + + Mock::given(method("POST")) + .and(path("/v1/responses")) + .respond_with(template) + .expect(2) + .mount(&server) + .await; + + let model_provider = ModelProviderInfo { + base_url: Some(format!("{}/v1", server.uri())), + ..built_in_model_providers()["openai"].clone() + }; + + let cwd = TempDir::new().unwrap(); + let codex_home = TempDir::new().unwrap(); + let mut config = load_default_config_for_test(&codex_home); + config.cwd = cwd.path().to_path_buf(); + config.model_provider = model_provider; + config.user_instructions = Some("be consistent and helpful".to_string()); + + let default_cwd = config.cwd.clone(); + let default_approval_policy = config.approval_policy; + let default_sandbox_policy = config.sandbox_policy.clone(); + let default_model = config.model.clone(); + let default_effort = config.model_reasoning_effort; + let default_summary = config.model_reasoning_summary; + + let conversation_manager = + ConversationManager::with_auth(CodexAuth::from_api_key("Test API Key")); + let codex = conversation_manager + .new_conversation(config.clone()) + .await + .expect("create new conversation") + .conversation; + + codex + .submit(Op::UserTurn { + items: vec![InputItem::Text { + text: "hello 1".into(), + }], + cwd: default_cwd.clone(), + approval_policy: default_approval_policy, + sandbox_policy: default_sandbox_policy.clone(), + model: default_model, + effort: default_effort, + summary: default_summary, + }) + .await + .unwrap(); + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + + codex + .submit(Op::UserTurn { + items: vec![InputItem::Text { + text: "hello 2".into(), + }], + cwd: default_cwd.clone(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model: "o3".to_string(), + effort: Some(ReasoningEffort::High), + summary: ReasoningSummary::Detailed, + }) + .await + .unwrap(); + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + + let requests = server.received_requests().await.unwrap(); + assert_eq!(requests.len(), 2, "expected two POST requests"); + + let body1 = requests[0].body_json::().unwrap(); + let body2 = requests[1].body_json::().unwrap(); + + let shell = default_user_shell().await; + let expected_ui_text = + "\n\nbe consistent and helpful\n\n"; + let expected_ui_msg = serde_json::json!({ + "type": "message", + "role": "user", + "content": [ { "type": "input_text", "text": expected_ui_text } ] + }); + let expected_env_text_1 = default_env_context_str(&default_cwd.to_string_lossy(), &shell); + let expected_env_msg_1 = text_user_input(expected_env_text_1); + let expected_user_message_1 = text_user_input("hello 1".to_string()); + let expected_input_1 = serde_json::Value::Array(vec![ + expected_ui_msg.clone(), + expected_env_msg_1.clone(), + expected_user_message_1.clone(), + ]); + assert_eq!(body1["input"], expected_input_1); + + let expected_env_msg_2 = text_user_input(format!( + r#" + {} + never + danger-full-access + enabled +"#, + default_cwd.to_string_lossy() + )); + let expected_user_message_2 = text_user_input("hello 2".to_string()); + let expected_input_2 = serde_json::Value::Array(vec![ + expected_ui_msg, + expected_env_msg_1, + expected_user_message_1, + expected_env_msg_2, + expected_user_message_2, + ]); + assert_eq!(body2["input"], expected_input_2); +} From 7fe4021f9593bd9f271fe2c58f673eb6eb1e74e1 Mon Sep 17 00:00:00 2001 From: dedrisian-oai Date: Tue, 16 Sep 2025 13:36:51 -0700 Subject: [PATCH 07/21] Review mode core updates (#3701) 1. Adds the environment prompt (including cwd) to review thread 2. Prepends the review prompt as a user message (temporary fix so the instructions are not replaced on backend) 3. Sets reasoning to low 4. Sets default review model to `gpt-5-codex` --- codex-rs/core/src/codex.rs | 30 +++++++++++----------- codex-rs/core/src/config.rs | 10 ++++---- codex-rs/core/src/lib.rs | 1 + codex-rs/core/tests/suite/review.rs | 39 ++++++++++++++++++++++------- 4 files changed, 52 insertions(+), 28 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index b170076d2260..59b3b6ae44c9 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1154,20 +1154,16 @@ impl AgentTask { fn abort(self, reason: TurnAbortReason) { // TOCTOU? if !self.handle.is_finished() { - if self.kind == AgentTaskKind::Review { - let sess = self.sess.clone(); - let sub_id = self.sub_id.clone(); - tokio::spawn(async move { - exit_review_mode(sess, sub_id, None).await; - }); - } self.handle.abort(); let event = Event { - id: self.sub_id, + id: self.sub_id.clone(), msg: EventMsg::TurnAborted(TurnAbortedEvent { reason }), }; let sess = self.sess; tokio::spawn(async move { + if self.kind == AgentTaskKind::Review { + exit_review_mode(sess.clone(), self.sub_id, None).await; + } sess.send_event(event).await; }); } @@ -1560,7 +1556,8 @@ async fn spawn_review_thread( experimental_unified_exec_tool: config.use_experimental_unified_exec_tool, }); - let base_instructions = Some(REVIEW_PROMPT.to_string()); + let base_instructions = REVIEW_PROMPT.to_string(); + let review_prompt = review_request.prompt.clone(); let provider = parent_turn_context.client.get_provider(); let auth_manager = parent_turn_context.client.get_auth_manager(); let model_family = review_model_family.clone(); @@ -1569,16 +1566,19 @@ async fn spawn_review_thread( let mut per_turn_config = (*config).clone(); per_turn_config.model = model.clone(); per_turn_config.model_family = model_family.clone(); + per_turn_config.model_reasoning_effort = Some(ReasoningEffortConfig::Low); + per_turn_config.model_reasoning_summary = ReasoningSummaryConfig::Detailed; if let Some(model_info) = get_model_info(&model_family) { per_turn_config.model_context_window = Some(model_info.context_window); } + let per_turn_config = Arc::new(per_turn_config); let client = ModelClient::new( - Arc::new(per_turn_config), + per_turn_config.clone(), auth_manager, provider, - parent_turn_context.client.get_reasoning_effort(), - parent_turn_context.client.get_reasoning_summary(), + per_turn_config.model_reasoning_effort, + per_turn_config.model_reasoning_summary, sess.conversation_id, ); @@ -1586,7 +1586,7 @@ async fn spawn_review_thread( client, tools_config, user_instructions: None, - base_instructions, + base_instructions: Some(base_instructions.clone()), approval_policy: parent_turn_context.approval_policy, sandbox_policy: parent_turn_context.sandbox_policy.clone(), shell_environment_policy: parent_turn_context.shell_environment_policy.clone(), @@ -1596,7 +1596,7 @@ async fn spawn_review_thread( // Seed the child task with the review prompt as the initial user message. let input: Vec = vec![InputItem::Text { - text: review_request.prompt.clone(), + text: format!("{base_instructions}\n\n---\n\nNow, here's your task: {review_prompt}"), }]; let tc = Arc::new(review_turn_context); @@ -1654,6 +1654,8 @@ async fn run_task( let is_review_mode = turn_context.is_review_mode; let mut review_thread_history: Vec = Vec::new(); if is_review_mode { + // Seed review threads with environment context so the model knows the working directory. + review_thread_history.extend(sess.build_initial_context(turn_context.as_ref())); review_thread_history.push(initial_input_for_turn.into()); } else { sess.record_input_and_rollout_usermsg(&initial_input_for_turn) diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 0479e857c227..6a613abbbad2 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -38,7 +38,7 @@ use toml_edit::Item as TomlItem; use toml_edit::Table as TomlTable; const OPENAI_DEFAULT_MODEL: &str = "gpt-5"; -const OPENAI_DEFAULT_REVIEW_MODEL: &str = "gpt-5"; +const OPENAI_DEFAULT_REVIEW_MODEL: &str = "gpt-5-codex"; pub const GPT_5_CODEX_MEDIUM_MODEL: &str = "gpt-5-codex"; /// Maximum number of bytes of the documentation that will be embedded. Larger @@ -1581,7 +1581,7 @@ model_verbosity = "high" assert_eq!( Config { model: "o3".to_string(), - review_model: "gpt-5".to_string(), + review_model: OPENAI_DEFAULT_REVIEW_MODEL.to_string(), model_family: find_family_for_model("o3").expect("known model slug"), model_context_window: Some(200_000), model_max_output_tokens: Some(100_000), @@ -1639,7 +1639,7 @@ model_verbosity = "high" )?; let expected_gpt3_profile_config = Config { model: "gpt-3.5-turbo".to_string(), - review_model: "gpt-5".to_string(), + review_model: OPENAI_DEFAULT_REVIEW_MODEL.to_string(), model_family: find_family_for_model("gpt-3.5-turbo").expect("known model slug"), model_context_window: Some(16_385), model_max_output_tokens: Some(4_096), @@ -1712,7 +1712,7 @@ model_verbosity = "high" )?; let expected_zdr_profile_config = Config { model: "o3".to_string(), - review_model: "gpt-5".to_string(), + review_model: OPENAI_DEFAULT_REVIEW_MODEL.to_string(), model_family: find_family_for_model("o3").expect("known model slug"), model_context_window: Some(200_000), model_max_output_tokens: Some(100_000), @@ -1771,7 +1771,7 @@ model_verbosity = "high" )?; let expected_gpt5_profile_config = Config { model: "gpt-5".to_string(), - review_model: "gpt-5".to_string(), + review_model: OPENAI_DEFAULT_REVIEW_MODEL.to_string(), model_family: find_family_for_model("gpt-5").expect("known model slug"), model_context_window: Some(272_000), model_max_output_tokens: Some(128_000), diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 7b9c3dc9f0e9..aa2e176b07e4 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -88,6 +88,7 @@ pub use codex_protocol::config_types as protocol_config_types; pub use client::ModelClient; pub use client_common::Prompt; +pub use client_common::REVIEW_PROMPT; pub use client_common::ResponseEvent; pub use client_common::ResponseStream; pub use codex_protocol::models::ContentItem; diff --git a/codex-rs/core/tests/suite/review.rs b/codex-rs/core/tests/suite/review.rs index f65913a2ca1b..1ea56d5ae38a 100644 --- a/codex-rs/core/tests/suite/review.rs +++ b/codex-rs/core/tests/suite/review.rs @@ -2,8 +2,10 @@ use codex_core::CodexAuth; use codex_core::CodexConversation; use codex_core::ConversationManager; use codex_core::ModelProviderInfo; +use codex_core::REVIEW_PROMPT; use codex_core::built_in_model_providers; use codex_core::config::Config; +use codex_core::protocol::ENVIRONMENT_CONTEXT_OPEN_TAG; use codex_core::protocol::EventMsg; use codex_core::protocol::ExitedReviewModeEvent; use codex_core::protocol::InputItem; @@ -419,17 +421,36 @@ async fn review_input_isolated_from_parent_history() { .await; let _complete = wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; - // Assert the request `input` contains only the single review user message. + // Assert the request `input` contains the environment context followed by the review prompt. let request = &server.received_requests().await.unwrap()[0]; let body = request.body_json::().unwrap(); - let expected_input = serde_json::json!([ - { - "type": "message", - "role": "user", - "content": [{"type": "input_text", "text": review_prompt}] - } - ]); - assert_eq!(body["input"], expected_input); + let input = body["input"].as_array().expect("input array"); + assert_eq!( + input.len(), + 2, + "expected environment context and review prompt" + ); + + let env_msg = &input[0]; + assert_eq!(env_msg["type"].as_str().unwrap(), "message"); + assert_eq!(env_msg["role"].as_str().unwrap(), "user"); + let env_text = env_msg["content"][0]["text"].as_str().expect("env text"); + assert!( + env_text.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG), + "environment context must be the first item" + ); + assert!( + env_text.contains(""), + "environment context should include cwd" + ); + + let review_msg = &input[1]; + assert_eq!(review_msg["type"].as_str().unwrap(), "message"); + assert_eq!(review_msg["role"].as_str().unwrap(), "user"); + assert_eq!( + review_msg["content"][0]["text"].as_str().unwrap(), + format!("{REVIEW_PROMPT}\n\n---\n\nNow, here's your task: Please review only this",) + ); server.verify().await; } From b8d2b1a5764c31678238286eb4c9b80c9ff3f366 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Tue, 16 Sep 2025 16:42:43 -0700 Subject: [PATCH 08/21] restyle thinking outputs (#3755) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Screenshot 2025-09-16 at 2 23 18 PM --- codex-rs/tui/src/chatwidget.rs | 7 +- codex-rs/tui/src/history_cell.rs | 116 +++++++++++++++++++--------- codex-rs/tui/src/markdown_stream.rs | 4 +- codex-rs/tui/src/wrapping.rs | 2 - 4 files changed, 85 insertions(+), 44 deletions(-) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index c03e7d3bc51c..fb91d75456b5 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -226,12 +226,11 @@ impl ChatWidget { // At the end of a reasoning block, record transcript-only content. self.full_reasoning_buffer.push_str(&self.reasoning_buffer); if !self.full_reasoning_buffer.is_empty() { - for cell in history_cell::new_reasoning_summary_block( + let cell = history_cell::new_reasoning_summary_block( self.full_reasoning_buffer.clone(), &self.config, - ) { - self.add_boxed_history(cell); - } + ); + self.add_boxed_history(cell); } self.reasoning_buffer.clear(); self.full_reasoning_buffer.clear(); diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index d6add6d328e7..9cdd29dd0273 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -121,6 +121,45 @@ impl HistoryCell for UserHistoryCell { } } +#[derive(Debug)] +pub(crate) struct ReasoningSummaryCell { + _header: Vec>, + content: Vec>, +} + +impl ReasoningSummaryCell { + pub(crate) fn new(header: Vec>, content: Vec>) -> Self { + Self { + _header: header, + content, + } + } +} + +impl HistoryCell for ReasoningSummaryCell { + fn display_lines(&self, width: u16) -> Vec> { + let summary_lines = self + .content + .iter() + .map(|l| l.clone().dim().italic()) + .collect::>(); + + word_wrap_lines( + &summary_lines, + RtOptions::new(width as usize) + .initial_indent("• ".into()) + .subsequent_indent(" ".into()), + ) + } + + fn transcript_lines(&self) -> Vec> { + let mut out: Vec> = Vec::new(); + out.push("thinking".magenta().bold().into()); + out.extend(self.content.clone()); + out + } +} + #[derive(Debug)] pub(crate) struct AgentMessageCell { lines: Vec>, @@ -1417,7 +1456,7 @@ pub(crate) fn new_reasoning_block( pub(crate) fn new_reasoning_summary_block( full_reasoning_buffer: String, config: &Config, -) -> Vec> { +) -> Box { if config.model_family.reasoning_summary_format == ReasoningSummaryFormat::Experimental { // Experimental format is following: // ** header ** @@ -1434,27 +1473,19 @@ pub(crate) fn new_reasoning_summary_block( // then we don't have a summary to inject into history if after_close_idx < full_reasoning_buffer.len() { let header_buffer = full_reasoning_buffer[..after_close_idx].to_string(); - let summary_buffer = full_reasoning_buffer[after_close_idx..].to_string(); - - let mut header_lines: Vec> = Vec::new(); - header_lines.push(Line::from("Thinking".magenta().italic())); + let mut header_lines = Vec::new(); append_markdown(&header_buffer, &mut header_lines, config); - let mut summary_lines: Vec> = Vec::new(); - summary_lines.push(Line::from("Thinking".magenta().bold())); + let summary_buffer = full_reasoning_buffer[after_close_idx..].to_string(); + let mut summary_lines = Vec::new(); append_markdown(&summary_buffer, &mut summary_lines, config); - return vec![ - Box::new(TranscriptOnlyHistoryCell { - lines: header_lines, - }), - Box::new(AgentMessageCell::new(summary_lines, true)), - ]; + return Box::new(ReasoningSummaryCell::new(header_lines, summary_lines)); } } } } - vec![Box::new(new_reasoning_block(full_reasoning_buffer, config))] + Box::new(new_reasoning_block(full_reasoning_buffer, config)) } struct OutputLinesParams { @@ -1558,6 +1589,7 @@ mod tests { use codex_core::config::ConfigOverrides; use codex_core::config::ConfigToml; use dirs::home_dir; + use pretty_assertions::assert_eq; fn test_config() -> Config { Config::load_from_base_config_with_overrides( @@ -2076,17 +2108,35 @@ mod tests { let rendered = render_lines(&lines).join("\n"); insta::assert_snapshot!(rendered); } + #[test] + fn reasoning_summary_block() { + let mut config = test_config(); + config.model_family.reasoning_summary_format = ReasoningSummaryFormat::Experimental; + + let cell = new_reasoning_summary_block( + "**High level reasoning**\n\nDetailed reasoning goes here.".to_string(), + &config, + ); + + let rendered_display = render_lines(&cell.display_lines(80)); + assert_eq!(rendered_display, vec!["• Detailed reasoning goes here."]); + + let rendered_transcript = render_transcript(cell.as_ref()); + assert_eq!( + rendered_transcript, + vec!["thinking", "Detailed reasoning goes here."] + ); + } #[test] fn reasoning_summary_block_returns_reasoning_cell_when_feature_disabled() { let mut config = test_config(); config.model_family.reasoning_summary_format = ReasoningSummaryFormat::Experimental; - let cells = + let cell = new_reasoning_summary_block("Detailed reasoning goes here.".to_string(), &config); - assert_eq!(cells.len(), 1); - let rendered = render_transcript(cells[0].as_ref()); + let rendered = render_transcript(cell.as_ref()); assert_eq!(rendered, vec!["thinking", "Detailed reasoning goes here."]); } @@ -2095,13 +2145,12 @@ mod tests { let mut config = test_config(); config.model_family.reasoning_summary_format = ReasoningSummaryFormat::Experimental; - let cells = new_reasoning_summary_block( + let cell = new_reasoning_summary_block( "**High level reasoning without closing".to_string(), &config, ); - assert_eq!(cells.len(), 1); - let rendered = render_transcript(cells[0].as_ref()); + let rendered = render_transcript(cell.as_ref()); assert_eq!( rendered, vec!["thinking", "**High level reasoning without closing"] @@ -2113,25 +2162,23 @@ mod tests { let mut config = test_config(); config.model_family.reasoning_summary_format = ReasoningSummaryFormat::Experimental; - let cells = new_reasoning_summary_block( + let cell = new_reasoning_summary_block( "**High level reasoning without closing**".to_string(), &config, ); - assert_eq!(cells.len(), 1); - let rendered = render_transcript(cells[0].as_ref()); + let rendered = render_transcript(cell.as_ref()); assert_eq!( rendered, vec!["thinking", "High level reasoning without closing"] ); - let cells = new_reasoning_summary_block( + let cell = new_reasoning_summary_block( "**High level reasoning without closing**\n\n ".to_string(), &config, ); - assert_eq!(cells.len(), 1); - let rendered = render_transcript(cells[0].as_ref()); + let rendered = render_transcript(cell.as_ref()); assert_eq!( rendered, vec!["thinking", "High level reasoning without closing"] @@ -2143,21 +2190,18 @@ mod tests { let mut config = test_config(); config.model_family.reasoning_summary_format = ReasoningSummaryFormat::Experimental; - let cells = new_reasoning_summary_block( + let cell = new_reasoning_summary_block( "**High level plan**\n\nWe should fix the bug next.".to_string(), &config, ); - assert_eq!(cells.len(), 2); - - let header_lines = render_transcript(cells[0].as_ref()); - assert_eq!(header_lines, vec!["Thinking", "High level plan"]); - - let summary_lines = render_transcript(cells[1].as_ref()); + let rendered_display = render_lines(&cell.display_lines(80)); + assert_eq!(rendered_display, vec!["• We should fix the bug next."]); + let rendered_transcript = render_transcript(cell.as_ref()); assert_eq!( - summary_lines, - vec!["codex", "Thinking", "We should fix the bug next."] - ) + rendered_transcript, + vec!["thinking", "We should fix the bug next."] + ); } } diff --git a/codex-rs/tui/src/markdown_stream.rs b/codex-rs/tui/src/markdown_stream.rs index 7245afb102a2..12b43082102b 100644 --- a/codex-rs/tui/src/markdown_stream.rs +++ b/codex-rs/tui/src/markdown_stream.rs @@ -333,11 +333,11 @@ mod tests { ); for (i, l) in non_blank.iter().enumerate() { assert_eq!( - l.style.fg, + l.spans[0].style.fg, Some(Color::Green), "wrapped line {} should preserve green style, got {:?}", i, - l.style.fg + l.spans[0].style.fg ); } } diff --git a/codex-rs/tui/src/wrapping.rs b/codex-rs/tui/src/wrapping.rs index bb309784b45f..c97f0e3c6c69 100644 --- a/codex-rs/tui/src/wrapping.rs +++ b/codex-rs/tui/src/wrapping.rs @@ -187,7 +187,6 @@ where // Build first wrapped line with initial indent. let mut first_line = rt_opts.initial_indent.clone(); - first_line.style = first_line.style.patch(line.style); { let sliced = slice_line_spans(line, &span_bounds, first_line_range); let mut spans = first_line.spans; @@ -216,7 +215,6 @@ where continue; } let mut subsequent_line = rt_opts.subsequent_indent.clone(); - subsequent_line.style = subsequent_line.style.patch(line.style); let offset_range = (r.start + base)..(r.end + base); let sliced = slice_line_spans(line, &span_bounds, &offset_range); let mut spans = subsequent_line.spans; From 72733e34c4caca9c38e894ccb6621834587b9cf7 Mon Sep 17 00:00:00 2001 From: dedrisian-oai Date: Tue, 16 Sep 2025 18:43:32 -0700 Subject: [PATCH 09/21] Add dev message upon review out (#3758) Proposal: We want to record a dev message like so: ``` { "type": "message", "role": "user", "content": [ { "type": "input_text", "text": " User initiated a review task. Here's the full review output from reviewer model. User may select one or more comments to resolve. review {findings_str} " } ] }, ``` Without showing in the chat transcript. Rough idea, but it fixes issue where the user finishes a review thread, and asks the parent "fix the rest of the review issues" thinking that the parent knows about it. ### Question: Why not a tool call? Because the agent didn't make the call, it was a human. + we haven't implemented sub-agents yet, and we'll need to think about the way we represent these human-led tool calls for the agent. --- codex-rs/core/src/codex.rs | 47 ++++++++++++++++- codex-rs/core/src/lib.rs | 1 + codex-rs/core/src/review_format.rs | 55 +++++++++++++++++++ codex-rs/core/tests/suite/review.rs | 82 +++++++++++++++++++++++++++++ 4 files changed, 183 insertions(+), 2 deletions(-) create mode 100644 codex-rs/core/src/review_format.rs diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 59b3b6ae44c9..b3b75ec76a7d 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -11,6 +11,7 @@ use std::time::Duration; use crate::AuthManager; use crate::client_common::REVIEW_PROMPT; use crate::event_mapping::map_response_item_to_event_messages; +use crate::review_format::format_review_findings_block; use async_channel::Receiver; use async_channel::Sender; use codex_apply_patch::ApplyPatchAction; @@ -3259,7 +3260,8 @@ fn convert_call_tool_result_to_function_call_output_payload( } } -/// Emits an ExitedReviewMode Event with optional ReviewOutput. +/// Emits an ExitedReviewMode Event with optional ReviewOutput, +/// and records a developer message with the review output. async fn exit_review_mode( session: Arc, task_sub_id: String, @@ -3267,9 +3269,50 @@ async fn exit_review_mode( ) { let event = Event { id: task_sub_id, - msg: EventMsg::ExitedReviewMode(ExitedReviewModeEvent { review_output }), + msg: EventMsg::ExitedReviewMode(ExitedReviewModeEvent { + review_output: review_output.clone(), + }), }; session.send_event(event).await; + + let mut user_message = String::new(); + if let Some(out) = review_output { + let mut findings_str = String::new(); + let text = out.overall_explanation.trim(); + if !text.is_empty() { + findings_str.push_str(text); + } + if !out.findings.is_empty() { + let block = format_review_findings_block(&out.findings, None); + findings_str.push_str(&format!("\n{block}")); + } + user_message.push_str(&format!( + r#" + User initiated a review task. Here's the full review output from reviewer model. User may select one or more comments to resolve. + review + + {findings_str} + + +"#)); + } else { + user_message.push_str(r#" + User initiated a review task, but was interrupted. If user asks about this, tell them to re-initiate a review with `/review` and wait for it to complete. + review + + None. + + +"#); + } + + session + .record_conversation_items(&[ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { text: user_message }], + }]) + .await; } #[cfg(test)] diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index aa2e176b07e4..e024effbe214 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -46,6 +46,7 @@ pub use model_provider_info::built_in_model_providers; pub use model_provider_info::create_oss_provider_with_base_url; mod conversation_manager; mod event_mapping; +pub mod review_format; pub use codex_protocol::protocol::InitialHistory; pub use conversation_manager::ConversationManager; pub use conversation_manager::NewConversation; diff --git a/codex-rs/core/src/review_format.rs b/codex-rs/core/src/review_format.rs new file mode 100644 index 000000000000..272010d56431 --- /dev/null +++ b/codex-rs/core/src/review_format.rs @@ -0,0 +1,55 @@ +use crate::protocol::ReviewFinding; + +// Note: We keep this module UI-agnostic. It returns plain strings that +// higher layers (e.g., TUI) may style as needed. + +fn format_location(item: &ReviewFinding) -> String { + let path = item.code_location.absolute_file_path.display(); + let start = item.code_location.line_range.start; + let end = item.code_location.line_range.end; + format!("{path}:{start}-{end}") +} + +/// Format a full review findings block as plain text lines. +/// +/// - When `selection` is `Some`, each item line includes a checkbox marker: +/// "[x]" for selected items and "[ ]" for unselected. Missing indices +/// default to selected. +/// - When `selection` is `None`, the marker is omitted and a simple bullet is +/// rendered ("- Title — path:start-end"). +pub fn format_review_findings_block( + findings: &[ReviewFinding], + selection: Option<&[bool]>, +) -> String { + let mut lines: Vec = Vec::new(); + + // Header + let header = if findings.len() > 1 { + "Full review comments:" + } else { + "Review comment:" + }; + lines.push(header.to_string()); + + for (idx, item) in findings.iter().enumerate() { + lines.push(String::new()); + + let title = &item.title; + let location = format_location(item); + + if let Some(flags) = selection { + // Default to selected if index is out of bounds. + let checked = flags.get(idx).copied().unwrap_or(true); + let marker = if checked { "[x]" } else { "[ ]" }; + lines.push(format!("- {marker} {title} — {location}")); + } else { + lines.push(format!("- {title} — {location}")); + } + + for body_line in item.body.lines() { + lines.push(format!(" {body_line}")); + } + } + + lines.join("\n") +} diff --git a/codex-rs/core/tests/suite/review.rs b/codex-rs/core/tests/suite/review.rs index 1ea56d5ae38a..a20807e4eceb 100644 --- a/codex-rs/core/tests/suite/review.rs +++ b/codex-rs/core/tests/suite/review.rs @@ -1,10 +1,13 @@ use codex_core::CodexAuth; use codex_core::CodexConversation; +use codex_core::ContentItem; use codex_core::ConversationManager; use codex_core::ModelProviderInfo; use codex_core::REVIEW_PROMPT; +use codex_core::ResponseItem; use codex_core::built_in_model_providers; use codex_core::config::Config; +use codex_core::protocol::ConversationPathResponseEvent; use codex_core::protocol::ENVIRONMENT_CONTEXT_OPEN_TAG; use codex_core::protocol::EventMsg; use codex_core::protocol::ExitedReviewModeEvent; @@ -15,6 +18,8 @@ use codex_core::protocol::ReviewFinding; use codex_core::protocol::ReviewLineRange; use codex_core::protocol::ReviewOutputEvent; use codex_core::protocol::ReviewRequest; +use codex_core::protocol::RolloutItem; +use codex_core::protocol::RolloutLine; use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use core_test_support::load_default_config_for_test; use core_test_support::load_sse_fixture_with_id_from_str; @@ -117,6 +122,46 @@ async fn review_op_emits_lifecycle_and_review_output() { assert_eq!(expected, review); let _complete = wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + // Also verify that a user message with the header and a formatted finding + // was recorded back in the parent session's rollout. + codex.submit(Op::GetPath).await.unwrap(); + let history_event = + wait_for_event(&codex, |ev| matches!(ev, EventMsg::ConversationPath(_))).await; + let path = match history_event { + EventMsg::ConversationPath(ConversationPathResponseEvent { path, .. }) => path, + other => panic!("expected ConversationPath event, got {other:?}"), + }; + let text = std::fs::read_to_string(&path).expect("read rollout file"); + + let mut saw_header = false; + let mut saw_finding_line = false; + for line in text.lines() { + if line.trim().is_empty() { + continue; + } + let v: serde_json::Value = serde_json::from_str(line).expect("jsonl line"); + let rl: RolloutLine = serde_json::from_value(v).expect("rollout line"); + if let RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) = rl.item + && role == "user" + { + for c in content { + if let ContentItem::InputText { text } = c { + if text.contains("full review output from reviewer model") { + saw_header = true; + } + if text.contains("- Prefer Stylize helpers — /tmp/file.rs:10-20") { + saw_finding_line = true; + } + } + } + } + } + assert!(saw_header, "user header missing from rollout"); + assert!( + saw_finding_line, + "formatted finding line missing from rollout" + ); + server.verify().await; } @@ -452,6 +497,43 @@ async fn review_input_isolated_from_parent_history() { format!("{REVIEW_PROMPT}\n\n---\n\nNow, here's your task: Please review only this",) ); + // Also verify that a user interruption note was recorded in the rollout. + codex.submit(Op::GetPath).await.unwrap(); + let history_event = + wait_for_event(&codex, |ev| matches!(ev, EventMsg::ConversationPath(_))).await; + let path = match history_event { + EventMsg::ConversationPath(ConversationPathResponseEvent { path, .. }) => path, + other => panic!("expected ConversationPath event, got {other:?}"), + }; + let text = std::fs::read_to_string(&path).expect("read rollout file"); + let mut saw_interruption_message = false; + for line in text.lines() { + if line.trim().is_empty() { + continue; + } + let v: serde_json::Value = serde_json::from_str(line).expect("jsonl line"); + let rl: RolloutLine = serde_json::from_value(v).expect("rollout line"); + if let RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) = rl.item + && role == "user" + { + for c in content { + if let ContentItem::InputText { text } = c + && text.contains("User initiated a review task, but was interrupted.") + { + saw_interruption_message = true; + break; + } + } + } + if saw_interruption_message { + break; + } + } + assert!( + saw_interruption_message, + "expected user interruption message in rollout" + ); + server.verify().await; } From 694565f0d925e28ef17c3b6e7299c79f3062809c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 17 Sep 2025 03:03:50 +0000 Subject: [PATCH 10/21] chore(merge): enforce policy removals (images + cargo caches) --- .github/auto/MERGE_PLAN.md | 24 ------------------------ .github/auto/MERGE_REPORT.md | 32 -------------------------------- 2 files changed, 56 deletions(-) delete mode 100644 .github/auto/MERGE_PLAN.md delete mode 100644 .github/auto/MERGE_REPORT.md diff --git a/.github/auto/MERGE_PLAN.md b/.github/auto/MERGE_PLAN.md deleted file mode 100644 index 38d17d1efc41..000000000000 --- a/.github/auto/MERGE_PLAN.md +++ /dev/null @@ -1,24 +0,0 @@ -# Upstream Merge Plan (by-bucket) - -Context: -- Upstream: openai/codex@main (remote: upstream) -- Target branch: upstream-merge (pre-created) -- Mode: by-bucket (apply policy globs) - -Strategy: -1) Prefer ours in protected areas: - - codex-rs/tui/**, codex-cli/** - - codex-rs/core/src/{openai_tools.rs,codex.rs,agent_tool.rs,default_client.rs} - - codex-rs/protocol/src/models.rs, workflows, docs, AGENTS.md, README.md, CHANGELOG.md -2) Prefer theirs for shared libs: - - codex-rs/common/**, codex-rs/exec/**, codex-rs/file-search/** (unless it breaks build/policies) -3) Default: adopt upstream outside protected areas; reconcile only where necessary. -4) Purge images per policy purge_globs if reintroduced. -5) Preserve invariants: browser_*/agent_* tool families, web_fetch exposure + gating, screenshot queue semantics, version/UA helpers, core re-exports. -6) Do not reintroduce perma-removed paths; record noteworthy deltas in MERGE_REPORT.md. - -Process: -- Merge upstream/main into upstream-merge with --no-commit. -- Resolve conflicts per globs; review upstream commit range for notable changes using artifacts. -- Run scripts/upstream-merge/verify.sh and ./build-fast.sh; fix warnings/errors minimally. -- Commit with a conventional message and push. diff --git a/.github/auto/MERGE_REPORT.md b/.github/auto/MERGE_REPORT.md deleted file mode 100644 index f7454890fcd9..000000000000 --- a/.github/auto/MERGE_REPORT.md +++ /dev/null @@ -1,32 +0,0 @@ -# Upstream Merge Report - -Branch: upstream-merge -Upstream: openai/codex@main -Mode: by-bucket -Date: $(date -u +%Y-%m-%dT%H:%M:%SZ) - -## Incorporated -- Adopted upstream review formatting module `codex-rs/core/src/review_format.rs` (UI-agnostic strings). -- Integrated upstream review tests under `codex-rs/core/tests/suite/review.rs` with minimal path fixes. -- Accepted upstream changes in `codex-rs/core/src/lib.rs` to export `review_format`. - -## Dropped / Overridden -- Resolved conflict in `codex-rs/core/src/codex.rs` by keeping our fork version per prefer_ours policy to preserve: - - browser_* and agent_* tool families and gating - - screenshot queue semantics - - UA/version helpers and exports - - Response export aliases (ModelClient, Prompt, ResponseEvent, ResponseStream) - -## Adjustments -- Fixed imports to use protocol crate types for review structures: - - `codex-rs/core/src/review_format.rs`: `use codex_protocol::protocol::ReviewFinding;` - - `codex-rs/core/tests/suite/review.rs`: switched Review* imports to `codex_protocol::protocol::*`. -- No purge_globs matches found to remove. - -## Verification -- scripts/upstream-merge/verify.sh: PASS (build_fast=ok, api_check=ok) -- ./build-fast.sh: PASS (no warnings observed) - -## Notes -- Kept our `codex-core` API re-exports and `models` alias intact. -- No ICU/sys-locale dependency changes needed. From 791d7b125f4166ef576531075688aac339350011 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 16 Sep 2025 20:33:59 -0700 Subject: [PATCH 11/21] fix: make GitHub Action publish to npm using trusted publishing (#3431) --- .github/workflows/rust-release.yml | 23 ++++++ codex-cli/package.json | 3 +- docs/release_management.md | 9 +-- scripts/publish_to_npm.py | 118 ----------------------------- 4 files changed, 26 insertions(+), 127 deletions(-) delete mode 100755 scripts/publish_to_npm.py diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index 07af62a17c07..804ac83d965e 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -11,6 +11,9 @@ on: tags: - "rust-v*.*.*" +permissions: + id-token: write # Required for OIDC + concurrency: group: ${{ github.workflow }} cancel-in-progress: true @@ -187,6 +190,20 @@ jobs: version="${GITHUB_REF_NAME#rust-v}" echo "name=${version}" >> $GITHUB_OUTPUT + # Publish to npm using OIDC authentication. + # July 31, 2025: https://github.blog/changelog/2025-07-31-npm-trusted-publishing-with-oidc-is-generally-available/ + # npm docs: https://docs.npmjs.com/trusted-publishers + - name: Setup Node.js + uses: actions/setup-node@v5 + with: + node-version: 22 + registry-url: "https://registry.npmjs.org" + scope: "@openai" + + # Trusted publishing requires npm CLI version 11.5.1 or later. + - name: Update npm + run: npm install -g npm@latest + - name: Stage npm package env: GH_TOKEN: ${{ github.token }} @@ -220,6 +237,12 @@ jobs: tag: ${{ github.ref_name }} config: .github/dotslash-config.json + # No NODE_AUTH_TOKEN needed because we use OIDC. + - name: Publish to npm + # Do not publish alphas to npm. + if: ${{ !contains(steps.release_name.outputs.name, '-') }} + run: npm publish "${GITHUB_WORKSPACE}/dist/npm/codex-npm-${{ steps.release_name.outputs.name }}.tgz" + update-branch: name: Update latest-alpha-cli branch permissions: diff --git a/codex-cli/package.json b/codex-cli/package.json index 614ca1a832e4..02124f32576f 100644 --- a/codex-cli/package.json +++ b/codex-cli/package.json @@ -15,7 +15,8 @@ ], "repository": { "type": "git", - "url": "git+https://github.com/openai/codex.git" + "url": "git+https://github.com/openai/codex.git", + "directory": "codex-cli" }, "dependencies": { "@vscode/ripgrep": "^1.15.14" diff --git a/docs/release_management.md b/docs/release_management.md index ed12de6e4a5c..d2b326455400 100644 --- a/docs/release_management.md +++ b/docs/release_management.md @@ -30,14 +30,7 @@ When the workflow finishes, the GitHub Release is "done," but you still have to ## Publishing to npm -After the GitHub Release is done, you can publish to npm. Note the GitHub Release includes the appropriate artifact for npm (which is the output of `npm pack`), which should be named `codex-npm-VERSION.tgz`. To publish to npm, run: - -``` -VERSION=0.21.0 -./scripts/publish_to_npm.py "$VERSION" -``` - -Note that you must have permissions to publish to https://www.npmjs.com/package/@openai/codex for this to succeed. +The GitHub Action is responsible for publishing to npm. ## Publishing to Homebrew diff --git a/scripts/publish_to_npm.py b/scripts/publish_to_npm.py deleted file mode 100755 index f79843ffc853..000000000000 --- a/scripts/publish_to_npm.py +++ /dev/null @@ -1,118 +0,0 @@ -#!/usr/bin/env python3 - -""" -Download a release artifact for the npm package and publish it. - -Given a release version like `0.20.0`, this script: - - Downloads the `codex-npm-.tgz` asset from the GitHub release - tagged `rust-v` in the `openai/codex` repository using `gh`. - - Runs `npm publish` on the downloaded tarball to publish `@openai/codex`. - -Flags: - - `--dry-run` delegates to `npm publish --dry-run`. The artifact is still - downloaded so npm can inspect the archive contents without publishing. - -Requirements: - - GitHub CLI (`gh`) must be installed and authenticated to access the repo. - - npm must be logged in with an account authorized to publish - `@openai/codex`. This may trigger a browser for 2FA. -""" - -import argparse -import os -import subprocess -import sys -import tempfile -from pathlib import Path - - -def run_checked(cmd: list[str], cwd: Path | None = None) -> None: - """Run a subprocess command and raise if it fails.""" - proc = subprocess.run(cmd, cwd=str(cwd) if cwd else None) - proc.check_returncode() - - -def main() -> int: - parser = argparse.ArgumentParser( - description=( - "Download the npm release artifact for a given version and publish it." - ) - ) - parser.add_argument( - "version", - help="Release version to publish, e.g. 0.20.0 (without the 'v' prefix)", - ) - parser.add_argument( - "--dir", - type=Path, - help=( - "Optional directory to download the artifact into. Defaults to a temporary directory." - ), - ) - parser.add_argument( - "-n", - "--dry-run", - action="store_true", - help="Delegate to `npm publish --dry-run` (still downloads the artifact).", - ) - args = parser.parse_args() - - version: str = args.version.lstrip("v") - tag = f"rust-v{version}" - asset_name = f"codex-npm-{version}.tgz" - - download_dir_context_manager = ( - tempfile.TemporaryDirectory() if args.dir is None else None - ) - # Use provided dir if set, else the temporary one created above - download_dir: Path = args.dir if args.dir else Path(download_dir_context_manager.name) # type: ignore[arg-type] - download_dir.mkdir(parents=True, exist_ok=True) - - # 1) Download the artifact using gh - repo = "openai/codex" - gh_cmd = [ - "gh", - "release", - "download", - tag, - "--repo", - repo, - "--pattern", - asset_name, - "--dir", - str(download_dir), - ] - print(f"Downloading {asset_name} from {repo}@{tag} into {download_dir}...") - # Even in --dry-run we download so npm can inspect the tarball. - run_checked(gh_cmd) - - artifact_path = download_dir / asset_name - if not args.dry_run and not artifact_path.is_file(): - print( - f"Error: expected artifact not found after download: {artifact_path}", - file=sys.stderr, - ) - return 1 - - # 2) Publish to npm - npm_cmd = ["npm", "publish"] - if args.dry_run: - npm_cmd.append("--dry-run") - npm_cmd.append(str(artifact_path)) - - # Ensure CI is unset so npm can open a browser for 2FA if needed. - env = os.environ.copy() - if env.get("CI"): - env.pop("CI") - - print("Running:", " ".join(npm_cmd)) - proc = subprocess.run(npm_cmd, env=env) - proc.check_returncode() - - print("Publish complete.") - # Keep the temporary directory alive until here; it is cleaned up on exit - return 0 - - -if __name__ == "__main__": - sys.exit(main()) From 38dfd6ad696ee2a2c6763e92f3459d435fe842c1 Mon Sep 17 00:00:00 2001 From: just-every-code Date: Wed, 17 Sep 2025 03:58:11 +0000 Subject: [PATCH 12/21] docs(upstream-merge): add MERGE_PLAN and MERGE_REPORT under .github/auto --- .github/auto/MERGE_PLAN.md | 33 +++++++++++++++++++++++++++++++++ .github/auto/MERGE_REPORT.md | 19 +++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 .github/auto/MERGE_PLAN.md create mode 100644 .github/auto/MERGE_REPORT.md diff --git a/.github/auto/MERGE_PLAN.md b/.github/auto/MERGE_PLAN.md new file mode 100644 index 000000000000..553aac343af6 --- /dev/null +++ b/.github/auto/MERGE_PLAN.md @@ -0,0 +1,33 @@ +Upstream merge plan + +Mode +- Strategy: by-bucket (from MERGE_MODE) + +Buckets and policy +- Prefer ours: `codex-rs/tui/**`, `codex-cli/**`, `codex-rs/core/src/{openai_tools.rs,codex.rs,agent_tool.rs,default_client.rs}`, `codex-rs/protocol/src/models.rs`, `.github/workflows/**`, `docs/**`, `AGENTS.md`, `README.md`, `CHANGELOG.md`. + - Rationale: preserve fork UX (TUI ordering, browser/agent tools, UA/version). + - Action: keep local versions unless a change is clearly compatible and beneficial; otherwise note in report. +- Prefer theirs: `codex-rs/common/**`, `codex-rs/exec/**`, `codex-rs/file-search/**`. + - Rationale: upstream correctness and compatibility; adopt unless it breaks our build or behavior. +- Default: adopt upstream for other paths when conflict-free and compatible. +- Purge: `.github/codex-cli-*.{png,jpg,jpeg,webp}` remain deleted even if reintroduced. + +Explicit invariants to preserve +- Tool families: keep `browser_*`, `agent_*`, and `web_fetch` handlers and schemas; maintain gating for browser tools exposure. +- Screenshot UX: preserve queuing semantics across turns (producer/consumer paths unchanged). +- Version/UA: continue to use `codex_version::version()` and `get_codex_user_agent_default()`. +- Public API: keep `ModelClient`, `Prompt`, `ResponseEvent`, `ResponseStream` re-exports; keep `codex_core::models` alias. +- Dependencies: do not remove ICU/sys-locale without confirming no usages. + +Process +1) Fetch origin/upstream; work on existing `upstream-merge` branch. +2) Merge `upstream/main` with `--no-commit`. +3) Resolve conflicts by bucket rules above; document notable choices in MERGE_REPORT.md. +4) Ensure purge globs are removed. +5) Run `scripts/upstream-merge/verify.sh` and fix minimal issues. +6) Build check via `./build-fast.sh` with zero warnings. +7) Commit with a conventional message and push `upstream-merge`. + +Notes from artifacts +- CHANGE_HISTOGRAM indicates heavy TUI churn upstream; we will keep ours unless specific improvements are compatible. +- Upstream touched `codex-rs/core/src/codex.rs` and tests; we will carefully merge non-conflicting improvements while preserving our tool wiring and UA/version semantics. diff --git a/.github/auto/MERGE_REPORT.md b/.github/auto/MERGE_REPORT.md new file mode 100644 index 000000000000..d1977e191b3c --- /dev/null +++ b/.github/auto/MERGE_REPORT.md @@ -0,0 +1,19 @@ +Upstream Merge Report + +Summary +- Merged upstream/main into `upstream-merge` using by-bucket policy. +- Build and guard checks passed. + +Incorporated +- Core: upstream additions around review mode; wired by exporting `Review*` types via `codex_core::protocol` for API compatibility. +- Docs: adopted upstream simplification for npm publishing in `docs/release_management.md`. + +Dropped/Kept Ours +- Workflows: kept our `.github/workflows/**`; removed upstream `.github/workflows/rust-release.yml` (policy: prefer ours). +- CLI: resolved conflict in `codex-cli/package.json` by keeping ours (policy: prefer ours). +- TUI: upstream touched many files (see histogram) but no direct conflicts; we retained our TUI implementation per policy. + +Other Notes +- Purge list: no banned `.github/codex-cli-*.(png|jpg|jpeg|webp)` reintroduced. +- Invariants preserved: browser_*/agent_* tools and gating untouched; screenshot queuing unchanged; UA/version helpers intact; public re-exports unchanged (plus added review re-exports). +- Verification: `scripts/upstream-merge/verify.sh` OK; `./build-fast.sh` succeeded with zero warnings. From c4a09f1553a19a0d2058634f7d28bc89e0154d8c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 17 Sep 2025 04:07:15 +0000 Subject: [PATCH 13/21] chore(merge): enforce policy removals (images + cargo caches) --- .github/auto/MERGE_PLAN.md | 33 --------------------------------- .github/auto/MERGE_REPORT.md | 19 ------------------- 2 files changed, 52 deletions(-) delete mode 100644 .github/auto/MERGE_PLAN.md delete mode 100644 .github/auto/MERGE_REPORT.md diff --git a/.github/auto/MERGE_PLAN.md b/.github/auto/MERGE_PLAN.md deleted file mode 100644 index 553aac343af6..000000000000 --- a/.github/auto/MERGE_PLAN.md +++ /dev/null @@ -1,33 +0,0 @@ -Upstream merge plan - -Mode -- Strategy: by-bucket (from MERGE_MODE) - -Buckets and policy -- Prefer ours: `codex-rs/tui/**`, `codex-cli/**`, `codex-rs/core/src/{openai_tools.rs,codex.rs,agent_tool.rs,default_client.rs}`, `codex-rs/protocol/src/models.rs`, `.github/workflows/**`, `docs/**`, `AGENTS.md`, `README.md`, `CHANGELOG.md`. - - Rationale: preserve fork UX (TUI ordering, browser/agent tools, UA/version). - - Action: keep local versions unless a change is clearly compatible and beneficial; otherwise note in report. -- Prefer theirs: `codex-rs/common/**`, `codex-rs/exec/**`, `codex-rs/file-search/**`. - - Rationale: upstream correctness and compatibility; adopt unless it breaks our build or behavior. -- Default: adopt upstream for other paths when conflict-free and compatible. -- Purge: `.github/codex-cli-*.{png,jpg,jpeg,webp}` remain deleted even if reintroduced. - -Explicit invariants to preserve -- Tool families: keep `browser_*`, `agent_*`, and `web_fetch` handlers and schemas; maintain gating for browser tools exposure. -- Screenshot UX: preserve queuing semantics across turns (producer/consumer paths unchanged). -- Version/UA: continue to use `codex_version::version()` and `get_codex_user_agent_default()`. -- Public API: keep `ModelClient`, `Prompt`, `ResponseEvent`, `ResponseStream` re-exports; keep `codex_core::models` alias. -- Dependencies: do not remove ICU/sys-locale without confirming no usages. - -Process -1) Fetch origin/upstream; work on existing `upstream-merge` branch. -2) Merge `upstream/main` with `--no-commit`. -3) Resolve conflicts by bucket rules above; document notable choices in MERGE_REPORT.md. -4) Ensure purge globs are removed. -5) Run `scripts/upstream-merge/verify.sh` and fix minimal issues. -6) Build check via `./build-fast.sh` with zero warnings. -7) Commit with a conventional message and push `upstream-merge`. - -Notes from artifacts -- CHANGE_HISTOGRAM indicates heavy TUI churn upstream; we will keep ours unless specific improvements are compatible. -- Upstream touched `codex-rs/core/src/codex.rs` and tests; we will carefully merge non-conflicting improvements while preserving our tool wiring and UA/version semantics. diff --git a/.github/auto/MERGE_REPORT.md b/.github/auto/MERGE_REPORT.md deleted file mode 100644 index d1977e191b3c..000000000000 --- a/.github/auto/MERGE_REPORT.md +++ /dev/null @@ -1,19 +0,0 @@ -Upstream Merge Report - -Summary -- Merged upstream/main into `upstream-merge` using by-bucket policy. -- Build and guard checks passed. - -Incorporated -- Core: upstream additions around review mode; wired by exporting `Review*` types via `codex_core::protocol` for API compatibility. -- Docs: adopted upstream simplification for npm publishing in `docs/release_management.md`. - -Dropped/Kept Ours -- Workflows: kept our `.github/workflows/**`; removed upstream `.github/workflows/rust-release.yml` (policy: prefer ours). -- CLI: resolved conflict in `codex-cli/package.json` by keeping ours (policy: prefer ours). -- TUI: upstream touched many files (see histogram) but no direct conflicts; we retained our TUI implementation per policy. - -Other Notes -- Purge list: no banned `.github/codex-cli-*.(png|jpg|jpeg|webp)` reintroduced. -- Invariants preserved: browser_*/agent_* tools and gating untouched; screenshot queuing unchanged; UA/version helpers intact; public re-exports unchanged (plus added review re-exports). -- Verification: `scripts/upstream-merge/verify.sh` OK; `./build-fast.sh` succeeded with zero warnings. From 5d87f5d24a2fe0364ecc90521a9581e5a31d6df1 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 16 Sep 2025 21:36:13 -0700 Subject: [PATCH 14/21] fix: ensure pnpm is installed before running `npm install` (#3763) Note we do the same thing in `ci.yml`: https://github.com/openai/codex/blob/791d7b125f4166ef576531075688aac339350011/.github/workflows/ci.yml#L17-L25 --- .github/workflows/rust-release.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index 804ac83d965e..1db294baab96 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -193,6 +193,14 @@ jobs: # Publish to npm using OIDC authentication. # July 31, 2025: https://github.blog/changelog/2025-07-31-npm-trusted-publishing-with-oidc-is-generally-available/ # npm docs: https://docs.npmjs.com/trusted-publishers + + # package.json has `packageManager: "pnpm@`, so we must get pnpm on the + # PATH before setting up Node.js. + - name: Setup pnpm + uses: pnpm/action-setup@v4 + with: + run_install: false + - name: Setup Node.js uses: actions/setup-node@v5 with: From 5332f6e2156e83a2dcd654a98a72696aa455f750 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 16 Sep 2025 22:55:53 -0700 Subject: [PATCH 15/21] fix: make publish-npm its own job with specific permissions (#3767) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The build for `v0.37.0-alpha.3` failed on the `Create GitHub Release` step: https://github.com/openai/codex/actions/runs/17786866086/job/50556513221 with: ``` ⚠️ GitHub release failed with status: 403 {"message":"Resource not accessible by integration","documentation_url":"https://docs.github.com/rest/releases/releases#create-a-release","status":"403"} Skip retry — your GitHub token/PAT does not have the required permission to create a release ``` I believe I should have not introduced a top-level `permissions` for the workflow in https://github.com/openai/codex/pull/3431 because that affected the `permissions` for each job in the workflow. This PR introduces `publish-npm` as its own job, which allows us to: - consolidate all the Node.js-related steps required for publishing - limit the reach of the `id-token: write` permission - skip it altogether if is an alpha build With this PR, each of `release`, `publish-npm`, and `update-branch` has an explicit `permissions` block. --- .github/workflows/rust-release.yml | 72 ++++++++++++++++++------------ 1 file changed, 44 insertions(+), 28 deletions(-) diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index 1db294baab96..8e6ac6582c2f 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -11,9 +11,6 @@ on: tags: - "rust-v*.*.*" -permissions: - id-token: write # Required for OIDC - concurrency: group: ${{ github.workflow }} cancel-in-progress: true @@ -170,6 +167,12 @@ jobs: needs: build name: release runs-on: ubuntu-latest + permissions: + contents: write + actions: read + outputs: + version: ${{ steps.release_name.outputs.name }} + tag: ${{ github.ref_name }} steps: - name: Checkout repository @@ -190,28 +193,6 @@ jobs: version="${GITHUB_REF_NAME#rust-v}" echo "name=${version}" >> $GITHUB_OUTPUT - # Publish to npm using OIDC authentication. - # July 31, 2025: https://github.blog/changelog/2025-07-31-npm-trusted-publishing-with-oidc-is-generally-available/ - # npm docs: https://docs.npmjs.com/trusted-publishers - - # package.json has `packageManager: "pnpm@`, so we must get pnpm on the - # PATH before setting up Node.js. - - name: Setup pnpm - uses: pnpm/action-setup@v4 - with: - run_install: false - - - name: Setup Node.js - uses: actions/setup-node@v5 - with: - node-version: 22 - registry-url: "https://registry.npmjs.org" - scope: "@openai" - - # Trusted publishing requires npm CLI version 11.5.1 or later. - - name: Update npm - run: npm install -g npm@latest - - name: Stage npm package env: GH_TOKEN: ${{ github.token }} @@ -245,11 +226,46 @@ jobs: tag: ${{ github.ref_name }} config: .github/dotslash-config.json + # Publish to npm using OIDC authentication. + # July 31, 2025: https://github.blog/changelog/2025-07-31-npm-trusted-publishing-with-oidc-is-generally-available/ + # npm docs: https://docs.npmjs.com/trusted-publishers + publish-npm: + # Skip this step for pre-releases (alpha/beta). + if: ${{ !contains(needs.release.outputs.version, '-') }} + name: publish-npm + needs: release + runs-on: ubuntu-latest + permissions: + id-token: write # Required for OIDC + contents: read + + steps: + - name: Setup Node.js + uses: actions/setup-node@v5 + with: + node-version: 22 + registry-url: "https://registry.npmjs.org" + scope: "@openai" + + # Trusted publishing requires npm CLI version 11.5.1 or later. + - name: Update npm + run: npm install -g npm@latest + + - name: Download npm tarball from release + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -euo pipefail + version="${{ needs.release.outputs.version }}" + tag="${{ needs.release.outputs.tag }}" + mkdir -p dist/npm + gh release download "$tag" \ + --pattern "codex-npm-${version}.tgz" \ + --dir dist/npm + # No NODE_AUTH_TOKEN needed because we use OIDC. - name: Publish to npm - # Do not publish alphas to npm. - if: ${{ !contains(steps.release_name.outputs.name, '-') }} - run: npm publish "${GITHUB_WORKSPACE}/dist/npm/codex-npm-${{ steps.release_name.outputs.name }}.tgz" + run: npm publish "${GITHUB_WORKSPACE}/dist/npm/codex-npm-${{ needs.release.outputs.version }}.tgz" update-branch: name: Update latest-alpha-cli branch From e5fdb5b0fd159c0ea80e872ff2111b49391fbbc7 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 17 Sep 2025 11:05:22 -0700 Subject: [PATCH 16/21] fix: specify --repo when calling gh (#3806) Often, `gh` infers `--repo` when it is run from a Git clone, but our `publish-npm` step is designed to avoid the overhead of cloning the repo, so add the `--repo` option explicitly to fix things. --- .github/workflows/rust-release.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index 8e6ac6582c2f..e3776f149c01 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -260,6 +260,7 @@ jobs: tag="${{ needs.release.outputs.tag }}" mkdir -p dist/npm gh release download "$tag" \ + --repo "${GITHUB_REPOSITORY}" \ --pattern "codex-npm-${version}.tgz" \ --dir dist/npm From 208089e58e9e5c49a3669dc995d8854560480a66 Mon Sep 17 00:00:00 2001 From: Abhishek Bhardwaj Date: Wed, 17 Sep 2025 11:06:59 -0700 Subject: [PATCH 17/21] AGENTS.md: Add instruction to install missing commands (#3807) This change instructs the model to install any missing command. Else tokens are wasted when it tries to run commands that aren't available multiple times before installing them. --- AGENTS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/AGENTS.md b/AGENTS.md index 697de7009fed..288719447e47 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,6 +4,7 @@ In the codex-rs folder where the rust code lives: - Crate names are prefixed with `codex-`. For example, the `core` folder's crate is named `codex-core` - When using format! and you can inline variables into {}, always do that. +- Install any commands the repo relies on (for example `just`, `rg`, or `cargo-insta`) if they aren't already available before running instructions here. - Never add or modify any code related to `CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR` or `CODEX_SANDBOX_ENV_VAR`. - You operate in a sandbox where `CODEX_SANDBOX_NETWORK_DISABLED=1` will be set whenever you use the `shell` tool. Any existing code that uses `CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR` was authored with this fact in mind. It is often used to early exit out of tests that the author knew you would not be able to run given your sandbox limitations. - Similarly, when you spawn a process using Seatbelt (`/usr/bin/sandbox-exec`), `CODEX_SANDBOX=seatbelt` will be set on the child process. Integration tests that want to run Seatbelt themselves cannot be run under Seatbelt, so checks for `CODEX_SANDBOX=seatbelt` are also often used to early exit out of tests, as appropriate. From 530382db0512e72cdd4e94271e2cbdb1d14b0b73 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Wed, 17 Sep 2025 11:23:46 -0700 Subject: [PATCH 18/21] Use agent reply text in turn notifications (#3756) Instead of "Agent turn complete", turn-complete notifications now include the first handful of chars from the agent's final message. --- codex-rs/tui/src/chatwidget.rs | 37 ++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index fb91d75456b5..7ab954f263a9 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -255,7 +255,7 @@ impl ChatWidget { self.request_redraw(); } - fn on_task_complete(&mut self) { + fn on_task_complete(&mut self, last_agent_message: Option) { // If a stream is currently active, finalize only that stream to flush any tail // without emitting stray headers for other streams. if self.stream.is_write_cycle_active() { @@ -270,7 +270,9 @@ impl ChatWidget { // If there is a queued user message, send exactly one now to begin the next turn. self.maybe_send_next_queued_input(); // Emit a notification when the turn completes (suppressed if focused). - self.notify(Notification::AgentTurnComplete); + self.notify(Notification::AgentTurnComplete { + response: last_agent_message.unwrap_or_default(), + }); } pub(crate) fn set_token_info(&mut self, info: Option) { @@ -1082,7 +1084,9 @@ impl ChatWidget { } EventMsg::AgentReasoningSectionBreak(_) => self.on_reasoning_section_break(), EventMsg::TaskStarted(_) => self.on_task_started(), - EventMsg::TaskComplete(TaskCompleteEvent { .. }) => self.on_task_complete(), + EventMsg::TaskComplete(TaskCompleteEvent { last_agent_message }) => { + self.on_task_complete(last_agent_message) + } EventMsg::TokenCount(ev) => self.set_token_info(ev.info), EventMsg::Error(ErrorEvent { message }) => self.on_error(message), EventMsg::TurnAborted(ev) => match ev.reason { @@ -1479,7 +1483,7 @@ impl WidgetRef for &ChatWidget { } enum Notification { - AgentTurnComplete, + AgentTurnComplete { response: String }, ExecApprovalRequested { command: String }, EditApprovalRequested { cwd: PathBuf, changes: Vec }, } @@ -1487,7 +1491,10 @@ enum Notification { impl Notification { fn display(&self) -> String { match self { - Notification::AgentTurnComplete => "Agent turn complete".to_string(), + Notification::AgentTurnComplete { response } => { + Notification::agent_turn_preview(response) + .unwrap_or_else(|| "Agent turn complete".to_string()) + } Notification::ExecApprovalRequested { command } => { format!("Approval requested: {}", truncate_text(command, 30)) } @@ -1507,7 +1514,7 @@ impl Notification { fn type_name(&self) -> &str { match self { - Notification::AgentTurnComplete => "agent-turn-complete", + Notification::AgentTurnComplete { .. } => "agent-turn-complete", Notification::ExecApprovalRequested { .. } | Notification::EditApprovalRequested { .. } => "approval-requested", } @@ -1519,8 +1526,26 @@ impl Notification { Notifications::Custom(allowed) => allowed.iter().any(|a| a == self.type_name()), } } + + fn agent_turn_preview(response: &str) -> Option { + let mut normalized = String::new(); + for part in response.split_whitespace() { + if !normalized.is_empty() { + normalized.push(' '); + } + normalized.push_str(part); + } + let trimmed = normalized.trim(); + if trimmed.is_empty() { + None + } else { + Some(truncate_text(trimmed, AGENT_NOTIFICATION_PREVIEW_GRAPHEMES)) + } + } } +const AGENT_NOTIFICATION_PREVIEW_GRAPHEMES: usize = 200; + const EXAMPLE_PROMPTS: [&str; 6] = [ "Explain this codebase", "Summarize recent commits", From cd82c17bdd48666187d05c6579fed8521e9d547e Mon Sep 17 00:00:00 2001 From: just-every-code Date: Wed, 17 Sep 2025 18:59:54 +0000 Subject: [PATCH 19/21] docs(upstream-merge): add MERGE_PLAN, MERGE_REPORT and PR body for this sync --- .github/auto/MERGE_PLAN.md | 47 ++++++++++++++++++++++++++++++++++++ .github/auto/MERGE_REPORT.md | 25 +++++++++++++++++++ .github/auto/PR_BODY.md | 20 +++++++++++++++ 3 files changed, 92 insertions(+) create mode 100644 .github/auto/MERGE_PLAN.md create mode 100644 .github/auto/MERGE_REPORT.md create mode 100644 .github/auto/PR_BODY.md diff --git a/.github/auto/MERGE_PLAN.md b/.github/auto/MERGE_PLAN.md new file mode 100644 index 000000000000..b3b8daa33b3b --- /dev/null +++ b/.github/auto/MERGE_PLAN.md @@ -0,0 +1,47 @@ +Upstream merge plan (by-bucket) + +Mode: by-bucket + +Strategy +- Adopt upstream broadly outside protected areas; resolve conflicts minimally. +- Prefer ours for TUI and fork-only core glue per policy to preserve browser/agent tools, UA/version semantics, and strict ordering. +- Prefer theirs for shared libraries (common, exec, file-search) unless it breaks our build or documented behavior. +- Purge any reintroduced GitHub image assets matching purge globs. + +Buckets +- prefer_ours_globs + - codex-rs/tui/** + - codex-cli/** + - codex-rs/core/src/openai_tools.rs + - codex-rs/core/src/codex.rs + - codex-rs/core/src/agent_tool.rs + - codex-rs/core/src/default_client.rs + - codex-rs/protocol/src/models.rs + - .github/workflows/** + - docs/** + - AGENTS.md, README.md, CHANGELOG.md + +- prefer_theirs_globs + - codex-rs/common/** + - codex-rs/exec/** + - codex-rs/file-search/** + +- purge_globs + - .github/codex-cli-*.png|jpg|jpeg|webp + +Invariants to preserve +- Handlers and schemas for browser_* and agent_* tools; include web_fetch. +- Exposure gating for browser tools remains intact. +- Screenshot queuing semantics across turns preserved. +- Version/UA helpers: codex_version::version() and get_codex_user_agent_default(). +- Public re-exports in codex-core: ModelClient, Prompt, ResponseEvent, ResponseStream. +- codex_core::models alias to protocol models remains. +- Do not remove ICU/sys-locale deps if referenced. + +Process +1) Merge upstream/main into upstream-merge with --no-commit. +2) Resolve conflicts per buckets (ours/theirs/default adopt upstream). +3) Ensure purge_globs stay deleted if reintroduced. +4) Run scripts/upstream-merge/verify.sh and ./build-fast.sh; fix minimal issues. +5) Commit with a conventional message and summarize results in MERGE_REPORT.md. + diff --git a/.github/auto/MERGE_REPORT.md b/.github/auto/MERGE_REPORT.md new file mode 100644 index 000000000000..65b4026cc651 --- /dev/null +++ b/.github/auto/MERGE_REPORT.md @@ -0,0 +1,25 @@ +Upstream merge report + +Branch: upstream-merge +Upstream: openai/codex@main +Mode: by-bucket + +Incorporated +- Adopted upstream changes across core, common, exec, and workspace where no fork-only invariants were impacted. +- No newly reintroduced purge_globs found; image purge not required this round. + +Dropped/Kept Ours +- codex-rs/tui/src/chatwidget.rs: kept ours to preserve strict streaming ordering and RunningCommand/HistoryCell semantics that our TUI relies on. +- Preserved fork-only core glue and APIs (no upstream changes conflicting this round): + - codex-rs/core/src/openai_tools.rs (browser_*/agent_* tools and web_fetch exposure) + - codex-rs/core/src/codex.rs (execution events + UA/version handling) + - codex-rs/core/src/agent_tool.rs (multi-agent orchestration) + - codex-rs/core/src/default_client.rs (versioned UA) + - codex-rs/protocol/src/models.rs (models mapping and aliases) + +Other Notes +- Public re-exports (ModelClient, Prompt, ResponseEvent, ResponseStream) were preserved. +- codex_core::models alias remains intact. +- Verify script passed: tools/UA/version guards OK. +- build-fast.sh passed with zero warnings. + diff --git a/.github/auto/PR_BODY.md b/.github/auto/PR_BODY.md new file mode 100644 index 000000000000..0df670fae028 --- /dev/null +++ b/.github/auto/PR_BODY.md @@ -0,0 +1,20 @@ +Title: merge(upstream): sync fork with openai/codex main (by-bucket) + +Summary +- Merge upstream main into our fork’s `upstream-merge` using by-bucket policy. +- Keep our TUI and fork-only core glue; adopt upstream improvements elsewhere. + +Key Decisions +- TUI: kept ours for `codex-rs/tui/src/chatwidget.rs` to preserve strict ordering and HistoryCell/RunningCommand semantics. +- Core invariants preserved: browser_*/agent_* tools and web_fetch exposure gating; screenshot queue semantics; UA/version helpers. +- Prefer theirs for shared crates (common/exec/file-search) where applicable. +- No purge_globs assets reintroduced. + +Verification +- scripts/upstream-merge/verify.sh: PASS +- ./build-fast.sh: PASS (no warnings) + +Notes +- Public re-exports (ModelClient, Prompt, ResponseEvent, ResponseStream) retained. +- codex_core::models alias preserved. + From cffba050ed4879f1297f05f874168b080fc37ce0 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 17 Sep 2025 19:09:13 +0000 Subject: [PATCH 20/21] chore(merge): enforce policy removals (images + cargo caches) --- .github/auto/MERGE_PLAN.md | 47 ------------------------------------ .github/auto/MERGE_REPORT.md | 25 ------------------- .github/auto/PR_BODY.md | 20 --------------- 3 files changed, 92 deletions(-) delete mode 100644 .github/auto/MERGE_PLAN.md delete mode 100644 .github/auto/MERGE_REPORT.md delete mode 100644 .github/auto/PR_BODY.md diff --git a/.github/auto/MERGE_PLAN.md b/.github/auto/MERGE_PLAN.md deleted file mode 100644 index b3b8daa33b3b..000000000000 --- a/.github/auto/MERGE_PLAN.md +++ /dev/null @@ -1,47 +0,0 @@ -Upstream merge plan (by-bucket) - -Mode: by-bucket - -Strategy -- Adopt upstream broadly outside protected areas; resolve conflicts minimally. -- Prefer ours for TUI and fork-only core glue per policy to preserve browser/agent tools, UA/version semantics, and strict ordering. -- Prefer theirs for shared libraries (common, exec, file-search) unless it breaks our build or documented behavior. -- Purge any reintroduced GitHub image assets matching purge globs. - -Buckets -- prefer_ours_globs - - codex-rs/tui/** - - codex-cli/** - - codex-rs/core/src/openai_tools.rs - - codex-rs/core/src/codex.rs - - codex-rs/core/src/agent_tool.rs - - codex-rs/core/src/default_client.rs - - codex-rs/protocol/src/models.rs - - .github/workflows/** - - docs/** - - AGENTS.md, README.md, CHANGELOG.md - -- prefer_theirs_globs - - codex-rs/common/** - - codex-rs/exec/** - - codex-rs/file-search/** - -- purge_globs - - .github/codex-cli-*.png|jpg|jpeg|webp - -Invariants to preserve -- Handlers and schemas for browser_* and agent_* tools; include web_fetch. -- Exposure gating for browser tools remains intact. -- Screenshot queuing semantics across turns preserved. -- Version/UA helpers: codex_version::version() and get_codex_user_agent_default(). -- Public re-exports in codex-core: ModelClient, Prompt, ResponseEvent, ResponseStream. -- codex_core::models alias to protocol models remains. -- Do not remove ICU/sys-locale deps if referenced. - -Process -1) Merge upstream/main into upstream-merge with --no-commit. -2) Resolve conflicts per buckets (ours/theirs/default adopt upstream). -3) Ensure purge_globs stay deleted if reintroduced. -4) Run scripts/upstream-merge/verify.sh and ./build-fast.sh; fix minimal issues. -5) Commit with a conventional message and summarize results in MERGE_REPORT.md. - diff --git a/.github/auto/MERGE_REPORT.md b/.github/auto/MERGE_REPORT.md deleted file mode 100644 index 65b4026cc651..000000000000 --- a/.github/auto/MERGE_REPORT.md +++ /dev/null @@ -1,25 +0,0 @@ -Upstream merge report - -Branch: upstream-merge -Upstream: openai/codex@main -Mode: by-bucket - -Incorporated -- Adopted upstream changes across core, common, exec, and workspace where no fork-only invariants were impacted. -- No newly reintroduced purge_globs found; image purge not required this round. - -Dropped/Kept Ours -- codex-rs/tui/src/chatwidget.rs: kept ours to preserve strict streaming ordering and RunningCommand/HistoryCell semantics that our TUI relies on. -- Preserved fork-only core glue and APIs (no upstream changes conflicting this round): - - codex-rs/core/src/openai_tools.rs (browser_*/agent_* tools and web_fetch exposure) - - codex-rs/core/src/codex.rs (execution events + UA/version handling) - - codex-rs/core/src/agent_tool.rs (multi-agent orchestration) - - codex-rs/core/src/default_client.rs (versioned UA) - - codex-rs/protocol/src/models.rs (models mapping and aliases) - -Other Notes -- Public re-exports (ModelClient, Prompt, ResponseEvent, ResponseStream) were preserved. -- codex_core::models alias remains intact. -- Verify script passed: tools/UA/version guards OK. -- build-fast.sh passed with zero warnings. - diff --git a/.github/auto/PR_BODY.md b/.github/auto/PR_BODY.md deleted file mode 100644 index 0df670fae028..000000000000 --- a/.github/auto/PR_BODY.md +++ /dev/null @@ -1,20 +0,0 @@ -Title: merge(upstream): sync fork with openai/codex main (by-bucket) - -Summary -- Merge upstream main into our fork’s `upstream-merge` using by-bucket policy. -- Keep our TUI and fork-only core glue; adopt upstream improvements elsewhere. - -Key Decisions -- TUI: kept ours for `codex-rs/tui/src/chatwidget.rs` to preserve strict ordering and HistoryCell/RunningCommand semantics. -- Core invariants preserved: browser_*/agent_* tools and web_fetch exposure gating; screenshot queue semantics; UA/version helpers. -- Prefer theirs for shared crates (common/exec/file-search) where applicable. -- No purge_globs assets reintroduced. - -Verification -- scripts/upstream-merge/verify.sh: PASS -- ./build-fast.sh: PASS (no warnings) - -Notes -- Public re-exports (ModelClient, Prompt, ResponseEvent, ResponseStream) retained. -- codex_core::models alias preserved. - From c9505488a120299b339814d73f57817ee79e114f Mon Sep 17 00:00:00 2001 From: Thibault Sottiaux Date: Wed, 17 Sep 2025 16:48:20 -0700 Subject: [PATCH 21/21] chore: update "Codex CLI harness, sandboxing, and approvals" section (#3822) --- codex-rs/core/gpt_5_codex_prompt.md | 30 ++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/codex-rs/core/gpt_5_codex_prompt.md b/codex-rs/core/gpt_5_codex_prompt.md index 2c49fafec62a..9a298f460f41 100644 --- a/codex-rs/core/gpt_5_codex_prompt.md +++ b/codex-rs/core/gpt_5_codex_prompt.md @@ -26,37 +26,41 @@ When using the planning tool: ## Codex CLI harness, sandboxing, and approvals -The Codex CLI harness supports several different sandboxing, and approval configurations that the user can choose from. +The Codex CLI harness supports several different configurations for sandboxing and escalation approvals that the user can choose from. -Filesystem sandboxing defines which files can be read or written. The options are: -- **read-only**: You can only read files. -- **workspace-write**: You can read files. You can write to files in this folder, but not outside it. -- **danger-full-access**: No filesystem sandboxing. +Filesystem sandboxing defines which files can be read or written. The options for `sandbox_mode` are: +- **read-only**: The sandbox only permits reading files. +- **workspace-write**: The sandbox permits reading files, and editing files in `cwd` and `writable_roots`. Editing files in other directories requires approval. +- **danger-full-access**: No filesystem sandboxing - all commands are permitted. -Network sandboxing defines whether network can be accessed without approval. Options are +Network sandboxing defines whether network can be accessed without approval. Options for `network_access` are: - **restricted**: Requires approval - **enabled**: No approval needed -Approvals are your mechanism to get user consent to perform more privileged actions. Although they introduce friction to the user because your work is paused until the user responds, you should leverage them to accomplish your important work. Do not let these settings or the sandbox deter you from attempting to accomplish the user's task unless it is set to "never", in which case never ask for approvals. - -Approval options are +Approvals are your mechanism to get user consent to run shell commands without the sandbox. Possible configuration options for `approval_policy` are - **untrusted**: The harness will escalate most commands for user approval, apart from a limited allowlist of safe "read" commands. - **on-failure**: The harness will allow all commands to run in the sandbox (if enabled), and failures will be escalated to the user for approval to run again without the sandbox. - **on-request**: Commands will be run in the sandbox by default, and you can specify in your tool call if you want to escalate a command to run without sandboxing. (Note that this mode is not always available. If it is, you'll see parameters for it in the `shell` command description.) - **never**: This is a non-interactive mode where you may NEVER ask the user for approval to run commands. Instead, you must always persist and work around constraints to solve the task for the user. You MUST do your utmost best to finish the task and validate your work before yielding. If this mode is paired with `danger-full-access`, take advantage of it to deliver the best outcome for the user. Further, in this mode, your default testing philosophy is overridden: Even if you don't see local patterns for testing, you may add tests and scripts to validate your work. Just remove them before yielding. -When you are running with approvals `on-request`, and sandboxing enabled, here are scenarios where you'll need to request approval: -- You need to run a command that writes to a directory that requires it (e.g. running tests that write to /tmp) +When you are running with `approval_policy == on-request`, and sandboxing enabled, here are scenarios where you'll need to request approval: +- You need to run a command that writes to a directory that requires it (e.g. running tests that write to /var) - You need to run a GUI app (e.g., open/xdg-open/osascript) to open browsers or files. - You are running sandboxed and need to run a command that requires network access (e.g. installing packages) -- If you run a command that is important to solving the user's query, but it fails because of sandboxing, rerun the command with approval. +- If you run a command that is important to solving the user's query, but it fails because of sandboxing, rerun the command with approval. ALWAYS proceed to use the `with_escalated_permissions` and `justification` parameters - do not message the user before requesting approval for the command. - You are about to take a potentially destructive action such as an `rm` or `git reset` that the user did not explicitly ask for - (for all of these, you should weigh alternative paths that do not require approval) -When sandboxing is set to read-only, you'll need to request approval for any command that isn't a read. +When `sandbox_mode` is set to read-only, you'll need to request approval for any command that isn't a read. You will be told what filesystem sandboxing, network sandboxing, and approval mode are active in a developer or user message. If you are not told about this, assume that you are running with workspace-write, network sandboxing enabled, and approval on-failure. +Although they introduce friction to the user because your work is paused until the user responds, you should leverage them when necessary to accomplish important work. If the completing the task requires escalated permissions, Do not let these settings or the sandbox deter you from attempting to accomplish the user's task unless it is set to "never", in which case never ask for approvals. + +When requesting approval to execute a command that will require escalated privileges: + - Provide the `with_escalated_permissions` parameter with the boolean value true + - Include a short, 1 sentence explanation for why you need to enable `with_escalated_permissions` in the justification parameter + ## Special user requests - If the user makes a simple request (such as asking for the time) which you can fulfill by running a terminal command (such as `date`), you should do so.