Mirrors/wrkflw
1
0
Fork 0
mirror of https://github.com/bahdotsh/wrkflw.git synced 2026-05-18 05:05:35 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix: correct multiple bugs found during full codebase review

Browse Source 
It turns out that build_image_inner() in docker.rs was calling
.elapsed() on a SystemTime to compute the tar mtime. That gives
you "seconds since modification" — which is *not* what mtime
means. Mtime is seconds since the Unix epoch. The fix is
.duration_since(UNIX_EPOCH) like a normal person would use.

While at it, the docker logs() call was passing None for options,
which means it wasn't actually requesting stdout or stderr. So
we were collecting logs from a stream that might not have any.
Explicitly set stdout: true and stderr: true.

The emulation runtime had a fun behavioral mismatch with Docker
and Podman: it returned Err on non-zero exit codes, swallowing
all stdout/stderr output. Docker and Podman return Ok with the
exit code and let the caller decide what to do. The engine
already handles non-zero exit codes in the Ok path, so the
emulation was just silently eating useful output for no reason.

The UI had a bounds check in next_job() that was mysteriously
absent from previous_job() — the kind of inconsistency that
waits patiently for someone to hit a stale workflow index and
get a panic. Added the same .filter() guard.

String slicing in the log processor wasn't checking char
boundaries, which is fine until someone's log contains a
multi-byte UTF-8 character before a bracket. Added
is_char_boundary() checks.

Step validation was accepting steps with only a 'name' field
and no 'uses' or 'run', which is not a valid step in GitHub
Actions. Fixed the validation to require at least one of the
two fields that actually *do* something.

Replaced .expect() calls on directory reads in main.rs with
proper error handling. Panicking because a directory isn't
readable is not great user experience.
This commit is contained in:
bahdotsh
2026-04-01 18:59:31 +05:30
parent debd89b8c6
commit aa3366a797
6 changed files with 34 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
crates/executor/src/docker.rs
View File

@@ -1009,10 +1009,15 @@ impl DockerRuntime {
};
// Get logs with a timeout
let log_options = Some(bollard::container::LogsOptions::<String> {
stdout: true,
stderr: true,
..Default::default()
});
let logs_result = tokio::time::timeout(
std::time::Duration::from_secs(10),
self.docker
.logs::<String>(&container.id, None)
.logs(&container.id, log_options)
.collect::<Vec<_>>(),
)
.await;
@@ -1112,10 +1117,10 @@ impl DockerRuntime {
e
))
})?
.elapsed()
.duration_since(std::time::UNIX_EPOCH)
.map_err(|e| {
ContainerError::ContainerExecution(format!(
"Failed to get elapsed time since modification: {}",
"Failed to convert modification time to Unix timestamp: {}",
e
))
})?

60
crates/runtime/src/emulation.rs
View File

