feat: add configurable write_stdin timeout (#12228)
Add max timeout as config for `write_stdin`. This is only used for empty `write_stdin`. Also increased the default value from 30s to 5mins.
This commit is contained in:
parent
f595e11723
commit
547f462385
5 changed files with 53 additions and 9 deletions
|
|
@ -1295,6 +1295,12 @@
|
|||
"default": null,
|
||||
"description": "Settings for app-specific controls."
|
||||
},
|
||||
"background_terminal_timeout": {
|
||||
"description": "Maximum poll window for background terminal output (`write_stdin`), in milliseconds. Default: `300000` (5 minutes).",
|
||||
"format": "uint64",
|
||||
"minimum": 0.0,
|
||||
"type": "integer"
|
||||
},
|
||||
"chatgpt_base_url": {
|
||||
"description": "Base URL for requests to ChatGPT (as opposed to the OpenAI API).",
|
||||
"type": "string"
|
||||
|
|
|
|||
|
|
@ -1287,7 +1287,9 @@ impl Session {
|
|||
let services = SessionServices {
|
||||
mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())),
|
||||
mcp_startup_cancellation_token: Mutex::new(CancellationToken::new()),
|
||||
unified_exec_manager: UnifiedExecProcessManager::default(),
|
||||
unified_exec_manager: UnifiedExecProcessManager::new(
|
||||
config.background_terminal_max_timeout,
|
||||
),
|
||||
zsh_exec_bridge,
|
||||
analytics_events_client: AnalyticsEventsClient::new(
|
||||
Arc::clone(&config),
|
||||
|
|
@ -7376,7 +7378,9 @@ mod tests {
|
|||
let services = SessionServices {
|
||||
mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())),
|
||||
mcp_startup_cancellation_token: Mutex::new(CancellationToken::new()),
|
||||
unified_exec_manager: UnifiedExecProcessManager::default(),
|
||||
unified_exec_manager: UnifiedExecProcessManager::new(
|
||||
config.background_terminal_max_timeout,
|
||||
),
|
||||
zsh_exec_bridge: ZshExecBridge::default(),
|
||||
analytics_events_client: AnalyticsEventsClient::new(
|
||||
Arc::clone(&config),
|
||||
|
|
@ -7525,7 +7529,9 @@ mod tests {
|
|||
let services = SessionServices {
|
||||
mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())),
|
||||
mcp_startup_cancellation_token: Mutex::new(CancellationToken::new()),
|
||||
unified_exec_manager: UnifiedExecProcessManager::default(),
|
||||
unified_exec_manager: UnifiedExecProcessManager::new(
|
||||
config.background_terminal_max_timeout,
|
||||
),
|
||||
zsh_exec_bridge: ZshExecBridge::default(),
|
||||
analytics_events_client: AnalyticsEventsClient::new(
|
||||
Arc::clone(&config),
|
||||
|
|
|
|||
|
|
@ -51,6 +51,8 @@ use crate::protocol::ReadOnlyAccess;
|
|||
use crate::protocol::SandboxPolicy;
|
||||
#[cfg(target_os = "macos")]
|
||||
use crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions;
|
||||
use crate::unified_exec::DEFAULT_MAX_BACKGROUND_TERMINAL_TIMEOUT_MS;
|
||||
use crate::unified_exec::MIN_EMPTY_YIELD_TIME_MS;
|
||||
use crate::windows_sandbox::WindowsSandboxLevelExt;
|
||||
use crate::windows_sandbox::resolve_windows_sandbox_mode;
|
||||
use codex_app_server_protocol::Tools;
|
||||
|
|
@ -380,6 +382,10 @@ pub struct Config {
|
|||
/// If set to `true`, used only the experimental unified exec tool.
|
||||
pub use_experimental_unified_exec_tool: bool,
|
||||
|
||||
/// Maximum poll window for background terminal output (`write_stdin`), in milliseconds.
|
||||
/// Default: `300000` (5 minutes).
|
||||
pub background_terminal_max_timeout: u64,
|
||||
|
||||
/// Settings for ghost snapshots (used for undo).
|
||||
pub ghost_snapshot: GhostSnapshotConfig,
|
||||
|
||||
|
|
@ -982,6 +988,10 @@ pub struct ConfigToml {
|
|||
/// Token budget applied when storing tool/function outputs in the context manager.
|
||||
pub tool_output_token_limit: Option<usize>,
|
||||
|
||||
/// Maximum poll window for background terminal output (`write_stdin`), in milliseconds.
|
||||
/// Default: `300000` (5 minutes).
|
||||
pub background_terminal_timeout: Option<u64>,
|
||||
|
||||
/// Optional absolute path to the Node runtime used by `js_repl`.
|
||||
pub js_repl_node_path: Option<AbsolutePathBuf>,
|
||||
|
||||
|
|
@ -1675,6 +1685,10 @@ impl Config {
|
|||
})
|
||||
.transpose()?
|
||||
.unwrap_or_default();
|
||||
let background_terminal_max_timeout = cfg
|
||||
.background_terminal_timeout
|
||||
.unwrap_or(DEFAULT_MAX_BACKGROUND_TERMINAL_TIMEOUT_MS)
|
||||
.max(MIN_EMPTY_YIELD_TIME_MS);
|
||||
|
||||
let ghost_snapshot = {
|
||||
let mut config = GhostSnapshotConfig::default();
|
||||
|
|
@ -1925,6 +1939,7 @@ impl Config {
|
|||
include_apply_patch_tool: include_apply_patch_tool_flag,
|
||||
web_search_mode: constrained_web_search_mode.value,
|
||||
use_experimental_unified_exec_tool,
|
||||
background_terminal_max_timeout,
|
||||
ghost_snapshot,
|
||||
features,
|
||||
suppress_unstable_features_warning: cfg
|
||||
|
|
@ -4346,6 +4361,7 @@ model_verbosity = "high"
|
|||
include_apply_patch_tool: false,
|
||||
web_search_mode: Constrained::allow_any(WebSearchMode::Cached),
|
||||
use_experimental_unified_exec_tool: !cfg!(windows),
|
||||
background_terminal_max_timeout: DEFAULT_MAX_BACKGROUND_TERMINAL_TIMEOUT_MS,
|
||||
ghost_snapshot: GhostSnapshotConfig::default(),
|
||||
features: Features::with_defaults(),
|
||||
suppress_unstable_features_warning: false,
|
||||
|
|
@ -4460,6 +4476,7 @@ model_verbosity = "high"
|
|||
include_apply_patch_tool: false,
|
||||
web_search_mode: Constrained::allow_any(WebSearchMode::Cached),
|
||||
use_experimental_unified_exec_tool: !cfg!(windows),
|
||||
background_terminal_max_timeout: DEFAULT_MAX_BACKGROUND_TERMINAL_TIMEOUT_MS,
|
||||
ghost_snapshot: GhostSnapshotConfig::default(),
|
||||
features: Features::with_defaults(),
|
||||
suppress_unstable_features_warning: false,
|
||||
|
|
@ -4572,6 +4589,7 @@ model_verbosity = "high"
|
|||
include_apply_patch_tool: false,
|
||||
web_search_mode: Constrained::allow_any(WebSearchMode::Cached),
|
||||
use_experimental_unified_exec_tool: !cfg!(windows),
|
||||
background_terminal_max_timeout: DEFAULT_MAX_BACKGROUND_TERMINAL_TIMEOUT_MS,
|
||||
ghost_snapshot: GhostSnapshotConfig::default(),
|
||||
features: Features::with_defaults(),
|
||||
suppress_unstable_features_warning: false,
|
||||
|
|
@ -4670,6 +4688,7 @@ model_verbosity = "high"
|
|||
include_apply_patch_tool: false,
|
||||
web_search_mode: Constrained::allow_any(WebSearchMode::Cached),
|
||||
use_experimental_unified_exec_tool: !cfg!(windows),
|
||||
background_terminal_max_timeout: DEFAULT_MAX_BACKGROUND_TERMINAL_TIMEOUT_MS,
|
||||
ghost_snapshot: GhostSnapshotConfig::default(),
|
||||
features: Features::with_defaults(),
|
||||
suppress_unstable_features_warning: false,
|
||||
|
|
|
|||
|
|
@ -54,6 +54,7 @@ pub(crate) const MIN_YIELD_TIME_MS: u64 = 250;
|
|||
// Minimum yield time for an empty `write_stdin`.
|
||||
pub(crate) const MIN_EMPTY_YIELD_TIME_MS: u64 = 5_000;
|
||||
pub(crate) const MAX_YIELD_TIME_MS: u64 = 30_000;
|
||||
pub(crate) const DEFAULT_MAX_BACKGROUND_TERMINAL_TIMEOUT_MS: u64 = 300_000;
|
||||
pub(crate) const DEFAULT_MAX_OUTPUT_TOKENS: usize = 10_000;
|
||||
pub(crate) const UNIFIED_EXEC_OUTPUT_MAX_BYTES: usize = 1024 * 1024; // 1 MiB
|
||||
pub(crate) const UNIFIED_EXEC_OUTPUT_MAX_TOKENS: usize = UNIFIED_EXEC_OUTPUT_MAX_BYTES / 4;
|
||||
|
|
@ -129,13 +130,22 @@ impl ProcessStore {
|
|||
|
||||
pub(crate) struct UnifiedExecProcessManager {
|
||||
process_store: Mutex<ProcessStore>,
|
||||
max_write_stdin_yield_time_ms: u64,
|
||||
}
|
||||
|
||||
impl UnifiedExecProcessManager {
|
||||
pub(crate) fn new(max_write_stdin_yield_time_ms: u64) -> Self {
|
||||
Self {
|
||||
process_store: Mutex::new(ProcessStore::default()),
|
||||
max_write_stdin_yield_time_ms: max_write_stdin_yield_time_ms
|
||||
.max(MIN_EMPTY_YIELD_TIME_MS),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Default for UnifiedExecProcessManager {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
process_store: Mutex::new(ProcessStore::default()),
|
||||
}
|
||||
Self::new(DEFAULT_MAX_BACKGROUND_TERMINAL_TIMEOUT_MS)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -33,6 +33,7 @@ use crate::unified_exec::ExecCommandRequest;
|
|||
use crate::unified_exec::MAX_UNIFIED_EXEC_PROCESSES;
|
||||
use crate::unified_exec::MAX_YIELD_TIME_MS;
|
||||
use crate::unified_exec::MIN_EMPTY_YIELD_TIME_MS;
|
||||
use crate::unified_exec::MIN_YIELD_TIME_MS;
|
||||
use crate::unified_exec::ProcessEntry;
|
||||
use crate::unified_exec::ProcessStore;
|
||||
use crate::unified_exec::UnifiedExecContext;
|
||||
|
|
@ -351,11 +352,13 @@ impl UnifiedExecProcessManager {
|
|||
|
||||
let max_tokens = resolve_max_tokens(request.max_output_tokens);
|
||||
let yield_time_ms = {
|
||||
let time_ms = clamp_yield_time(request.yield_time_ms);
|
||||
// Empty polls use configurable background timeout bounds. Non-empty
|
||||
// writes keep a fixed max cap so interactive stdin remains responsive.
|
||||
let time_ms = request.yield_time_ms.max(MIN_YIELD_TIME_MS);
|
||||
if request.input.is_empty() {
|
||||
time_ms.clamp(MIN_EMPTY_YIELD_TIME_MS, MAX_YIELD_TIME_MS)
|
||||
time_ms.clamp(MIN_EMPTY_YIELD_TIME_MS, self.max_write_stdin_yield_time_ms)
|
||||
} else {
|
||||
time_ms
|
||||
time_ms.min(MAX_YIELD_TIME_MS)
|
||||
}
|
||||
};
|
||||
let start = Instant::now();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue