mirror of
https://github.com/openai/codex.git
synced 2026-11-08 20:40:33 +00:00
Start cloud threads with the local GitHub project (#60007)
## What changed Pass the local project's GitHub origin to cloud prewarming so new cloud threads can provision a repository checkout. Preserve the attached local project when switching backends and resolve its origin for each thread start. Normalize supported GitHub origins to credential-free HTTPS URLs and select a repository-specific cloud working directory. Unsupported origins retain the blank environment behavior. Keep project metadata outside the app-server wire protocol, and fix HTTP Git URL parsing so query strings and fragments cannot be mistaken for authority text. ## Testing Add coverage for project routing, origin normalization and rejection, managed environment encoding, local project retention, and Git URL credential stripping. GitOrigin-RevId: 5859769965faf060461ccc1cbb3fd48f8e2f2155
This commit is contained in:
committed by
copyberry
parent
df769889c0
commit
7de8a0afc0
@@ -856,6 +856,9 @@ impl App {
|
||||
self.select_agent_thread(tui, app_server, root_thread_id)
|
||||
.await?;
|
||||
}
|
||||
if self.current_displayed_thread_id() == Some(root_thread_id) {
|
||||
self.remember_attached_host_project(app_server);
|
||||
}
|
||||
let read_only = self.chat_widget.is_external_writer_view();
|
||||
if !read_only {
|
||||
self.replay_agents_overview_requests(app_server, root_thread_id)
|
||||
|
||||
@@ -167,6 +167,7 @@ impl App {
|
||||
/*session_start_source*/ None,
|
||||
remote_cwd.as_deref(),
|
||||
selected_profile.as_ref(),
|
||||
self.backend_preference_context.local_cwd.as_deref(),
|
||||
)),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -76,6 +76,18 @@ impl BackendPreferenceContext {
|
||||
}
|
||||
|
||||
impl App {
|
||||
pub(super) fn remember_attached_host_project(&mut self, app_server: &AppServerSession) {
|
||||
if !app_server.uses_remote_workspace()
|
||||
&& !crate::uses_remote_workspace_or_environment(
|
||||
&self.app_server_target,
|
||||
&self.environment_manager,
|
||||
)
|
||||
{
|
||||
self.backend_preference_context.local_cwd =
|
||||
Some(self.chat_widget.config_ref().cwd.to_path_buf());
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) async fn new_thread_host_preference(&self) -> Option<&str> {
|
||||
let defaults = &self.local_settings.tui.default_hosts;
|
||||
if defaults.is_empty() {
|
||||
@@ -134,6 +146,9 @@ impl App {
|
||||
.get(&owner)
|
||||
.cloned()
|
||||
.ok_or("Connection settings are unavailable.")?;
|
||||
if settings.target == AppServerTarget::Cloud && self.chat_widget.thread_id().is_some() {
|
||||
self.remember_attached_host_project(backends.session_at(outgoing_slot));
|
||||
}
|
||||
let preference = if matches!(session, NewBackendSession::CarryDraft)
|
||||
&& let Some(key) = backends.preference_key(slot)
|
||||
{
|
||||
@@ -191,6 +206,7 @@ impl App {
|
||||
/*session_start_source*/ None,
|
||||
/*remote_cwd_override*/ None,
|
||||
/*selected_profile*/ None,
|
||||
self.backend_preference_context.local_cwd.as_deref(),
|
||||
)
|
||||
.await
|
||||
.map_err(|error| format!("Failed to start the new conversation: {error}"))?;
|
||||
|
||||
@@ -1163,6 +1163,7 @@ impl App {
|
||||
session_start_source,
|
||||
/*remote_cwd_override*/ None,
|
||||
/*selected_profile*/ None,
|
||||
self.backend_preference_context.local_cwd.as_deref(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
@@ -1609,6 +1610,7 @@ impl App {
|
||||
.await
|
||||
{
|
||||
Ok(()) => {
|
||||
self.remember_attached_host_project(app_server);
|
||||
if let Some(mut input) = retained_input {
|
||||
// A successful resume is the authority for clearing the old rejection.
|
||||
input.provider_error = None;
|
||||
|
||||
@@ -1170,6 +1170,7 @@ See the Codex keymap documentation for supported actions and examples."
|
||||
return shutdown_on_startup_error(app_server, &mut other_backends, err).await;
|
||||
}
|
||||
}
|
||||
app.remember_attached_host_project(&app_server);
|
||||
if let Some(error) = provider_error {
|
||||
app.chat_widget
|
||||
.show_provider_error(error, /*rejected_message_id*/ None);
|
||||
|
||||
@@ -358,12 +358,21 @@ async fn command_center_new_reads_server_defaults_for_actual_destination() -> Re
|
||||
} else {
|
||||
expected_model.to_string()
|
||||
};
|
||||
app.backend_preference_context.local_cwd = Some(launch.path().to_path_buf());
|
||||
app.new_agents_overview_session(
|
||||
&mut tui,
|
||||
&mut server,
|
||||
explicit_cwd.then(|| destination.path().to_path_buf().abs()),
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
app.backend_preference_context.local_cwd.as_deref(),
|
||||
Some(if !mode.starts_with("remote") && explicit_cwd {
|
||||
destination.path()
|
||||
} else {
|
||||
launch.path()
|
||||
}),
|
||||
);
|
||||
let cwd = match expected_cwd {
|
||||
"launch" => launch.path().display().to_string(),
|
||||
"destination" => destination.path().display().to_string(),
|
||||
@@ -733,6 +742,7 @@ async fn command_center_new_preserves_only_selected_server_profiles() -> Result<
|
||||
/*session_start_source*/ None,
|
||||
/*remote_cwd_override*/ None,
|
||||
Some(&selection),
|
||||
/*host_project_cwd*/ None,
|
||||
)
|
||||
.await?;
|
||||
app.enqueue_primary_thread_session(started.session, started.turns)
|
||||
@@ -814,9 +824,14 @@ async fn command_center_new_preserves_only_selected_server_profiles() -> Result<
|
||||
ServerNotification::ThreadSettingsUpdated(settings),
|
||||
)
|
||||
.await?;
|
||||
app.backend_preference_context.local_cwd = Some(home.path().to_owned());
|
||||
for target in [other, thread_id] {
|
||||
app.select_agents_overview_thread(&mut tui, &mut server, target)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
app.backend_preference_context.local_cwd.as_deref(),
|
||||
Some(home.path()),
|
||||
);
|
||||
assert_eq!(
|
||||
app.chat_widget.thread_id(),
|
||||
Some(target),
|
||||
@@ -1284,6 +1299,7 @@ async fn command_center_new_checkout_and_worktree_preserve_source_and_default_br
|
||||
.await?;
|
||||
let view = app.agents_overview_view(Vec::new(), /*selected_thread_id*/ None);
|
||||
app.chat_widget.show_bottom_pane_view(Box::new(view));
|
||||
let host_project = app.backend_preference_context.local_cwd.clone();
|
||||
app.start_agents_overview_session(
|
||||
&mut tui,
|
||||
&mut failed_server,
|
||||
@@ -1293,6 +1309,7 @@ async fn command_center_new_checkout_and_worktree_preserve_source_and_default_br
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(app.chat_widget.thread_id(), Some(second));
|
||||
assert_eq!(app.backend_preference_context.local_cwd, host_project);
|
||||
assert_eq!(manager.owner(&unused.root).unwrap(), None);
|
||||
assert!(unused.root.exists());
|
||||
let rendered = crate::chatwidget::tests::helpers::render_bottom_popup(
|
||||
|
||||
@@ -880,6 +880,7 @@ async fn same_thread_retry_keeps_subscription_and_restores_draft() -> Result<()>
|
||||
app.harness_overrides.cwd = Some(app.config.cwd.to_path_buf());
|
||||
requests.lock().expect("request recorder lock").clear();
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
app.backend_preference_context.local_cwd = None;
|
||||
app.resume_target_session(
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
@@ -892,6 +893,10 @@ async fn same_thread_retry_keeps_subscription_and_restores_draft() -> Result<()>
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
app.backend_preference_context.local_cwd.as_deref(),
|
||||
Some(app.chat_widget.config_ref().cwd.as_path()),
|
||||
);
|
||||
assert_eq!(recorded_params(&requests, "thread/resume").len(), 1);
|
||||
assert!(recorded_params(&requests, "thread/unsubscribe").is_empty());
|
||||
assert!(!app.chat_widget.is_external_writer_view());
|
||||
|
||||
@@ -466,6 +466,7 @@ impl App {
|
||||
/*session_start_source*/ None,
|
||||
/*remote_cwd_override*/ None,
|
||||
selected_profile.as_ref(),
|
||||
/*host_project_cwd*/ None,
|
||||
)
|
||||
.await
|
||||
};
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
|
||||
mod account;
|
||||
pub(crate) use account::SessionAccount;
|
||||
mod cloud_project;
|
||||
mod daybreak_api_key_compat;
|
||||
mod events;
|
||||
mod external_agent_config;
|
||||
@@ -1012,6 +1013,7 @@ impl AppServerSession {
|
||||
/*session_start_source*/ None,
|
||||
/*remote_cwd_override*/ None,
|
||||
/*selected_profile*/ None,
|
||||
/*host_project_cwd*/ None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -1023,6 +1025,7 @@ impl AppServerSession {
|
||||
session_start_source: Option<ThreadStartSource>,
|
||||
remote_cwd_override: Option<&std::path::Path>,
|
||||
selected_profile: Option<&PermissionProfileSelection>,
|
||||
host_project_cwd: Option<&std::path::Path>,
|
||||
) -> Result<AppServerStartedThread> {
|
||||
if self.thread_params_mode.settings_owner() == ThreadSettingsOwner::Server
|
||||
&& selected_profile.is_some()
|
||||
@@ -1063,12 +1066,27 @@ impl AppServerSession {
|
||||
self.thread_tool_transport().configure(&mut params);
|
||||
}
|
||||
let request_handle = self.request_handle();
|
||||
let (response, history_support, task_tools_available) =
|
||||
request_thread_start_with_history_fallback(&request_handle, request_id, params)
|
||||
let project = if self.thread_params_mode == ThreadParamsMode::Cloud {
|
||||
match host_project_cwd {
|
||||
Some(cwd) => cloud_project::from_local_cwd(cwd).await,
|
||||
None => None,
|
||||
}
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let started = match project {
|
||||
Some(project) => request_handle
|
||||
.start_cloud_project_thread(request_id, params, project)
|
||||
.await
|
||||
.map_err(|err| {
|
||||
bootstrap_request_error("thread/start failed during TUI bootstrap", err)
|
||||
})?;
|
||||
.map(|response| (response, ThreadHistorySupport::Paginated, false)),
|
||||
None => {
|
||||
request_thread_start_with_history_fallback(&request_handle, request_id, params)
|
||||
.await
|
||||
}
|
||||
};
|
||||
let (response, history_support, task_tools_available) = started.map_err(|err| {
|
||||
bootstrap_request_error("thread/start failed during TUI bootstrap", err)
|
||||
})?;
|
||||
if history_support == ThreadHistorySupport::LegacyOnly {
|
||||
self.history_support = ThreadHistorySupport::LegacyOnly;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
//! Resolves a host project into credential-free GitHub checkout metadata.
|
||||
//! Local paths never become cloud executor cwd values or public wire fields.
|
||||
|
||||
use codex_app_server_client::ProjectEnvironment;
|
||||
use std::path::Path;
|
||||
|
||||
pub(super) async fn from_local_cwd(cwd: &Path) -> Option<ProjectEnvironment> {
|
||||
let origin = codex_git_utils::get_git_origin_url(cwd).await?;
|
||||
github_project(origin.as_str())
|
||||
}
|
||||
|
||||
fn github_project(origin: &str) -> Option<ProjectEnvironment> {
|
||||
// A relative local path such as github.com/org/repo is not a network origin.
|
||||
if !origin.contains("://")
|
||||
&& !origin.split_once(':').is_some_and(|(host, _)| {
|
||||
host.rsplit('@')
|
||||
.next()
|
||||
.is_some_and(|host| host.eq_ignore_ascii_case("github.com"))
|
||||
})
|
||||
{
|
||||
return None;
|
||||
}
|
||||
let mut canonical = codex_git_utils::canonicalize_git_remote_url(origin)?;
|
||||
canonical.make_ascii_lowercase();
|
||||
let (host, path) = canonical.split_once('/')?;
|
||||
if !(host == "github.com" || (origin.starts_with("ssh://") && host == "ssh.github.com:443")) {
|
||||
return None;
|
||||
}
|
||||
let (owner, repository) = path.split_once('/')?;
|
||||
if ![owner, repository].iter().all(|part| {
|
||||
!part.is_empty()
|
||||
&& !matches!(*part, "." | "..")
|
||||
&& part.chars().all(|character| {
|
||||
character.is_ascii_alphanumeric() || matches!(character, '-' | '_' | '.')
|
||||
})
|
||||
}) {
|
||||
return None;
|
||||
}
|
||||
Some(ProjectEnvironment {
|
||||
name: repository.to_owned(),
|
||||
cwd: Some(format!("/workspace/{owner}/{repository}")),
|
||||
origin_urls: vec![format!("https://github.com/{owner}/{repository}")],
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "cloud_project_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,153 @@
|
||||
#![cfg(test)]
|
||||
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn github_origins_are_normalized_without_credentials_or_query_strings() {
|
||||
for origin in [
|
||||
"git@github.com:Org/Repo.git",
|
||||
"ssh://git@github.com/Org/Repo.git",
|
||||
"https://user:password@github.com/Org/Repo.git?token=secret#fragment",
|
||||
] {
|
||||
let project = github_project(origin).unwrap();
|
||||
assert_eq!(
|
||||
(project.name, project.origin_urls),
|
||||
(
|
||||
"repo".to_owned(),
|
||||
vec!["https://github.com/org/repo".to_owned()]
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cloud_starts_use_each_requests_host_project_without_caching() {
|
||||
use super::super::AppServerSession;
|
||||
use super::super::ThreadParamsMode;
|
||||
use crate::legacy_core::config::ConfigBuilder;
|
||||
use crate::local_settings::LocalSettings;
|
||||
use codex_app_server_client::AppServerClient;
|
||||
use codex_app_server_client::CloudClient;
|
||||
use codex_app_server_client::CloudOperation;
|
||||
use codex_app_server_protocol::ClientRequest;
|
||||
use codex_app_server_protocol::JSONRPCErrorError;
|
||||
|
||||
let home = tempfile::tempdir().unwrap();
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(home.path().to_path_buf())
|
||||
.build()
|
||||
.await
|
||||
.unwrap();
|
||||
let local_settings = LocalSettings::from(&config);
|
||||
let first = tempfile::tempdir().unwrap();
|
||||
let second = tempfile::tempdir().unwrap();
|
||||
let misleading = tempfile::tempdir().unwrap();
|
||||
for (directory, origin) in [
|
||||
(first.path(), "git@github.com:org/first.git"),
|
||||
(second.path(), "ssh://git@ssh.github.com:443/Org/Second.git"),
|
||||
(
|
||||
misleading.path(),
|
||||
"https://example.com?target=@github.com/org/wrong",
|
||||
),
|
||||
] {
|
||||
for args in [
|
||||
vec!["init", "--quiet"],
|
||||
vec!["remote", "add", "origin", origin],
|
||||
] {
|
||||
assert!(
|
||||
tokio::process::Command::new("git")
|
||||
.args(args)
|
||||
.current_dir(directory)
|
||||
.status()
|
||||
.await
|
||||
.unwrap()
|
||||
.success()
|
||||
);
|
||||
}
|
||||
}
|
||||
let nested = first.path().join("src");
|
||||
std::fs::create_dir(&nested).unwrap();
|
||||
let projectless = tempfile::tempdir().unwrap();
|
||||
let (client, mut commands, _events) = CloudClient::channel(/*capacity*/ 1);
|
||||
let mut session = AppServerSession::new(
|
||||
crate::backend_id::BackendId::new(),
|
||||
AppServerClient::Cloud(client),
|
||||
ThreadParamsMode::Cloud,
|
||||
);
|
||||
let worker = tokio::spawn(async move {
|
||||
for expected in [Some("first"), Some("second"), None, None, None, None] {
|
||||
let command = commands.recv().await.unwrap();
|
||||
match (expected, command.operation) {
|
||||
(
|
||||
Some(expected),
|
||||
CloudOperation::StartProjectThread {
|
||||
params, project, ..
|
||||
},
|
||||
) => {
|
||||
assert_eq!(project.name, expected);
|
||||
assert_eq!(project.cwd, Some(format!("/workspace/org/{expected}")));
|
||||
assert_eq!(params.cwd, None);
|
||||
assert_eq!(params.runtime_workspace_roots, None);
|
||||
assert_eq!(
|
||||
project.origin_urls,
|
||||
vec![format!("https://github.com/org/{expected}")]
|
||||
);
|
||||
}
|
||||
(None, CloudOperation::Request(request)) => {
|
||||
assert!(matches!(*request, ClientRequest::ThreadStart { .. }))
|
||||
}
|
||||
_ => panic!("incorrect project routing"),
|
||||
}
|
||||
command
|
||||
.response
|
||||
.send(Ok(Err(JSONRPCErrorError {
|
||||
code: -32000,
|
||||
message: "checkout failed".into(),
|
||||
data: None,
|
||||
})))
|
||||
.unwrap();
|
||||
}
|
||||
});
|
||||
for (mode, host) in [
|
||||
(ThreadParamsMode::Cloud, Some(nested.as_path())),
|
||||
(ThreadParamsMode::Cloud, Some(second.path())),
|
||||
(ThreadParamsMode::Cloud, Some(misleading.path())),
|
||||
(ThreadParamsMode::Cloud, Some(projectless.path())),
|
||||
(ThreadParamsMode::Cloud, None),
|
||||
(ThreadParamsMode::Remote, Some(first.path())),
|
||||
] {
|
||||
session.thread_params_mode = mode;
|
||||
let result = session
|
||||
.start_thread_with_session_start_source(
|
||||
&local_settings,
|
||||
&config,
|
||||
/*session_start_source*/ None,
|
||||
/*remote_cwd_override*/ None,
|
||||
/*selected_profile*/ None,
|
||||
host,
|
||||
)
|
||||
.await;
|
||||
assert!(result.unwrap_err().to_string().contains("checkout failed"));
|
||||
}
|
||||
worker.await.unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unsupported_or_non_repository_origins_leave_the_thread_blank() {
|
||||
for origin in [
|
||||
"/local/repository",
|
||||
"github.com/org/repo",
|
||||
"https://git.example.com/org/repo",
|
||||
"http://ssh.github.com:443/org/repo.git",
|
||||
"ssh://git@ssh.github.com:8443/org/repo.git",
|
||||
"https://github.com/org",
|
||||
"https://github.com/org/repo/tree/main",
|
||||
"https://github.com/org/repo%2fother",
|
||||
] {
|
||||
assert!(
|
||||
github_project(origin).is_none(),
|
||||
"unexpected origin: {origin}"
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -60,6 +60,7 @@ async fn lifecycle_metadata_uses_local_prompt_history() -> Result<()> {
|
||||
/*session_start_source*/ None,
|
||||
/*remote_cwd_override*/ None,
|
||||
/*selected_profile*/ None,
|
||||
/*host_project_cwd*/ None,
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(started.session.message_history, expected);
|
||||
|
||||
Reference in New Issue
Block a user