@@ -271,23 +271,6 @@ impl ContainerRuntime for EmulationRuntime {
exit_code
));
if exit_code != 0 {
let mut error_details = format!(
"Command failed with exit code: {}\nCommand: {}\n\nError output:\n{}",
exit_code, command_str, error
);
// Add environment variables to error details
error_details.push_str("\n\nEnvironment variables:\n");
for (key, value) in env_vars {
if key.starts_with("GITHUB_") || key.starts_with("CI_") {
error_details.push_str(&format!("{}={}\n", key, value));
}
}
return Err(ContainerError::ContainerExecution(error_details));
}
return Ok(ContainerOutput {
stdout: output,
stderr: error,
@@ -354,27 +337,6 @@ impl ContainerRuntime for EmulationRuntime {
wrkflw_logging::debug(&format!("Command exit code: {}", exit_code));
if exit_code != 0 {
let mut error_details = format!(
"Command failed with exit code: {}\nCommand: {}\n\nError output:\n{}",
exit_code, command_str, error
);
// Add environment variables to error details
error_details.push_str("\n\nEnvironment variables:\n");
for (key, value) in env_vars {
if key.starts_with("GITHUB_")
|| key.starts_with("RUST")
|| key.starts_with("CARGO")
|| key.starts_with("CI_")
{
error_details.push_str(&format!("{}={}\n", key, value));
}
}
return Err(ContainerError::ContainerExecution(error_details));
}
return Ok(ContainerOutput {
stdout: output,
stderr: error,
@@ -409,28 +371,8 @@ impl ContainerRuntime for EmulationRuntime {
wrkflw_logging::debug(&format!("Command completed with exit code: {}", exit_code));
if exit_code != 0 {
let mut error_details = format!(
"Command failed with exit code: {}\nCommand: {}\n\nError output:\n{}",
exit_code, command_str, error
);
// Add environment variables to error details
error_details.push_str("\n\nEnvironment variables:\n");
for (key, value) in env_vars {
if key.starts_with("GITHUB_") || key.starts_with("CI_") {
error_details.push_str(&format!("{}={}\n", key, value));
}
}
return Err(ContainerError::ContainerExecution(error_details));
}
Ok(ContainerOutput {
stdout: format!(
"Emulated container execution with command: {}\n\nOutput:\n{}",
command_str, output
),
stdout: output,
stderr: error,
exit_code,
})

6
crates/ui/src/app/state.rs
View File

@@ -304,12 +304,10 @@ impl App {
pub fn previous_job(&mut self) {
let current_workflow_idx = self
.current_execution
.or_else(|| self.workflow_list_state.selected());
.or_else(|| self.workflow_list_state.selected())
.filter(|&idx| idx < self.workflows.len());
if let Some(workflow_idx) = current_workflow_idx {
if workflow_idx >= self.workflows.len() {
return;
}
if let Some(execution) = &self.workflows[workflow_idx].execution_details {
if execution.jobs.is_empty() {

8
crates/ui/src/log_processor.rs
View File

@@ -202,7 +202,7 @@ impl LogProcessor {
// Extract timestamp from log format [HH:MM:SS]
let timestamp = if log_line.starts_with('[') && log_line.contains(']') {
let end = log_line.find(']').unwrap_or(0);
if end > 1 {
if end > 1 && log_line.is_char_boundary(1) && log_line.is_char_boundary(end) {
log_line[1..end].to_string()
} else {
"??:??:??".to_string()
@@ -240,7 +240,11 @@ impl LogProcessor {
// Extract content after timestamp
let content = if log_line.starts_with('[') && log_line.contains(']') {
let start = log_line.find(']').unwrap_or(0) + 1;
log_line[start..].trim()
if log_line.is_char_boundary(start) {
log_line[start..].trim()
} else {
log_line
}
} else {
log_line
};

6
crates/validators/src/steps.rs
View File

@@ -8,12 +8,12 @@ pub fn validate_steps(steps: &[Value], job_name: &str, result: &mut ValidationRe
for (i, step) in steps.iter().enumerate() {
if let Some(step_map) = step.as_mapping() {
if !step_map.contains_key(Value::String("name".to_string()))
&& !step_map.contains_key(Value::String("uses".to_string()))
// A step must have either 'uses' or 'run' (name alone is not sufficient)
if !step_map.contains_key(Value::String("uses".to_string()))
&& !step_map.contains_key(Value::String("run".to_string()))
{
result.add_issue(format!(
"Job '{}', step {}: Missing 'name', 'uses', or 'run' field",
"Job '{}', step {}: Missing required 'uses' or 'run' field",
job_name,
i + 1
));

18
crates/wrkflw/src/main.rs
View File

@@ -338,8 +338,13 @@ async fn main() {
if validate_path.is_dir() {
// Validate all workflow files in the directory
let entries = std::fs::read_dir(&validate_path)
.expect("Failed to read directory")
let entries = match std::fs::read_dir(&validate_path) {
Ok(rd) => rd,
Err(e) => {
eprintln!("Failed to read directory {}: {}", validate_path.display(), e);
std::process::exit(1);
}
}
.filter_map(|entry| entry.ok())
.filter(|entry| {
entry.path().is_file()
@@ -648,8 +653,13 @@ fn list_workflows_and_pipelines(verbose: bool) {
if github_path.exists() && github_path.is_dir() {
println!("GitHub Workflows:");
let entries = std::fs::read_dir(&github_path)
.expect("Failed to read directory")
let entries = match std::fs::read_dir(&github_path) {
Ok(rd) => rd,
Err(e) => {
eprintln!("Failed to read directory {}: {}", github_path.display(), e);
return;
}
}
.filter_map(|entry| entry.ok())
.filter(|entry| {
entry.path().is_file()