Smooth codex login (#1155)

* Add codex setup helper

Pass exit result

Move codex setup to routes

FIx fmt

Fix finalize

* Rename scriptcontext (vibe-kanban 79207902)

Rename the gh cli script context to something more general and use it for installs in crates/server/src/routes/task_attempts/cursor_setup.rs

Rename scriptcontext (vibe-kanban 79207902)

Rename the gh cli script context to something more general and use it for installs in crates/server/src/routes/task_attempts/cursor_setup.rs

Fmt

* Fix missing overrides for codex
This commit is contained in:
Alex Netsch
2025-11-24 19:12:29 +00:00
committed by GitHub
parent fd5ef916b0
commit b50f9ddce3
16 changed files with 234 additions and 56 deletions

View File

@@ -23,7 +23,7 @@ pub enum ScriptContext {
SetupScript,
CleanupScript,
DevServer,
GithubCliSetupScript,
ToolInstallScript,
}
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, TS)]

View File

@@ -19,7 +19,7 @@ use workspace_utils::stream_lines::LinesStreamExt;
use super::{AcpClient, SessionManager};
use crate::{
command::CommandParts,
executors::{ExecutorError, SpawnedChild, acp::AcpEvent},
executors::{ExecutorError, ExecutorExitResult, SpawnedChild, acp::AcpEvent},
};
/// Reusable harness for ACP-based conns (Gemini, Qwen, etc.)
@@ -68,7 +68,7 @@ impl AcpAgentHarness {
let mut child = command.group_spawn()?;
let (exit_tx, exit_rx) = tokio::sync::oneshot::channel::<()>();
let (exit_tx, exit_rx) = tokio::sync::oneshot::channel::<ExecutorExitResult>();
Self::bootstrap_acp_connection(
&mut child,
current_dir.to_path_buf(),
@@ -105,7 +105,7 @@ impl AcpAgentHarness {
let mut child = command.group_spawn()?;
let (exit_tx, exit_rx) = tokio::sync::oneshot::channel::<()>();
let (exit_tx, exit_rx) = tokio::sync::oneshot::channel::<ExecutorExitResult>();
Self::bootstrap_acp_connection(
&mut child,
current_dir.to_path_buf(),
@@ -127,7 +127,7 @@ impl AcpAgentHarness {
cwd: PathBuf,
existing_session: Option<String>,
prompt: String,
exit_signal: Option<tokio::sync::oneshot::Sender<()>>,
exit_signal: Option<tokio::sync::oneshot::Sender<ExecutorExitResult>>,
session_namespace: String,
) -> Result<(), ExecutorError> {
// Take child's stdio for ACP wiring
@@ -387,7 +387,7 @@ impl AcpAgentHarness {
}
// Notify container of completion
if let Some(tx) = exit_signal_tx.take() {
let _ = tx.send(());
let _ = tx.send(ExecutorExitResult::Success);
}
// Cancel session work

View File

@@ -33,7 +33,8 @@ use crate::{
approvals::ExecutorApprovalService,
command::{CmdOverrides, CommandBuilder, CommandParts, apply_overrides},
executors::{
AppendPrompt, AvailabilityInfo, ExecutorError, SpawnedChild, StandardCodingAgentExecutor,
AppendPrompt, AvailabilityInfo, ExecutorError, ExecutorExitResult, SpawnedChild,
StandardCodingAgentExecutor,
codex::{jsonrpc::ExitSignalSender, normalize_logs::Error},
},
stdout_dup::create_stdout_pipe_writer,
@@ -195,9 +196,13 @@ impl StandardCodingAgentExecutor for Codex {
}
impl Codex {
fn build_command_builder(&self) -> CommandBuilder {
let mut builder = CommandBuilder::new("npx -y @openai/codex@0.60.1 app-server");
pub fn base_command() -> &'static str {
"npx -y @openai/codex@0.60.1"
}
fn build_command_builder(&self) -> CommandBuilder {
let mut builder = CommandBuilder::new(Self::base_command());
builder = builder.extend_params(["app-server"]);
if self.oss.unwrap_or(false) {
builder = builder.extend_params(["--oss"]);
}
@@ -328,17 +333,36 @@ impl Codex {
)
.await
{
if matches!(&err, ExecutorError::Io(io_err) if io_err.kind() == std::io::ErrorKind::BrokenPipe)
{
// Broken pipe likely means the parent process exited, so we can ignore it
return;
match &err {
ExecutorError::Io(io_err)
if io_err.kind() == std::io::ErrorKind::BrokenPipe =>
{
// Broken pipe likely means the parent process exited, so we can ignore it
return;
}
ExecutorError::AuthRequired(message) => {
log_writer
.log_raw(&Error::auth_required(message.clone()).raw())
.await
.ok();
// Send failure signal so the process is marked as failed
exit_signal_tx
.send_exit_signal(ExecutorExitResult::Failure)
.await;
return;
}
_ => {
tracing::error!("Codex spawn error: {}", err);
log_writer
.log_raw(&Error::launch_error(err.to_string()).raw())
.await
.ok();
}
}
tracing::error!("Codex spawn error: {}", err);
log_writer
.log_raw(&Error::launch_error(err.to_string()).raw())
.await
.ok();
exit_signal_tx.send_exit_signal().await;
// For other errors, also send failure signal
exit_signal_tx
.send_exit_signal(ExecutorExitResult::Failure)
.await;
}
});
@@ -365,6 +389,12 @@ impl Codex {
JsonRpcPeer::spawn(child_stdin, child_stdout, client.clone(), exit_signal_tx);
client.connect(rpc_peer);
client.initialize().await?;
let auth_status = client.get_auth_status().await?;
if auth_status.auth_method.is_none() {
return Err(ExecutorError::AuthRequired(
"Codex authentication required".to_string(),
));
}
match resume_session {
None => {
let params = conversation_params;

View File

@@ -8,11 +8,11 @@ use std::{
use async_trait::async_trait;
use codex_app_server_protocol::{
AddConversationListenerParams, AddConversationSubscriptionResponse, ApplyPatchApprovalResponse,
ClientInfo, ClientNotification, ClientRequest, ExecCommandApprovalResponse, InitializeParams,
InitializeResponse, InputItem, JSONRPCError, JSONRPCNotification, JSONRPCRequest,
JSONRPCResponse, NewConversationParams, NewConversationResponse, RequestId,
ResumeConversationParams, ResumeConversationResponse, SendUserMessageParams,
SendUserMessageResponse, ServerNotification, ServerRequest,
ClientInfo, ClientNotification, ClientRequest, ExecCommandApprovalResponse,
GetAuthStatusParams, GetAuthStatusResponse, InitializeParams, InitializeResponse, InputItem,
JSONRPCError, JSONRPCNotification, JSONRPCRequest, JSONRPCResponse, NewConversationParams,
NewConversationResponse, RequestId, ResumeConversationParams, ResumeConversationResponse,
SendUserMessageParams, SendUserMessageResponse, ServerNotification, ServerRequest,
};
use codex_protocol::{ConversationId, protocol::ReviewDecision};
use serde::{Serialize, de::DeserializeOwned};
@@ -131,6 +131,16 @@ impl AppServerClient {
self.send_request(request, "sendUserMessage").await
}
pub async fn get_auth_status(&self) -> Result<GetAuthStatusResponse, ExecutorError> {
let request = ClientRequest::GetAuthStatus {
request_id: self.next_request_id(),
params: GetAuthStatusParams {
include_token: Some(true),
refresh_token: Some(false),
},
};
self.send_request(request, "getAuthStatus").await
}
async fn handle_server_request(
&self,
peer: &JsonRpcPeer,
@@ -454,6 +464,7 @@ fn request_id(request: &ClientRequest) -> RequestId {
match request {
ClientRequest::Initialize { request_id, .. }
| ClientRequest::NewConversation { request_id, .. }
| ClientRequest::GetAuthStatus { request_id, .. }
| ClientRequest::ResumeConversation { request_id, .. }
| ClientRequest::AddConversationListener { request_id, .. }
| ClientRequest::SendUserMessage { request_id, .. } => request_id.clone(),

View File

@@ -27,7 +27,7 @@ use tokio::{
sync::{Mutex, oneshot},
};
use crate::executors::ExecutorError;
use crate::executors::{ExecutorError, ExecutorExitResult};
#[derive(Debug)]
pub enum PendingResponse {
@@ -38,18 +38,19 @@ pub enum PendingResponse {
#[derive(Clone)]
pub struct ExitSignalSender {
inner: Arc<Mutex<Option<oneshot::Sender<()>>>>,
inner: Arc<Mutex<Option<oneshot::Sender<ExecutorExitResult>>>>,
}
impl ExitSignalSender {
pub fn new(sender: oneshot::Sender<()>) -> Self {
pub fn new(sender: oneshot::Sender<ExecutorExitResult>) -> Self {
Self {
inner: Arc::new(Mutex::new(Some(sender))),
}
}
pub async fn send_exit_signal(&self) {
pub async fn send_exit_signal(&self, result: ExecutorExitResult) {
if let Some(sender) = self.inner.lock().await.take() {
let _ = sender.send(());
let _ = sender.send(result);
}
}
}
@@ -155,7 +156,7 @@ impl JsonRpcPeer {
}
}
exit_tx.send_exit_signal().await;
exit_tx.send_exit_signal(ExecutorExitResult::Success).await;
let _ = reader_peer.shutdown().await;
});

View File

@@ -1035,12 +1035,16 @@ lazy_static! {
#[derive(Serialize, Deserialize, Debug)]
pub enum Error {
LaunchError { error: String },
AuthRequired { error: String },
}
impl Error {
pub fn launch_error(error: String) -> Self {
Self::LaunchError { error }
}
pub fn auth_required(error: String) -> Self {
Self::AuthRequired { error }
}
pub fn raw(&self) -> String {
serde_json::to_string(self).unwrap_or_default()
@@ -1049,15 +1053,23 @@ impl Error {
impl ToNormalizedEntry for Error {
fn to_normalized_entry(&self) -> NormalizedEntry {
NormalizedEntry {
timestamp: None,
entry_type: NormalizedEntryType::ErrorMessage {
error_type: NormalizedEntryError::Other,
match self {
Error::LaunchError { error } => NormalizedEntry {
timestamp: None,
entry_type: NormalizedEntryType::ErrorMessage {
error_type: NormalizedEntryError::Other,
},
content: error.clone(),
metadata: None,
},
content: match self {
Error::LaunchError { error } => error.clone(),
Error::AuthRequired { error } => NormalizedEntry {
timestamp: None,
entry_type: NormalizedEntryType::ErrorMessage {
error_type: NormalizedEntryError::SetupRequired,
},
content: error.clone(),
metadata: None,
},
metadata: None,
}
}
}

View File

@@ -67,6 +67,8 @@ pub enum ExecutorError {
ExecutableNotFound { program: String },
#[error("Setup helper not supported")]
SetupHelperNotSupported,
#[error("Auth required: {0}")]
AuthRequired(String),
}
#[enum_dispatch]
@@ -154,10 +156,13 @@ impl CodingAgent {
match self {
Self::ClaudeCode(_)
| Self::Amp(_)
| Self::Codex(_)
| Self::Gemini(_)
| Self::QwenCode(_)
| Self::Droid(_) => vec![BaseAgentCapability::SessionFork],
Self::Codex(_) => vec![
BaseAgentCapability::SessionFork,
BaseAgentCapability::SetupHelper,
],
Self::CursorAgent(_) => vec![BaseAgentCapability::SetupHelper],
Self::Opencode(_) | Self::Copilot(_) => vec![],
}
@@ -217,10 +222,19 @@ pub trait StandardCodingAgentExecutor {
}
}
/// Result communicated through the exit signal
#[derive(Debug, Clone, Copy)]
pub enum ExecutorExitResult {
/// Process completed successfully (exit code 0)
Success,
/// Process should be marked as failed (non-zero exit)
Failure,
}
/// Optional exit notification from an executor.
/// When this receiver resolves, the container should gracefully stop the process
/// and mark it as successful (exit code 0).
pub type ExecutorExitSignal = tokio::sync::oneshot::Receiver<()>;
/// and mark it according to the result.
pub type ExecutorExitSignal = tokio::sync::oneshot::Receiver<ExecutorExitResult>;
#[derive(Debug)]
pub struct SpawnedChild {