Setup script error output (#166)

* Improved the setup script error reporting UX

Show stdout/stderr output from setup script even when it exits with non-zero error code.

* UI: don't display the internal maker "---STDERR_CHUNK_BOUNDARY---"

* fmt

* Address review feedback
This commit is contained in:
Solomon
2025-07-14 16:15:29 +01:00
committed by GitHub
parent 7964b644f4
commit fde1ef5c58
6 changed files with 172 additions and 34 deletions

View File

@@ -24,7 +24,7 @@ tracing-subscriber = { workspace = true }
sqlx = { version = "0.8.6", features = ["runtime-tokio-rustls", "sqlite", "chrono", "uuid"] }
chrono = { version = "0.4", features = ["serde"] }
uuid = { version = "1.0", features = ["v4", "serde"] }
ts-rs = { version = "9.0", features = ["uuid-impl", "chrono-impl"] }
ts-rs = { version = "9.0", features = ["uuid-impl", "chrono-impl", "no-serde-warnings"] }
dirs = "5.0"
git2 = "0.18"
async-trait = "0.1"
@@ -56,7 +56,7 @@ tempfile = "3.8"
[build-dependencies]
dotenv = "0.15"
ts-rs = { version = "9.0", features = ["uuid-impl", "chrono-impl"] }
ts-rs = { version = "9.0", features = ["uuid-impl", "chrono-impl", "no-serde-warnings"] }
[profile.release]
debug = true

View File

@@ -8,6 +8,7 @@ use uuid::Uuid;
use crate::executors::{
AmpExecutor, ClaudeExecutor, EchoExecutor, GeminiExecutor, OpencodeExecutor,
SetupScriptExecutor,
};
// Constants for database streaming
@@ -345,6 +346,7 @@ pub enum ExecutorConfig {
Amp,
Gemini,
Opencode,
SetupScript { script: String },
// Future executors can be added here
// Shell { command: String },
// Docker { image: String, command: String },
@@ -368,6 +370,9 @@ impl FromStr for ExecutorConfig {
"amp" => Ok(ExecutorConfig::Amp),
"gemini" => Ok(ExecutorConfig::Gemini),
"opencode" => Ok(ExecutorConfig::Opencode),
"setup_script" => Ok(ExecutorConfig::SetupScript {
script: "setup script".to_string(),
}),
_ => Err(format!("Unknown executor type: {}", s)),
}
}
@@ -381,6 +386,9 @@ impl ExecutorConfig {
ExecutorConfig::Amp => Box::new(AmpExecutor),
ExecutorConfig::Gemini => Box::new(GeminiExecutor),
ExecutorConfig::Opencode => Box::new(OpencodeExecutor),
ExecutorConfig::SetupScript { script } => {
Box::new(SetupScriptExecutor::new(script.clone()))
}
}
}
@@ -395,6 +403,7 @@ impl ExecutorConfig {
ExecutorConfig::Gemini => {
dirs::home_dir().map(|home| home.join(".gemini").join("settings.json"))
}
ExecutorConfig::SetupScript { .. } => None,
}
}
@@ -406,12 +415,16 @@ impl ExecutorConfig {
ExecutorConfig::Claude => Some(vec!["mcpServers"]),
ExecutorConfig::Amp => Some(vec!["amp", "mcpServers"]), // Nested path for Amp
ExecutorConfig::Gemini => Some(vec!["mcpServers"]),
ExecutorConfig::SetupScript { .. } => None, // Setup scripts don't support MCP
}
}
/// Check if this executor supports MCP configuration
pub fn supports_mcp(&self) -> bool {
!matches!(self, ExecutorConfig::Echo)
!matches!(
self,
ExecutorConfig::Echo | ExecutorConfig::SetupScript { .. }
)
}
/// Get the display name for this executor
@@ -422,6 +435,7 @@ impl ExecutorConfig {
ExecutorConfig::Claude => "Claude",
ExecutorConfig::Amp => "Amp",
ExecutorConfig::Gemini => "Gemini",
ExecutorConfig::SetupScript { .. } => "Setup Script",
}
}
}
@@ -434,6 +448,7 @@ impl std::fmt::Display for ExecutorConfig {
ExecutorConfig::Amp => "amp",
ExecutorConfig::Gemini => "gemini",
ExecutorConfig::Opencode => "opencode",
ExecutorConfig::SetupScript { .. } => "setup_script",
};
write!(f, "{}", s)
}

View File

@@ -14,6 +14,12 @@ pub struct SetupScriptExecutor {
pub script: String,
}
impl SetupScriptExecutor {
pub fn new(script: String) -> Self {
Self { script }
}
}
#[async_trait]
impl Executor for SetupScriptExecutor {
async fn spawn(
@@ -50,4 +56,75 @@ impl Executor for SetupScriptExecutor {
Ok(child)
}
/// Normalize setup script logs into a readable format
fn normalize_logs(
&self,
logs: &str,
_worktree_path: &str,
) -> Result<crate::executor::NormalizedConversation, String> {
let mut entries = Vec::new();
// Add script command as first entry
entries.push(crate::executor::NormalizedEntry {
timestamp: None,
entry_type: crate::executor::NormalizedEntryType::SystemMessage,
content: format!("Executing setup script:\n{}", self.script),
metadata: None,
});
// Process the logs - split by lines and create entries
if !logs.trim().is_empty() {
let lines: Vec<&str> = logs.lines().collect();
let mut current_chunk = String::new();
for line in lines {
current_chunk.push_str(line);
current_chunk.push('\n');
// Create entry for every 10 lines or when we encounter an error-like line
if current_chunk.lines().count() >= 10
|| line.to_lowercase().contains("error")
|| line.to_lowercase().contains("failed")
|| line.to_lowercase().contains("exception")
{
let entry_type = if line.to_lowercase().contains("error")
|| line.to_lowercase().contains("failed")
|| line.to_lowercase().contains("exception")
{
crate::executor::NormalizedEntryType::ErrorMessage
} else {
crate::executor::NormalizedEntryType::SystemMessage
};
entries.push(crate::executor::NormalizedEntry {
timestamp: Some(chrono::Utc::now().to_rfc3339()),
entry_type,
content: current_chunk.trim().to_string(),
metadata: None,
});
current_chunk.clear();
}
}
// Add any remaining content
if !current_chunk.trim().is_empty() {
entries.push(crate::executor::NormalizedEntry {
timestamp: Some(chrono::Utc::now().to_rfc3339()),
entry_type: crate::executor::NormalizedEntryType::SystemMessage,
content: current_chunk.trim().to_string(),
metadata: None,
});
}
}
Ok(crate::executor::NormalizedConversation {
entries,
session_id: None,
executor_type: "setup_script".to_string(),
prompt: Some(self.script.clone()),
summary: None,
})
}
}

View File

@@ -1,11 +1,27 @@
use chrono::{DateTime, Utc};
use serde::{Deserialize, Serialize};
use serde::{Deserialize, Serialize, Serializer};
use sqlx::{FromRow, SqlitePool, Type};
use ts_rs::TS;
use uuid::Uuid;
use crate::app_state::ExecutionType;
/// Filter out stderr boundary markers from output
fn filter_stderr_boundary_markers(stderr: &Option<String>) -> Option<String> {
stderr
.as_ref()
.map(|s| s.replace("---STDERR_CHUNK_BOUNDARY---", ""))
}
/// Custom serializer for stderr field that filters out boundary markers
fn serialize_filtered_stderr<S>(stderr: &Option<String>, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
let filtered = filter_stderr_boundary_markers(stderr);
filtered.serialize(serializer)
}
#[derive(Debug, Clone, Type, Serialize, Deserialize, PartialEq, TS)]
#[sqlx(type_name = "execution_process_status", rename_all = "lowercase")]
#[serde(rename_all = "lowercase")]
@@ -59,6 +75,7 @@ pub struct ExecutionProcess {
pub args: Option<String>, // JSON array of arguments
pub working_directory: String,
pub stdout: Option<String>,
#[serde(serialize_with = "serialize_filtered_stderr")]
pub stderr: Option<String>,
pub exit_code: Option<i64>,
pub started_at: DateTime<Utc>,

View File

@@ -13,7 +13,9 @@ use crate::{
executor::{ExecutorConfig, NormalizedConversation, NormalizedEntry, NormalizedEntryType},
models::{
config::Config,
execution_process::{ExecutionProcess, ExecutionProcessStatus, ExecutionProcessSummary},
execution_process::{
ExecutionProcess, ExecutionProcessStatus, ExecutionProcessSummary, ExecutionProcessType,
},
executor_session::ExecutorSession,
task::Task,
task_attempt::{
@@ -1215,22 +1217,33 @@ pub async fn get_execution_process_normalized_logs(
if !stdout.trim().is_empty() {
// Determine executor type and create appropriate executor for normalization
let executor_type = process.executor_type.as_deref().unwrap_or("unknown");
let executor_config = match executor_type {
"amp" => ExecutorConfig::Amp,
"claude" => ExecutorConfig::Claude,
"echo" => ExecutorConfig::Echo,
"gemini" => ExecutorConfig::Gemini,
"opencode" => ExecutorConfig::Opencode,
_ => {
tracing::warn!(
"Unsupported executor type: {}, cannot normalize logs properly",
executor_type
);
return Ok(ResponseJson(ApiResponse {
success: false,
data: None,
message: Some(format!("Unsupported executor type: {}", executor_type)),
}));
let executor_config = if process.process_type == ExecutionProcessType::SetupScript {
// For setup scripts, use the setup script executor
ExecutorConfig::SetupScript {
script: executor_session
.as_ref()
.and_then(|s| s.prompt.clone())
.unwrap_or_else(|| "setup script".to_string()),
}
} else {
match executor_type {
"amp" => ExecutorConfig::Amp,
"claude" => ExecutorConfig::Claude,
"echo" => ExecutorConfig::Echo,
"gemini" => ExecutorConfig::Gemini,
"opencode" => ExecutorConfig::Opencode,
_ => {
tracing::warn!(
"Unsupported executor type: {}, cannot normalize logs properly",
executor_type
);
return Ok(ResponseJson(ApiResponse {
success: false,
data: None,
message: Some(format!("Unsupported executor type: {}", executor_type)),
}));
}
}
};
@@ -1248,9 +1261,9 @@ pub async fn get_execution_process_normalized_logs(
}
Err(_) => {
tracing::debug!(
"Working directory {} no longer exists, using stored path for normalization",
process.working_directory
);
"Working directory {} no longer exists, using stored path for normalization",
process.working_directory
);
process.working_directory.clone()
}
};
@@ -1292,12 +1305,16 @@ pub async fn get_execution_process_normalized_logs(
for chunk in chunks {
let chunk_trimmed = chunk.trim();
if !chunk_trimmed.is_empty() {
stderr_entries.push(NormalizedEntry {
timestamp: Some(chrono::Utc::now().to_rfc3339()),
entry_type: NormalizedEntryType::ErrorMessage,
content: chunk_trimmed.to_string(),
metadata: None,
});
// Filter out any remaining boundary markers from the chunk content
let filtered_content = chunk_trimmed.replace("---STDERR_CHUNK_BOUNDARY---", "");
if !filtered_content.trim().is_empty() {
stderr_entries.push(NormalizedEntry {
timestamp: Some(chrono::Utc::now().to_rfc3339()),
entry_type: NormalizedEntryType::ErrorMessage,
content: filtered_content.trim().to_string(),
metadata: None,
});
}
}
}
@@ -1323,13 +1340,19 @@ pub async fn get_execution_process_normalized_logs(
});
// Create final normalized conversation
let executor_type = if process.process_type == ExecutionProcessType::SetupScript {
"setup_script".to_string()
} else {
process
.executor_type
.clone()
.unwrap_or("unknown".to_string())
};
let normalized_conversation = NormalizedConversation {
entries: all_entries,
session_id: None,
executor_type: process
.executor_type
.clone()
.unwrap_or("unknown".to_string()),
executor_type,
prompt: executor_session.as_ref().and_then(|s| s.prompt.clone()),
summary: executor_session.as_ref().and_then(|s| s.summary.clone()),
};

View File

@@ -673,6 +673,7 @@ impl ProcessService {
crate::executor::ExecutorConfig::Amp => "amp",
crate::executor::ExecutorConfig::Gemini => "gemini",
crate::executor::ExecutorConfig::Opencode => "opencode",
crate::executor::ExecutorConfig::SetupScript { .. } => "setup_script",
};
(
"executor".to_string(),
@@ -687,6 +688,7 @@ impl ProcessService {
crate::executor::ExecutorConfig::Amp => "amp",
crate::executor::ExecutorConfig::Gemini => "gemini",
crate::executor::ExecutorConfig::Opencode => "opencode",
crate::executor::ExecutorConfig::SetupScript { .. } => "setup_script",
};
(
"followup_executor".to_string(),
@@ -847,6 +849,10 @@ impl ProcessService {
return Err(TaskAttemptError::TaskNotFound); // No session ID for followup
}
}
crate::executor::ExecutorConfig::SetupScript { .. } => {
// Setup scripts don't support followup, use regular setup script
config.create_executor()
}
};
executor