Update default yield time (#6610)
10s for exec and 250ms for write_stdin
This commit is contained in:
parent
6cda3de3a4
commit
0792a7953d
3 changed files with 36 additions and 39 deletions
|
|
@ -32,8 +32,8 @@ struct ExecCommandArgs {
|
|||
shell: String,
|
||||
#[serde(default = "default_login")]
|
||||
login: bool,
|
||||
#[serde(default)]
|
||||
yield_time_ms: Option<u64>,
|
||||
#[serde(default = "default_exec_yield_time_ms")]
|
||||
yield_time_ms: u64,
|
||||
#[serde(default)]
|
||||
max_output_tokens: Option<usize>,
|
||||
#[serde(default)]
|
||||
|
|
@ -47,12 +47,20 @@ struct WriteStdinArgs {
|
|||
session_id: i32,
|
||||
#[serde(default)]
|
||||
chars: String,
|
||||
#[serde(default)]
|
||||
yield_time_ms: Option<u64>,
|
||||
#[serde(default = "default_write_stdin_yield_time_ms")]
|
||||
yield_time_ms: u64,
|
||||
#[serde(default)]
|
||||
max_output_tokens: Option<usize>,
|
||||
}
|
||||
|
||||
fn default_exec_yield_time_ms() -> u64 {
|
||||
10000
|
||||
}
|
||||
|
||||
fn default_write_stdin_yield_time_ms() -> u64 {
|
||||
250
|
||||
}
|
||||
|
||||
fn default_shell() -> String {
|
||||
"/bin/bash".to_string()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -41,7 +41,6 @@ mod session_manager;
|
|||
pub(crate) use errors::UnifiedExecError;
|
||||
pub(crate) use session::UnifiedExecSession;
|
||||
|
||||
pub(crate) const DEFAULT_YIELD_TIME_MS: u64 = 10_000;
|
||||
pub(crate) const MIN_YIELD_TIME_MS: u64 = 250;
|
||||
pub(crate) const MAX_YIELD_TIME_MS: u64 = 30_000;
|
||||
pub(crate) const DEFAULT_MAX_OUTPUT_TOKENS: usize = 10_000;
|
||||
|
|
@ -68,7 +67,7 @@ pub(crate) struct ExecCommandRequest<'a> {
|
|||
pub command: &'a str,
|
||||
pub shell: &'a str,
|
||||
pub login: bool,
|
||||
pub yield_time_ms: Option<u64>,
|
||||
pub yield_time_ms: u64,
|
||||
pub max_output_tokens: Option<usize>,
|
||||
pub workdir: Option<PathBuf>,
|
||||
pub with_escalated_permissions: Option<bool>,
|
||||
|
|
@ -79,7 +78,7 @@ pub(crate) struct ExecCommandRequest<'a> {
|
|||
pub(crate) struct WriteStdinRequest<'a> {
|
||||
pub session_id: i32,
|
||||
pub input: &'a str,
|
||||
pub yield_time_ms: Option<u64>,
|
||||
pub yield_time_ms: u64,
|
||||
pub max_output_tokens: Option<usize>,
|
||||
}
|
||||
|
||||
|
|
@ -110,11 +109,8 @@ struct SessionEntry {
|
|||
started_at: tokio::time::Instant,
|
||||
}
|
||||
|
||||
pub(crate) fn clamp_yield_time(yield_time_ms: Option<u64>) -> u64 {
|
||||
match yield_time_ms {
|
||||
Some(value) => value.clamp(MIN_YIELD_TIME_MS, MAX_YIELD_TIME_MS),
|
||||
None => DEFAULT_YIELD_TIME_MS,
|
||||
}
|
||||
pub(crate) fn clamp_yield_time(yield_time_ms: u64) -> u64 {
|
||||
yield_time_ms.clamp(MIN_YIELD_TIME_MS, MAX_YIELD_TIME_MS)
|
||||
}
|
||||
|
||||
pub(crate) fn resolve_max_tokens(max_tokens: Option<usize>) -> usize {
|
||||
|
|
@ -187,7 +183,7 @@ mod tests {
|
|||
session: &Arc<Session>,
|
||||
turn: &Arc<TurnContext>,
|
||||
cmd: &str,
|
||||
yield_time_ms: Option<u64>,
|
||||
yield_time_ms: u64,
|
||||
) -> Result<UnifiedExecResponse, UnifiedExecError> {
|
||||
let context =
|
||||
UnifiedExecContext::new(Arc::clone(session), Arc::clone(turn), "call".to_string());
|
||||
|
|
@ -215,7 +211,7 @@ mod tests {
|
|||
session: &Arc<Session>,
|
||||
session_id: i32,
|
||||
input: &str,
|
||||
yield_time_ms: Option<u64>,
|
||||
yield_time_ms: u64,
|
||||
) -> Result<UnifiedExecResponse, UnifiedExecError> {
|
||||
session
|
||||
.services
|
||||
|
|
@ -253,14 +249,14 @@ mod tests {
|
|||
|
||||
let (session, turn) = test_session_and_turn();
|
||||
|
||||
let open_shell = exec_command(&session, &turn, "bash -i", Some(2_500)).await?;
|
||||
let open_shell = exec_command(&session, &turn, "bash -i", 2_500).await?;
|
||||
let session_id = open_shell.session_id.expect("expected session_id");
|
||||
|
||||
write_stdin(
|
||||
&session,
|
||||
session_id,
|
||||
"export CODEX_INTERACTIVE_SHELL_VAR=codex\n",
|
||||
Some(2_500),
|
||||
2_500,
|
||||
)
|
||||
.await?;
|
||||
|
||||
|
|
@ -268,7 +264,7 @@ mod tests {
|
|||
&session,
|
||||
session_id,
|
||||
"echo $CODEX_INTERACTIVE_SHELL_VAR\n",
|
||||
Some(2_500),
|
||||
2_500,
|
||||
)
|
||||
.await?;
|
||||
assert!(
|
||||
|
|
@ -285,24 +281,19 @@ mod tests {
|
|||
|
||||
let (session, turn) = test_session_and_turn();
|
||||
|
||||
let shell_a = exec_command(&session, &turn, "bash -i", Some(2_500)).await?;
|
||||
let shell_a = exec_command(&session, &turn, "bash -i", 2_500).await?;
|
||||
let session_a = shell_a.session_id.expect("expected session id");
|
||||
|
||||
write_stdin(
|
||||
&session,
|
||||
session_a,
|
||||
"export CODEX_INTERACTIVE_SHELL_VAR=codex\n",
|
||||
Some(2_500),
|
||||
2_500,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let out_2 = exec_command(
|
||||
&session,
|
||||
&turn,
|
||||
"echo $CODEX_INTERACTIVE_SHELL_VAR",
|
||||
Some(2_500),
|
||||
)
|
||||
.await?;
|
||||
let out_2 =
|
||||
exec_command(&session, &turn, "echo $CODEX_INTERACTIVE_SHELL_VAR", 2_500).await?;
|
||||
assert!(
|
||||
out_2.session_id.is_none(),
|
||||
"short command should not retain a session"
|
||||
|
|
@ -316,7 +307,7 @@ mod tests {
|
|||
&session,
|
||||
session_a,
|
||||
"echo $CODEX_INTERACTIVE_SHELL_VAR\n",
|
||||
Some(2_500),
|
||||
2_500,
|
||||
)
|
||||
.await?;
|
||||
assert!(
|
||||
|
|
@ -333,14 +324,14 @@ mod tests {
|
|||
|
||||
let (session, turn) = test_session_and_turn();
|
||||
|
||||
let open_shell = exec_command(&session, &turn, "bash -i", Some(2_500)).await?;
|
||||
let open_shell = exec_command(&session, &turn, "bash -i", 2_500).await?;
|
||||
let session_id = open_shell.session_id.expect("expected session id");
|
||||
|
||||
write_stdin(
|
||||
&session,
|
||||
session_id,
|
||||
"export CODEX_INTERACTIVE_SHELL_VAR=codex\n",
|
||||
Some(2_500),
|
||||
2_500,
|
||||
)
|
||||
.await?;
|
||||
|
||||
|
|
@ -348,7 +339,7 @@ mod tests {
|
|||
&session,
|
||||
session_id,
|
||||
"sleep 5 && echo $CODEX_INTERACTIVE_SHELL_VAR\n",
|
||||
Some(10),
|
||||
10,
|
||||
)
|
||||
.await?;
|
||||
assert!(
|
||||
|
|
@ -358,7 +349,7 @@ mod tests {
|
|||
|
||||
tokio::time::sleep(Duration::from_secs(7)).await;
|
||||
|
||||
let out_3 = write_stdin(&session, session_id, "", Some(100)).await?;
|
||||
let out_3 = write_stdin(&session, session_id, "", 100).await?;
|
||||
|
||||
assert!(
|
||||
out_3.output.contains("codex"),
|
||||
|
|
@ -373,7 +364,7 @@ mod tests {
|
|||
async fn requests_with_large_timeout_are_capped() -> anyhow::Result<()> {
|
||||
let (session, turn) = test_session_and_turn();
|
||||
|
||||
let result = exec_command(&session, &turn, "echo codex", Some(120_000)).await?;
|
||||
let result = exec_command(&session, &turn, "echo codex", 120_000).await?;
|
||||
|
||||
assert!(result.session_id.is_none());
|
||||
assert!(result.output.contains("codex"));
|
||||
|
|
@ -385,7 +376,7 @@ mod tests {
|
|||
#[ignore] // Ignored while we have a better way to test this.
|
||||
async fn completed_commands_do_not_persist_sessions() -> anyhow::Result<()> {
|
||||
let (session, turn) = test_session_and_turn();
|
||||
let result = exec_command(&session, &turn, "echo codex", Some(2_500)).await?;
|
||||
let result = exec_command(&session, &turn, "echo codex", 2_500).await?;
|
||||
|
||||
assert!(
|
||||
result.session_id.is_none(),
|
||||
|
|
@ -412,14 +403,14 @@ mod tests {
|
|||
|
||||
let (session, turn) = test_session_and_turn();
|
||||
|
||||
let open_shell = exec_command(&session, &turn, "bash -i", Some(2_500)).await?;
|
||||
let open_shell = exec_command(&session, &turn, "bash -i", 2_500).await?;
|
||||
let session_id = open_shell.session_id.expect("expected session id");
|
||||
|
||||
write_stdin(&session, session_id, "exit\n", Some(2_500)).await?;
|
||||
write_stdin(&session, session_id, "exit\n", 2_500).await?;
|
||||
|
||||
tokio::time::sleep(Duration::from_millis(200)).await;
|
||||
|
||||
let err = write_stdin(&session, session_id, "", Some(100))
|
||||
let err = write_stdin(&session, session_id, "", 100)
|
||||
.await
|
||||
.expect_err("expected unknown session error");
|
||||
|
||||
|
|
|
|||
|
|
@ -19,7 +19,6 @@ use crate::tools::runtimes::unified_exec::UnifiedExecRuntime;
|
|||
use crate::tools::sandboxing::ToolCtx;
|
||||
|
||||
use super::ExecCommandRequest;
|
||||
use super::MIN_YIELD_TIME_MS;
|
||||
use super::SessionEntry;
|
||||
use super::UnifiedExecContext;
|
||||
use super::UnifiedExecError;
|
||||
|
|
@ -61,8 +60,7 @@ impl UnifiedExecSessionManager {
|
|||
.await?;
|
||||
|
||||
let max_tokens = resolve_max_tokens(request.max_output_tokens);
|
||||
let yield_time_ms =
|
||||
clamp_yield_time(Some(request.yield_time_ms.unwrap_or(MIN_YIELD_TIME_MS)));
|
||||
let yield_time_ms = clamp_yield_time(request.yield_time_ms);
|
||||
|
||||
let start = Instant::now();
|
||||
let (output_buffer, output_notify) = session.output_handles();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue