mirror of
https://github.com/openai/codex.git
synced 2026-05-29 15:30:22 +00:00
collab-stack: import collab stack with local repairs
Start from the pinned `origin/dev/friel/collab-stack` snapshot and fold in the local follow-up repairs that made that imported stack usable here: watchdog spawn/registration plumbing, deferred-tool fallback behavior, collab discovery fixture alignment, schema mirroring, and subagent-panel fixes. Original imported source: - source ref: `refs/remotes/origin/dev/friel/collab-stack` - source tip: `599ed9dc05eafd116192184bd54a2a55a2c49366` - original base: `c1d18ceb6f22ae3acd67bbd6badad0f475b31dfc`
This commit is contained in:
@@ -16,6 +16,12 @@ pub struct Cli {
|
||||
#[command(subcommand)]
|
||||
pub command: Option<Command>,
|
||||
|
||||
/// Fork from an existing session id (or thread name) before sending the prompt.
|
||||
///
|
||||
/// This creates a new session with copied history, similar to `codex fork`.
|
||||
#[arg(long = "fork", value_name = "SESSION_ID")]
|
||||
pub fork_session_id: Option<String>,
|
||||
|
||||
#[clap(flatten)]
|
||||
pub shared: ExecSharedCliOptions,
|
||||
|
||||
@@ -156,7 +162,18 @@ fn mark_exec_global_args(cmd: clap::Command) -> clap::Command {
|
||||
arg.global(true)
|
||||
})
|
||||
}
|
||||
impl Cli {
|
||||
pub fn validate(self) -> Result<Self, clap::Error> {
|
||||
if self.fork_session_id.is_some() && self.command.is_some() {
|
||||
return Err(clap::Error::raw(
|
||||
clap::error::ErrorKind::ArgumentConflict,
|
||||
"--fork cannot be used with subcommands",
|
||||
));
|
||||
}
|
||||
|
||||
Ok(self)
|
||||
}
|
||||
}
|
||||
#[derive(Debug, clap::Subcommand)]
|
||||
pub enum Command {
|
||||
/// Resume a previous session by id or pick the most recent with --last.
|
||||
|
||||
@@ -80,3 +80,22 @@ fn removed_full_auto_flag_reports_migration_path() {
|
||||
Some("warning: `--full-auto` is deprecated; use `--sandbox workspace-write` instead.")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fork_option_parses_prompt() {
|
||||
const PROMPT: &str = "echo fork-non-interactive";
|
||||
let cli = Cli::parse_from(["codex-exec", "--fork", "session-123", "--json", PROMPT]);
|
||||
|
||||
assert_eq!(cli.fork_session_id.as_deref(), Some("session-123"));
|
||||
assert_eq!(cli.prompt.as_deref(), Some(PROMPT));
|
||||
assert!(cli.command.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fork_option_conflicts_with_subcommands() {
|
||||
let err = Cli::try_parse_from(["codex-exec", "--fork", "session-123", "resume"])
|
||||
.and_then(Cli::validate)
|
||||
.expect_err("fork should conflict with subcommands");
|
||||
|
||||
assert_eq!(err.kind(), clap::error::ErrorKind::ArgumentConflict);
|
||||
}
|
||||
|
||||
@@ -32,6 +32,8 @@ use codex_app_server_protocol::ReviewTarget as ApiReviewTarget;
|
||||
use codex_app_server_protocol::ServerNotification;
|
||||
use codex_app_server_protocol::ServerRequest;
|
||||
use codex_app_server_protocol::Thread as AppServerThread;
|
||||
use codex_app_server_protocol::ThreadForkParams;
|
||||
use codex_app_server_protocol::ThreadForkResponse;
|
||||
use codex_app_server_protocol::ThreadItem as AppServerThreadItem;
|
||||
use codex_app_server_protocol::ThreadListParams;
|
||||
use codex_app_server_protocol::ThreadListResponse;
|
||||
@@ -195,6 +197,7 @@ struct ExecRunArgs {
|
||||
config: Config,
|
||||
dangerously_bypass_approvals_and_sandbox: bool,
|
||||
exec_span: tracing::Span,
|
||||
fork_session_id: Option<String>,
|
||||
images: Vec<PathBuf>,
|
||||
json_mode: bool,
|
||||
last_message_file: Option<PathBuf>,
|
||||
@@ -227,6 +230,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
|
||||
|
||||
let Cli {
|
||||
command,
|
||||
fork_session_id,
|
||||
shared,
|
||||
skip_git_repo_check,
|
||||
ephemeral,
|
||||
@@ -525,6 +529,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
|
||||
config,
|
||||
dangerously_bypass_approvals_and_sandbox,
|
||||
exec_span: exec_span.clone(),
|
||||
fork_session_id,
|
||||
images,
|
||||
json_mode,
|
||||
last_message_file,
|
||||
@@ -546,6 +551,7 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> {
|
||||
config,
|
||||
dangerously_bypass_approvals_and_sandbox,
|
||||
exec_span,
|
||||
fork_session_id,
|
||||
images,
|
||||
json_mode,
|
||||
last_message_file,
|
||||
@@ -663,10 +669,10 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> {
|
||||
anyhow::anyhow!("failed to initialize in-process app-server client: {err}")
|
||||
})?;
|
||||
|
||||
// Handle resume subcommand through existing `thread/list` + `thread/resume`
|
||||
// APIs so exec no longer reaches into rollout storage directly.
|
||||
let (primary_thread_id, fallback_session_configured) =
|
||||
if let Some(ExecCommand::Resume(args)) = command.as_ref() {
|
||||
// Handle resume/fork/start through app-server APIs so exec no longer reaches into
|
||||
// rollout storage directly for normal bootstrap.
|
||||
let (primary_thread_id, fallback_session_configured) = match command.as_ref() {
|
||||
Some(ExecCommand::Resume(args)) => {
|
||||
if let Some(thread_id) = resolve_resume_thread_id(&client, &config, args).await? {
|
||||
let response: ThreadResumeResponse = send_request_with_response(
|
||||
&client,
|
||||
@@ -696,22 +702,41 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> {
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
(session_configured.session_id, session_configured)
|
||||
}
|
||||
} else {
|
||||
let response: ThreadStartResponse = send_request_with_response(
|
||||
&client,
|
||||
ClientRequest::ThreadStart {
|
||||
request_id: request_ids.next(),
|
||||
params: thread_start_params_from_config(&config),
|
||||
},
|
||||
"thread/start",
|
||||
)
|
||||
.await
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
let session_configured = session_configured_from_thread_start_response(&response)
|
||||
}
|
||||
Some(ExecCommand::Review(_)) | None => {
|
||||
if let Some(session_id) = fork_session_id.as_deref() {
|
||||
let response: ThreadForkResponse = send_request_with_response(
|
||||
&client,
|
||||
ClientRequest::ThreadFork {
|
||||
request_id: request_ids.next(),
|
||||
params: thread_fork_params_from_config(
|
||||
&config, session_id, /*path*/ None,
|
||||
),
|
||||
},
|
||||
"thread/fork",
|
||||
)
|
||||
.await
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
(session_configured.session_id, session_configured)
|
||||
};
|
||||
|
||||
let session_configured = session_configured_from_thread_fork_response(&response)
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
(session_configured.session_id, session_configured)
|
||||
} else {
|
||||
let response: ThreadStartResponse = send_request_with_response(
|
||||
&client,
|
||||
ClientRequest::ThreadStart {
|
||||
request_id: request_ids.next(),
|
||||
params: thread_start_params_from_config(&config),
|
||||
},
|
||||
"thread/start",
|
||||
)
|
||||
.await
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
let session_configured = session_configured_from_thread_start_response(&response)
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
(session_configured.session_id, session_configured)
|
||||
}
|
||||
}
|
||||
};
|
||||
let primary_thread_id_for_span = primary_thread_id.to_string();
|
||||
// Use the start/resume response as the authoritative bootstrap payload.
|
||||
// Waiting for a later streamed `SessionConfigured` event adds up to 10s of
|
||||
@@ -958,6 +983,26 @@ fn approvals_reviewer_override_from_config(
|
||||
Some(config.approvals_reviewer.into())
|
||||
}
|
||||
|
||||
fn thread_fork_params_from_config(
|
||||
config: &Config,
|
||||
thread_id: &str,
|
||||
path: Option<PathBuf>,
|
||||
) -> ThreadForkParams {
|
||||
ThreadForkParams {
|
||||
thread_id: thread_id.to_string(),
|
||||
path,
|
||||
model: config.model.clone(),
|
||||
model_provider: Some(config.model_provider_id.clone()),
|
||||
cwd: Some(config.cwd.to_string_lossy().to_string()),
|
||||
approval_policy: Some(config.permissions.approval_policy.value().into()),
|
||||
approvals_reviewer: approvals_reviewer_override_from_config(config),
|
||||
sandbox: None,
|
||||
permission_profile: Some(config.permissions.permission_profile().into()),
|
||||
config: config_request_overrides_from_config(config),
|
||||
..ThreadForkParams::default()
|
||||
}
|
||||
}
|
||||
|
||||
async fn send_request_with_response<T>(
|
||||
client: &InProcessAppServerClient,
|
||||
request: ClientRequest,
|
||||
@@ -1013,6 +1058,25 @@ fn session_configured_from_thread_resume_response(
|
||||
)
|
||||
}
|
||||
|
||||
fn session_configured_from_thread_fork_response(
|
||||
response: &ThreadForkResponse,
|
||||
) -> Result<SessionConfiguredEvent, String> {
|
||||
session_configured_from_thread_response(
|
||||
&response.thread.id,
|
||||
response.thread.name.clone(),
|
||||
response.thread.path.clone(),
|
||||
response.model.clone(),
|
||||
response.model_provider.clone(),
|
||||
response.service_tier,
|
||||
response.approval_policy.to_core(),
|
||||
response.approvals_reviewer.to_core(),
|
||||
response.sandbox.to_core(),
|
||||
response.permission_profile.clone().map(Into::into),
|
||||
response.cwd.clone(),
|
||||
response.reasoning_effort,
|
||||
)
|
||||
}
|
||||
|
||||
fn review_target_to_api(target: ReviewTarget) -> ApiReviewTarget {
|
||||
match target {
|
||||
ReviewTarget::UncommittedChanges => ApiReviewTarget::UncommittedChanges,
|
||||
|
||||
@@ -29,7 +29,10 @@ fn main() -> anyhow::Result<()> {
|
||||
arg0_dispatch_or_else(|arg0_paths: Arg0DispatchPaths| async move {
|
||||
let top_cli = TopCli::parse();
|
||||
// Merge root-level overrides into inner CLI struct so downstream logic remains unchanged.
|
||||
let mut inner = top_cli.inner;
|
||||
let mut inner = match top_cli.inner.validate() {
|
||||
Ok(inner) => inner,
|
||||
Err(err) => err.exit(),
|
||||
};
|
||||
inner
|
||||
.config_overrides
|
||||
.raw_overrides
|
||||
|
||||
@@ -35,3 +35,24 @@ fn top_cli_parses_resume_prompt_after_config_flag() {
|
||||
"reasoning_level=xhigh"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn top_cli_parses_fork_option_with_root_config() {
|
||||
let cli = TopCli::parse_from([
|
||||
"codex-exec",
|
||||
"--config",
|
||||
"reasoning_level=xhigh",
|
||||
"--fork",
|
||||
"session-123",
|
||||
"echo fork",
|
||||
]);
|
||||
|
||||
assert_eq!(cli.inner.fork_session_id.as_deref(), Some("session-123"));
|
||||
assert!(cli.inner.command.is_none());
|
||||
assert_eq!(cli.inner.prompt.as_deref(), Some("echo fork"));
|
||||
assert_eq!(cli.config_overrides.raw_overrides.len(), 1);
|
||||
assert_eq!(
|
||||
cli.config_overrides.raw_overrides[0],
|
||||
"reasoning_level=xhigh"
|
||||
);
|
||||
}
|
||||
|
||||
162
codex-rs/exec/tests/suite/fork.rs
Normal file
162
codex-rs/exec/tests/suite/fork.rs
Normal file
@@ -0,0 +1,162 @@
|
||||
#![allow(clippy::unwrap_used, clippy::expect_used)]
|
||||
|
||||
use anyhow::Context;
|
||||
use codex_utils_cargo_bin::find_resource;
|
||||
use core_test_support::test_codex_exec::test_codex_exec;
|
||||
use serde_json::Value;
|
||||
use std::string::ToString;
|
||||
use uuid::Uuid;
|
||||
use walkdir::WalkDir;
|
||||
|
||||
/// Utility: scan the sessions dir for a rollout file that contains `marker`
|
||||
/// in any response_item.message.content entry. Returns the absolute path.
|
||||
fn find_session_file_containing_marker(
|
||||
sessions_dir: &std::path::Path,
|
||||
marker: &str,
|
||||
) -> Option<std::path::PathBuf> {
|
||||
for entry in WalkDir::new(sessions_dir) {
|
||||
let entry = match entry {
|
||||
Ok(e) => e,
|
||||
Err(_) => continue,
|
||||
};
|
||||
if !entry.file_type().is_file() {
|
||||
continue;
|
||||
}
|
||||
if !entry.file_name().to_string_lossy().ends_with(".jsonl") {
|
||||
continue;
|
||||
}
|
||||
let path = entry.path();
|
||||
let Ok(content) = std::fs::read_to_string(path) else {
|
||||
continue;
|
||||
};
|
||||
// Skip the first meta line and scan remaining JSONL entries.
|
||||
let mut lines = content.lines();
|
||||
if lines.next().is_none() {
|
||||
continue;
|
||||
}
|
||||
for line in lines {
|
||||
if line.trim().is_empty() {
|
||||
continue;
|
||||
}
|
||||
let Ok(item): Result<Value, _> = serde_json::from_str(line) else {
|
||||
continue;
|
||||
};
|
||||
if item.get("type").and_then(|t| t.as_str()) == Some("response_item")
|
||||
&& let Some(payload) = item.get("payload")
|
||||
&& payload.get("type").and_then(|t| t.as_str()) == Some("message")
|
||||
&& payload
|
||||
.get("content")
|
||||
.map(ToString::to_string)
|
||||
.unwrap_or_default()
|
||||
.contains(marker)
|
||||
{
|
||||
return Some(path.to_path_buf());
|
||||
}
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// Extract the conversation UUID from the first SessionMeta line in the rollout file.
|
||||
fn extract_conversation_id(path: &std::path::Path) -> String {
|
||||
let content = std::fs::read_to_string(path).unwrap();
|
||||
let mut lines = content.lines();
|
||||
let meta_line = lines.next().expect("missing meta line");
|
||||
let meta: Value = serde_json::from_str(meta_line).expect("invalid meta json");
|
||||
meta.get("payload")
|
||||
.and_then(|p| p.get("id"))
|
||||
.and_then(|v| v.as_str())
|
||||
.unwrap_or_default()
|
||||
.to_string()
|
||||
}
|
||||
|
||||
fn extract_forked_from_id(path: &std::path::Path) -> Option<String> {
|
||||
let content = std::fs::read_to_string(path).unwrap();
|
||||
let mut lines = content.lines();
|
||||
let meta_line = lines.next().expect("missing meta line");
|
||||
let meta: Value = serde_json::from_str(meta_line).expect("invalid meta json");
|
||||
meta.get("payload")
|
||||
.and_then(|payload| payload.get("forked_from_id"))
|
||||
.and_then(Value::as_str)
|
||||
.map(ToString::to_string)
|
||||
}
|
||||
|
||||
fn rollout_contains_fork_reference(path: &std::path::Path) -> bool {
|
||||
let Ok(content) = std::fs::read_to_string(path) else {
|
||||
return false;
|
||||
};
|
||||
content.lines().skip(1).any(|line| {
|
||||
serde_json::from_str::<Value>(line)
|
||||
.ok()
|
||||
.and_then(|item| item.get("type").and_then(Value::as_str).map(str::to_string))
|
||||
.as_deref()
|
||||
== Some("fork_reference")
|
||||
})
|
||||
}
|
||||
|
||||
fn exec_fixture() -> anyhow::Result<std::path::PathBuf> {
|
||||
Ok(find_resource!("tests/fixtures/cli_responses_fixture.sse")?)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_fork_by_id_creates_new_session_with_copied_history() -> anyhow::Result<()> {
|
||||
let test = test_codex_exec();
|
||||
let fixture = exec_fixture()?;
|
||||
|
||||
let marker = format!("fork-base-{}", Uuid::new_v4());
|
||||
let prompt = format!("echo {marker}");
|
||||
|
||||
test.cmd()
|
||||
.env("CODEX_RS_SSE_FIXTURE", &fixture)
|
||||
.env("OPENAI_BASE_URL", "http://unused.local")
|
||||
.arg("--skip-git-repo-check")
|
||||
.arg(&prompt)
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
let sessions_dir = test.home_path().join("sessions");
|
||||
let original_path = find_session_file_containing_marker(&sessions_dir, &marker)
|
||||
.context("no session file found after first run")?;
|
||||
let session_id = extract_conversation_id(&original_path);
|
||||
|
||||
let marker2 = format!("fork-follow-up-{}", Uuid::new_v4());
|
||||
let prompt2 = format!("echo {marker2}");
|
||||
|
||||
test.cmd()
|
||||
.env("CODEX_RS_SSE_FIXTURE", &fixture)
|
||||
.env("OPENAI_BASE_URL", "http://unused.local")
|
||||
.arg("--skip-git-repo-check")
|
||||
.arg("--fork")
|
||||
.arg(&session_id)
|
||||
.arg(&prompt2)
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
let forked_path = find_session_file_containing_marker(&sessions_dir, &marker2)
|
||||
.context("no forked session file found for second marker")?;
|
||||
|
||||
assert_ne!(
|
||||
forked_path, original_path,
|
||||
"fork should create a new session file"
|
||||
);
|
||||
|
||||
let forked_content = std::fs::read_to_string(&forked_path)?;
|
||||
assert_eq!(
|
||||
extract_forked_from_id(&forked_path).as_deref(),
|
||||
Some(session_id.as_str())
|
||||
);
|
||||
assert!(
|
||||
forked_content.contains(&marker) || rollout_contains_fork_reference(&forked_path),
|
||||
"forked rollout should either inline parent history or record a fork reference"
|
||||
);
|
||||
assert!(forked_content.contains(&marker2));
|
||||
|
||||
let original_content = std::fs::read_to_string(&original_path)?;
|
||||
assert!(original_content.contains(&marker));
|
||||
assert!(
|
||||
!original_content.contains(&marker2),
|
||||
"original session should not receive the forked prompt"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -3,6 +3,7 @@ mod add_dir;
|
||||
mod apply_patch;
|
||||
mod auth_env;
|
||||
mod ephemeral;
|
||||
mod fork;
|
||||
mod mcp_required_exit;
|
||||
mod originator;
|
||||
mod output_schema;
|
||||
|
||||
@@ -145,9 +145,9 @@ fn exec_resume_last_appends_to_existing_file() -> anyhow::Result<()> {
|
||||
.arg("--skip-git-repo-check")
|
||||
.arg("-C")
|
||||
.arg(&repo_root)
|
||||
.arg(&prompt2)
|
||||
.arg("resume")
|
||||
.arg("--last")
|
||||
.arg(&prompt2)
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user