mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
Merge remote-tracking branch 'origin/jif/search-term' into jif/search-term
This commit is contained in:
61
.github/workflows/bazel.yml
vendored
61
.github/workflows/bazel.yml
vendored
@@ -107,6 +107,45 @@ jobs:
|
||||
BUILDBUDDY_API_KEY: ${{ secrets.BUILDBUDDY_API_KEY }}
|
||||
shell: bash
|
||||
run: |
|
||||
set -o pipefail
|
||||
|
||||
bazel_console_log="$(mktemp)"
|
||||
|
||||
print_failed_bazel_test_logs() {
|
||||
local console_log="$1"
|
||||
local testlogs_dir
|
||||
|
||||
testlogs_dir="$(bazel $BAZEL_STARTUP_ARGS info bazel-testlogs 2>/dev/null || echo bazel-testlogs)"
|
||||
|
||||
local failed_targets=()
|
||||
while IFS= read -r target; do
|
||||
failed_targets+=("$target")
|
||||
done < <(
|
||||
grep -E '^FAIL: //' "$console_log" \
|
||||
| sed -E 's#^FAIL: (//[^ ]+).*#\1#' \
|
||||
| sort -u
|
||||
)
|
||||
|
||||
if [[ ${#failed_targets[@]} -eq 0 ]]; then
|
||||
echo "No failed Bazel test targets were found in console output."
|
||||
return
|
||||
fi
|
||||
|
||||
for target in "${failed_targets[@]}"; do
|
||||
local rel_path="${target#//}"
|
||||
rel_path="${rel_path/:/\/}"
|
||||
local test_log="${testlogs_dir}/${rel_path}/test.log"
|
||||
|
||||
echo "::group::Bazel test log tail for ${target}"
|
||||
if [[ -f "$test_log" ]]; then
|
||||
tail -n 200 "$test_log"
|
||||
else
|
||||
echo "Missing test log: $test_log"
|
||||
fi
|
||||
echo "::endgroup::"
|
||||
done
|
||||
}
|
||||
|
||||
bazel_args=(
|
||||
test
|
||||
//...
|
||||
@@ -119,10 +158,19 @@ jobs:
|
||||
|
||||
if [[ -n "${BUILDBUDDY_API_KEY:-}" ]]; then
|
||||
echo "BuildBuddy API key is available; using remote Bazel configuration."
|
||||
# Work around Bazel 9 remote repo contents cache / overlay materialization failures
|
||||
# seen in CI (for example "is not a symlink" or permission errors while
|
||||
# materializing external repos such as rules_perl). We still use BuildBuddy for
|
||||
# remote execution/cache; this only disables the startup-level repo contents cache.
|
||||
set +e
|
||||
bazel $BAZEL_STARTUP_ARGS \
|
||||
--noexperimental_remote_repo_contents_cache \
|
||||
--bazelrc=.github/workflows/ci.bazelrc \
|
||||
"${bazel_args[@]}" \
|
||||
"--remote_header=x-buildbuddy-api-key=$BUILDBUDDY_API_KEY"
|
||||
"--remote_header=x-buildbuddy-api-key=$BUILDBUDDY_API_KEY" \
|
||||
2>&1 | tee "$bazel_console_log"
|
||||
bazel_status=${PIPESTATUS[0]}
|
||||
set -e
|
||||
else
|
||||
echo "BuildBuddy API key is not available; using local Bazel configuration."
|
||||
# Keep fork/community PRs on Bazel but disable remote services that are
|
||||
@@ -141,9 +189,18 @@ jobs:
|
||||
# clear remote cache/execution endpoints configured in .bazelrc.
|
||||
# https://bazel.build/reference/command-line-reference#common_options-flag--remote_cache
|
||||
# https://bazel.build/reference/command-line-reference#common_options-flag--remote_executor
|
||||
set +e
|
||||
bazel $BAZEL_STARTUP_ARGS \
|
||||
--noexperimental_remote_repo_contents_cache \
|
||||
"${bazel_args[@]}" \
|
||||
--remote_cache= \
|
||||
--remote_executor=
|
||||
--remote_executor= \
|
||||
2>&1 | tee "$bazel_console_log"
|
||||
bazel_status=${PIPESTATUS[0]}
|
||||
set -e
|
||||
fi
|
||||
|
||||
if [[ ${bazel_status:-0} -ne 0 ]]; then
|
||||
print_failed_bazel_test_logs "$bazel_console_log"
|
||||
exit "$bazel_status"
|
||||
fi
|
||||
|
||||
1
codex-rs/Cargo.lock
generated
1
codex-rs/Cargo.lock
generated
@@ -2485,6 +2485,7 @@ name = "codex-utils-string"
|
||||
version = "0.0.0"
|
||||
dependencies = [
|
||||
"pretty_assertions",
|
||||
"regex-lite",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
@@ -1112,6 +1112,34 @@ mod tests {
|
||||
assert_eq!(args.prompt.as_deref(), Some("2+2"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_resume_accepts_output_last_message_flag_after_subcommand() {
|
||||
let cli = MultitoolCli::try_parse_from([
|
||||
"codex",
|
||||
"exec",
|
||||
"resume",
|
||||
"session-123",
|
||||
"-o",
|
||||
"/tmp/resume-output.md",
|
||||
"re-review",
|
||||
])
|
||||
.expect("parse should succeed");
|
||||
|
||||
let Some(Subcommand::Exec(exec)) = cli.subcommand else {
|
||||
panic!("expected exec subcommand");
|
||||
};
|
||||
let Some(codex_exec::Command::Resume(args)) = exec.command else {
|
||||
panic!("expected exec resume");
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
exec.last_message_file,
|
||||
Some(std::path::PathBuf::from("/tmp/resume-output.md"))
|
||||
);
|
||||
assert_eq!(args.session_id.as_deref(), Some("session-123"));
|
||||
assert_eq!(args.prompt.as_deref(), Some("re-review"));
|
||||
}
|
||||
|
||||
fn app_server_from_args(args: &[&str]) -> AppServerCommand {
|
||||
let cli = MultitoolCli::try_parse_from(args).expect("parse");
|
||||
let Subcommand::AppServer(app_server) = cli.subcommand.expect("app-server present") else {
|
||||
|
||||
@@ -49,10 +49,12 @@ async fn features_enable_under_development_feature_prints_warning() -> Result<()
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
let mut cmd = codex_command(codex_home.path())?;
|
||||
cmd.args(["features", "enable", "sqlite"])
|
||||
cmd.args(["features", "enable", "runtime_metrics"])
|
||||
.assert()
|
||||
.success()
|
||||
.stderr(contains("Under-development features enabled: sqlite."));
|
||||
.stderr(contains(
|
||||
"Under-development features enabled: runtime_metrics.",
|
||||
));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -349,6 +349,9 @@
|
||||
"js_repl_tools_only": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"memories": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"memory_tool": {
|
||||
"type": "boolean"
|
||||
},
|
||||
@@ -1627,6 +1630,9 @@
|
||||
"js_repl_tools_only": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"memories": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"memory_tool": {
|
||||
"type": "boolean"
|
||||
},
|
||||
|
||||
@@ -118,6 +118,9 @@ impl Guards {
|
||||
} else {
|
||||
active_agents.used_agent_nicknames.clear();
|
||||
active_agents.nickname_reset_count += 1;
|
||||
if let Some(metrics) = codex_otel::metrics::global() {
|
||||
let _ = metrics.counter("codex.multi_agent.nickname_pool_reset", 1, &[]);
|
||||
}
|
||||
names.choose(&mut rand::rng())?.to_string()
|
||||
}
|
||||
};
|
||||
|
||||
@@ -13,7 +13,7 @@ use std::path::Path;
|
||||
use std::sync::LazyLock;
|
||||
use toml::Value as TomlValue;
|
||||
|
||||
const DEFAULT_ROLE_NAME: &str = "default";
|
||||
pub const DEFAULT_ROLE_NAME: &str = "default";
|
||||
const AGENT_TYPE_UNAVAILABLE_ERROR: &str = "agent type is currently not available";
|
||||
|
||||
/// Applies a role config layer to a mutable config and preserves unspecified keys.
|
||||
|
||||
@@ -494,12 +494,12 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
FeatureSpec {
|
||||
id: Feature::Sqlite,
|
||||
key: "sqlite",
|
||||
stage: Stage::UnderDevelopment,
|
||||
default_enabled: false,
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::MemoryTool,
|
||||
key: "memory_tool",
|
||||
key: "memories",
|
||||
stage: Stage::UnderDevelopment,
|
||||
default_enabled: false,
|
||||
},
|
||||
|
||||
@@ -37,6 +37,10 @@ const ALIASES: &[Alias] = &[
|
||||
legacy_key: "collab",
|
||||
feature: Feature::Collab,
|
||||
},
|
||||
Alias {
|
||||
legacy_key: "memory_tool",
|
||||
feature: Feature::MemoryTool,
|
||||
},
|
||||
];
|
||||
|
||||
pub(crate) fn legacy_feature_keys() -> impl Iterator<Item = &'static str> {
|
||||
|
||||
@@ -93,6 +93,7 @@ impl ToolHandler for MultiAgentHandler {
|
||||
|
||||
mod spawn {
|
||||
use super::*;
|
||||
use crate::agent::role::DEFAULT_ROLE_NAME;
|
||||
use crate::agent::role::apply_role_to_config;
|
||||
|
||||
use crate::agent::exceeds_thread_spawn_depth_limit;
|
||||
@@ -109,6 +110,7 @@ mod spawn {
|
||||
#[derive(Debug, Serialize)]
|
||||
struct SpawnAgentResult {
|
||||
agent_id: String,
|
||||
nickname: Option<String>,
|
||||
}
|
||||
|
||||
pub async fn handle(
|
||||
@@ -183,6 +185,7 @@ mod spawn {
|
||||
.unwrap_or((None, None)),
|
||||
None => (None, None),
|
||||
};
|
||||
let nickname = new_agent_nickname.clone();
|
||||
session
|
||||
.send_event(
|
||||
&turn,
|
||||
@@ -199,9 +202,13 @@ mod spawn {
|
||||
)
|
||||
.await;
|
||||
let new_thread_id = result?;
|
||||
let role_tag = role_name.unwrap_or(DEFAULT_ROLE_NAME);
|
||||
turn.otel_manager
|
||||
.counter("codex.multi_agent.spawn", 1, &[("role", role_tag)]);
|
||||
|
||||
let content = serde_json::to_string(&SpawnAgentResult {
|
||||
agent_id: new_thread_id.to_string(),
|
||||
nickname,
|
||||
})
|
||||
.map_err(|err| {
|
||||
FunctionCallError::Fatal(format!("failed to serialize spawn_agent result: {err}"))
|
||||
@@ -406,6 +413,8 @@ mod resume_agent {
|
||||
if let Some(err) = error {
|
||||
return Err(err);
|
||||
}
|
||||
turn.otel_manager
|
||||
.counter("codex.multi_agent.resume", 1, &[]);
|
||||
|
||||
let content = serde_json::to_string(&ResumeAgentResult { status }).map_err(|err| {
|
||||
FunctionCallError::Fatal(format!("failed to serialize resume_agent result: {err}"))
|
||||
@@ -1085,6 +1094,7 @@ mod tests {
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct SpawnAgentResult {
|
||||
agent_id: String,
|
||||
nickname: Option<String>,
|
||||
}
|
||||
|
||||
let (mut session, mut turn) = make_session_and_context().await;
|
||||
@@ -1121,6 +1131,12 @@ mod tests {
|
||||
let result: SpawnAgentResult =
|
||||
serde_json::from_str(&content).expect("spawn_agent result should be json");
|
||||
let agent_id = agent_id(&result.agent_id).expect("agent_id should be valid");
|
||||
assert!(
|
||||
result
|
||||
.nickname
|
||||
.as_deref()
|
||||
.is_some_and(|nickname| !nickname.is_empty())
|
||||
);
|
||||
let snapshot = manager
|
||||
.get_thread(agent_id)
|
||||
.await
|
||||
@@ -1184,6 +1200,7 @@ mod tests {
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct SpawnAgentResult {
|
||||
agent_id: String,
|
||||
nickname: Option<String>,
|
||||
}
|
||||
|
||||
let (mut session, mut turn) = make_session_and_context().await;
|
||||
@@ -1221,6 +1238,12 @@ mod tests {
|
||||
let result: SpawnAgentResult =
|
||||
serde_json::from_str(&content).expect("spawn_agent result should be json");
|
||||
assert!(!result.agent_id.is_empty());
|
||||
assert!(
|
||||
result
|
||||
.nickname
|
||||
.as_deref()
|
||||
.is_some_and(|nickname| !nickname.is_empty())
|
||||
);
|
||||
assert_eq!(success, Some(true));
|
||||
}
|
||||
|
||||
|
||||
@@ -557,7 +557,7 @@ fn create_spawn_agent_tool(config: &ToolsConfig) -> ToolSpec {
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
name: "spawn_agent".to_string(),
|
||||
description:
|
||||
"Spawn a sub-agent for a well-scoped task. Returns the agent id to use to communicate with this agent."
|
||||
"Spawn a sub-agent for a well-scoped task. Returns the agent id (and user-facing nickname when available) to use to communicate with this agent."
|
||||
.to_string(),
|
||||
strict: false,
|
||||
parameters: JsonSchema::Object {
|
||||
|
||||
@@ -96,7 +96,12 @@ pub struct Cli {
|
||||
pub json: bool,
|
||||
|
||||
/// Specifies file where the last message from the agent should be written.
|
||||
#[arg(long = "output-last-message", short = 'o', value_name = "FILE")]
|
||||
#[arg(
|
||||
long = "output-last-message",
|
||||
short = 'o',
|
||||
value_name = "FILE",
|
||||
global = true
|
||||
)]
|
||||
pub last_message_file: Option<PathBuf>,
|
||||
|
||||
/// Initial instructions for the agent. If not provided as an argument (or
|
||||
@@ -283,4 +288,27 @@ mod tests {
|
||||
});
|
||||
assert_eq!(effective_prompt.as_deref(), Some(PROMPT));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resume_accepts_output_last_message_flag_after_subcommand() {
|
||||
const PROMPT: &str = "echo resume-with-output-file";
|
||||
let cli = Cli::parse_from([
|
||||
"codex-exec",
|
||||
"resume",
|
||||
"session-123",
|
||||
"-o",
|
||||
"/tmp/resume-output.md",
|
||||
PROMPT,
|
||||
]);
|
||||
|
||||
assert_eq!(
|
||||
cli.last_message_file,
|
||||
Some(PathBuf::from("/tmp/resume-output.md"))
|
||||
);
|
||||
let Some(Command::Resume(args)) = cli.command else {
|
||||
panic!("expected resume command");
|
||||
};
|
||||
assert_eq!(args.session_id.as_deref(), Some("session-123"));
|
||||
assert_eq!(args.prompt.as_deref(), Some(PROMPT));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,11 +4,7 @@ use codex_utils_cargo_bin::find_resource;
|
||||
use core_test_support::test_codex_exec::test_codex_exec;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use std::fs::FileTimes;
|
||||
use std::fs::OpenOptions;
|
||||
use std::string::ToString;
|
||||
use std::time::Duration;
|
||||
use std::time::SystemTime;
|
||||
use tempfile::TempDir;
|
||||
use uuid::Uuid;
|
||||
use walkdir::WalkDir;
|
||||
@@ -257,21 +253,26 @@ fn exec_resume_last_respects_cwd_filter_and_all_flag() -> anyhow::Result<()> {
|
||||
.success();
|
||||
|
||||
let sessions_dir = test.home_path().join("sessions");
|
||||
let path_a = find_session_file_containing_marker(&sessions_dir, &marker_a)
|
||||
find_session_file_containing_marker(&sessions_dir, &marker_a)
|
||||
.expect("no session file found for marker_a");
|
||||
let path_b = find_session_file_containing_marker(&sessions_dir, &marker_b)
|
||||
.expect("no session file found for marker_b");
|
||||
|
||||
// Files are ordered by `updated_at`, then by `uuid`.
|
||||
// We mutate the mtimes to ensure file_b is the newest file.
|
||||
let file_a = OpenOptions::new().write(true).open(&path_a)?;
|
||||
file_a.set_times(
|
||||
FileTimes::new().set_modified(SystemTime::UNIX_EPOCH + Duration::from_secs(1)),
|
||||
)?;
|
||||
let file_b = OpenOptions::new().write(true).open(&path_b)?;
|
||||
file_b.set_times(
|
||||
FileTimes::new().set_modified(SystemTime::UNIX_EPOCH + Duration::from_secs(2)),
|
||||
)?;
|
||||
// Make thread B deterministically newest according to rollout metadata.
|
||||
let session_id_b = extract_conversation_id(&path_b);
|
||||
let marker_b_touch = format!("resume-cwd-b-touch-{}", Uuid::new_v4());
|
||||
let prompt_b_touch = format!("echo {marker_b_touch}");
|
||||
test.cmd()
|
||||
.env("CODEX_RS_SSE_FIXTURE", &fixture)
|
||||
.env("OPENAI_BASE_URL", "http://unused.local")
|
||||
.arg("--skip-git-repo-check")
|
||||
.arg("-C")
|
||||
.arg(dir_b.path())
|
||||
.arg("resume")
|
||||
.arg(&session_id_b)
|
||||
.arg(&prompt_b_touch)
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
let marker_b2 = format!("resume-cwd-b-2-{}", Uuid::new_v4());
|
||||
let prompt_b2 = format!("echo {marker_b2}");
|
||||
@@ -312,8 +313,8 @@ fn exec_resume_last_respects_cwd_filter_and_all_flag() -> anyhow::Result<()> {
|
||||
let resumed_path_cwd = find_session_file_containing_marker(&sessions_dir, &marker_a2)
|
||||
.expect("no resumed session file containing marker_a2");
|
||||
assert_eq!(
|
||||
resumed_path_cwd, path_a,
|
||||
"resume --last should prefer sessions from the same cwd"
|
||||
resumed_path_cwd, path_b,
|
||||
"resume --last should prefer sessions whose latest turn context matches the current cwd"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
|
||||
@@ -7,5 +7,8 @@ license.workspace = true
|
||||
[lints]
|
||||
workspace = true
|
||||
|
||||
[dependencies]
|
||||
regex-lite = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
pretty_assertions = { workspace = true }
|
||||
|
||||
@@ -62,11 +62,54 @@ pub fn sanitize_metric_tag_value(value: &str) -> String {
|
||||
}
|
||||
}
|
||||
|
||||
/// Find all UUIDs in a string.
|
||||
#[allow(clippy::unwrap_used)]
|
||||
pub fn find_uuids(s: &str) -> Vec<String> {
|
||||
static RE: std::sync::OnceLock<regex_lite::Regex> = std::sync::OnceLock::new();
|
||||
let re = RE.get_or_init(|| {
|
||||
regex_lite::Regex::new(
|
||||
r"[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}",
|
||||
)
|
||||
.unwrap() // Unwrap is safe thanks to the tests.
|
||||
});
|
||||
|
||||
re.find_iter(s).map(|m| m.as_str().to_string()).collect()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::find_uuids;
|
||||
use super::sanitize_metric_tag_value;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn find_uuids_finds_multiple() {
|
||||
let input =
|
||||
"x 00112233-4455-6677-8899-aabbccddeeff-k y 12345678-90ab-cdef-0123-456789abcdef";
|
||||
assert_eq!(
|
||||
find_uuids(input),
|
||||
vec![
|
||||
"00112233-4455-6677-8899-aabbccddeeff".to_string(),
|
||||
"12345678-90ab-cdef-0123-456789abcdef".to_string(),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn find_uuids_ignores_invalid() {
|
||||
let input = "not-a-uuid-1234-5678-9abc-def0-123456789abc";
|
||||
assert_eq!(find_uuids(input), Vec::<String>::new());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn find_uuids_handles_non_ascii_without_overlap() {
|
||||
let input = "🙂 55e5d6f7-8a7f-4d2a-8d88-123456789012abc";
|
||||
assert_eq!(
|
||||
find_uuids(input),
|
||||
vec!["55e5d6f7-8a7f-4d2a-8d88-123456789012".to_string()]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sanitize_metric_tag_value_trims_and_fills_unspecified() {
|
||||
let msg = "///";
|
||||
|
||||
Reference in New Issue
Block a user