scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-08-28 05:27:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Fix asset capture robustness and retry filtering

Browse source
This commit is contained in:
Bryan Helmkamp 2026-03-29 22:23:33 -04:00
parent 4268d04052
commit 31ef250e2c
No known key found for this signature in database
7 changed files with 213 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
docs/reference/cli.mdx
View file

@ -759,12 +759,14 @@ List assets (screenshots, test reports, traces) collected from a workflow run.
```bash
fabro asset list <RUN_ID>
fabro asset list <RUN_ID> --node verify --json
fabro asset list <RUN_ID> --node verify --retry 2
```
| Argument / Flag | Description |
|---|---|
| `<RUN_ID>` | Run ID or unambiguous prefix (required) |
| `--node <NODE>` | Filter to assets from a specific node |
| `--retry <N>` | Filter to assets from a specific retry attempt |
| `--json` | Output as JSON |
## `fabro asset cp`
@ -775,6 +777,7 @@ Copy assets from a workflow run to the local filesystem.
fabro asset cp <RUN_ID> ./output # all assets, flat
fabro asset cp <RUN_ID> ./output --tree # preserve directory structure
fabro asset cp <RUN_ID>:report.html ./output # specific file
fabro asset cp <RUN_ID>:report.html ./output --node verify --retry 2
```
| Argument / Flag | Description |
@ -782,9 +785,10 @@ fabro asset cp <RUN_ID>:report.html ./output # specific file
| `<SOURCE>` | `RUN_ID` (all assets) or `RUN_ID:path` (specific file) |
| `[DEST]` | Destination directory (defaults to `.`) |
| `--node <NODE>` | Filter to assets from a specific node |
| `--retry <N>` | Filter to assets from a specific retry attempt |
| `--tree` | Preserve `\{node\}/\{retry\}/` directory structure |
When copying all assets in flat mode, filenames must be unique across nodes. Use `--tree` or `--node` to disambiguate.
When copying all assets in flat mode, filenames must be unique across nodes and retries. Use `--tree`, `--node`, or `--retry` to disambiguate.
## `fabro provider login`

8
lib/crates/fabro-cli/src/args.rs
View file

@ -321,6 +321,10 @@ pub(crate) struct AssetListArgs {
#[arg(long)]
pub(crate) node: Option<String>,
/// Filter to assets from a specific retry attempt
#[arg(long)]
pub(crate) retry: Option<u32>,
/// Output as JSON
#[arg(long)]
pub(crate) json: bool,
@ -339,6 +343,10 @@ pub(crate) struct AssetCpArgs {
#[arg(long)]
pub(crate) node: Option<String>,
/// Filter to assets from a specific retry attempt
#[arg(long)]
pub(crate) retry: Option<u32>,
/// Preserve {node_slug}/retry_{N}/ directory structure
#[arg(long)]
pub(crate) tree: bool,

16
lib/crates/fabro-cli/src/commands/asset/cp.rs
View file

@ -16,7 +16,11 @@ pub(super) fn cp_command(args: &AssetCpArgs, globals: &GlobalArgs) -> Result<()>
let (run_id, asset_path) = parse_source(&args.source);
let run = resolve_run(&base, run_id)?;
let runtime_state = RuntimeState::new(&run.path);
let entries = scan_assets(&runtime_state.assets_dir(), args.node.as_deref())?;
let entries = scan_assets(
&runtime_state.assets_dir(),
args.node.as_deref(),
args.retry,
)?;
if entries.is_empty() {
bail!("No assets found for this run");
@ -33,14 +37,14 @@ pub(super) fn cp_command(args: &AssetCpArgs, globals: &GlobalArgs) -> Result<()>
if matching.is_empty() {
bail!("No asset matching path '{path}' found in this run");
}
if matching.len() > 1 && args.node.is_none() {
let nodes: Vec<_> = matching
if matching.len() > 1 {
let candidates: Vec<_> = matching
.iter()
.map(|entry| entry.node_slug.as_str())
.map(|entry| format!("{}:retry_{}", entry.node_slug, entry.retry))
.collect();
bail!(
"Path '{path}' exists in multiple nodes: {}. Use --node to disambiguate.",
nodes.join(", ")
"Path '{path}' matches multiple assets: {}. Use --node and/or --retry to disambiguate.",
candidates.join(", ")
);
}

6
lib/crates/fabro-cli/src/commands/asset/list.rs
View file

@ -13,7 +13,11 @@ pub(super) fn list_command(args: &AssetListArgs, globals: &GlobalArgs) -> Result
let base = runs_base(&cli_settings.storage_dir());
let run = resolve_run(&base, &args.run_id)?;
let runtime_state = RuntimeState::new(&run.path);
let entries = scan_assets(&runtime_state.assets_dir(), args.node.as_deref())?;
let entries = scan_assets(
&runtime_state.assets_dir(),
args.node.as_deref(),
args.retry,
)?;
if args.json {
println!("{}", serde_json::to_string_pretty(&entries)?);

35
lib/crates/fabro-cli/tests/it/scenario.rs
View file

@ -783,7 +783,7 @@ fn local_run_lifecycle() {
);
// 6. Seed a synthetic asset so asset list/cp have something to work with.
let asset_dir = RuntimeState::new(&run_dir).asset_stage_dir("step1", 0);
let asset_dir = RuntimeState::new(&run_dir).asset_stage_dir("step1", 1);
std::fs::create_dir_all(&asset_dir).unwrap();
std::fs::write(asset_dir.join("output.txt"), "asset-content-42").unwrap();
std::fs::write(
@ -791,8 +791,16 @@ fn local_run_lifecycle() {
r#"{"files_copied":1,"total_bytes":16,"files_skipped":0,"download_errors":0,"hash_errors":0,"captured_assets":[{"path":"output.txt","mime":"text/plain","content_md5":"f02439728c0a94b7bfc465acb1201a1f","content_sha256":"0af9dea3e1c2dec968531c18c9331659b8268e8c9cf24b01cda7b8ce51d2ff00","bytes":16}]}"#,
)
.unwrap();
let retry_two_dir = RuntimeState::new(&run_dir).asset_stage_dir("step1", 2);
std::fs::create_dir_all(&retry_two_dir).unwrap();
std::fs::write(retry_two_dir.join("output.txt"), "asset-content-84").unwrap();
std::fs::write(
retry_two_dir.join("manifest.json"),
r#"{"files_copied":1,"total_bytes":16,"files_skipped":0,"download_errors":0,"hash_errors":0,"captured_assets":[{"path":"output.txt","mime":"text/plain","content_md5":"5b4e23e40a1630f9caa15a4cb6cfb79b","content_sha256":"1f71e0df61fc3b4e1ee3aba7ceac9ae391af22595b5b5630d97d34cf33d4d540","bytes":16}]}"#,
)
.unwrap();
// 7. asset list — now shows the seeded asset
// 7. asset list — now shows the seeded assets
let asset_list_out2 = fabro_home(&["asset", "list", &run_id, "--json"]).success();
let asset_list_stdout2 =
String::from_utf8(asset_list_out2.get_output().stdout.clone()).unwrap();
@ -800,13 +808,21 @@ fn local_run_lifecycle() {
.expect("asset list --json should produce a JSON array");
assert_eq!(
assets.len(),
1,
"should have one asset: {asset_list_stdout2}"
2,
"should have two assets: {asset_list_stdout2}"
);
assert_eq!(assets[0]["relative_path"].as_str(), Some("output.txt"));
assert_eq!(assets[0]["node_slug"].as_str(), Some("step1"));
let retry_filtered_out =
fabro_home(&["asset", "list", &run_id, "--retry", "1", "--json"]).success();
let retry_filtered_stdout =
String::from_utf8(retry_filtered_out.get_output().stdout.clone()).unwrap();
let retry_filtered_assets: Vec<Value> = serde_json::from_str(&retry_filtered_stdout)
.expect("asset list --json should produce a JSON array");
assert_eq!(retry_filtered_assets.len(), 1);
assert_eq!(retry_filtered_assets[0]["retry"].as_u64(), Some(1));
// 8. asset cp — copy the asset out
// 8. asset cp — ambiguous without --retry when multiple retries captured the same path
let asset_dest = tmp.path().join("asset_copy");
fabro_home(&[
"asset",
@ -814,6 +830,15 @@ fn local_run_lifecycle() {
&format!("{run_id}:output.txt"),
asset_dest.to_str().unwrap(),
])
.failure();
fabro_home(&[
"asset",
"cp",
&format!("{run_id}:output.txt"),
asset_dest.to_str().unwrap(),
"--retry",
"1",
])
.success();
let copied = std::fs::read_to_string(asset_dest.join("output.txt")).unwrap();
assert_eq!(

93
lib/crates/fabro-workflows/src/asset_snapshot.rs
View file

@ -1,8 +1,8 @@
use fabro_agent::Sandbox;
use fabro_sandbox::shell_quote;
use serde::{Deserialize, Serialize};
use sha2::{Digest, Sha256};
use std::path::Path;
use tokio::fs;
use tracing::{debug, warn};
/// A file discovered by the find command.
@ -67,12 +67,12 @@ const MAX_TOTAL_SIZE: u64 = 50 * 1024 * 1024;
/// Globs with `/` are treated as directory patterns: the trailing `/**` (if any) is stripped
/// and the remainder is matched via `-path '*/{dir}/*'`.
pub fn build_find_command(root: &str, platform: &str, globs: &[String]) -> String {
let mut cmd = format!("find {root}");
let mut cmd = format!("find {}", shell_quote(root));
// Prune excluded directories
let prune_parts: Vec<String> = EXCLUDE_DIRS
.iter()
.map(|d| format!("-name '{d}'"))
.map(|d| format!("-name {}", shell_quote(d)))
.collect();
cmd.push_str(" \\( ");
cmd.push_str(&prune_parts.join(" -o "));
@ -86,10 +86,10 @@ pub fn build_find_command(root: &str, platform: &str, globs: &[String]) -> Strin
if glob.contains('/') {
// Directory-style glob: strip trailing /** and match as path
let dir = glob.trim_end_matches("/**").trim_end_matches("/*");
conditions.push(format!(" -path '*/{dir}/*'"));
conditions.push(format!(" -path {}", shell_quote(&format!("*/{dir}/*"))));
} else {
// Filename glob
conditions.push(format!(" -name '{glob}'"));
conditions.push(format!(" -name {}", shell_quote(glob)));
}
}
cmd.push_str(&conditions.join(" -o"));
@ -275,6 +275,31 @@ fn compute_asset_info(
})
}
fn write_asset_manifest(stage_dir: &Path, summary: &AssetCollectionSummary) -> Result<(), String> {
let json = serde_json::to_string_pretty(summary)
.map_err(|e| format!("failed to serialize manifest: {e}"))?;
let manifest_path = stage_dir.join("manifest.json");
if let Some(parent) = manifest_path.parent() {
std::fs::create_dir_all(parent).map_err(|e| {
format!(
"failed to create manifest directory {}: {e}",
parent.display()
)
})?;
}
std::fs::write(&manifest_path, json)
.map_err(|e| format!("failed to write {}: {e}", manifest_path.display()))?;
Ok(())
}
fn cleanup_asset_stage_dir(stage_dir: &Path) -> Result<(), String> {
if !stage_dir.exists() {
return Ok(());
}
std::fs::remove_dir_all(stage_dir)
.map_err(|e| format!("failed to clean up {}: {e}", stage_dir.display()))
}
/// Collect asset files matching the configured globs that were created during this stage.
pub async fn collect_assets(
sandbox: &dyn Sandbox,
@ -348,12 +373,12 @@ pub async fn collect_assets(
};
if files_copied > 0 {
if let Ok(json) = serde_json::to_string_pretty(&summary) {
let manifest_path = stage_dir.join("manifest.json");
if let Some(parent) = manifest_path.parent() {
let _ = fs::create_dir_all(parent).await;
}
let _ = fs::write(&manifest_path, json).await;
if let Err(e) = write_asset_manifest(stage_dir, &summary) {
let cleanup_suffix = match cleanup_asset_stage_dir(stage_dir) {
Ok(()) => String::new(),
Err(cleanup_err) => format!("; cleanup failed: {cleanup_err}"),
};
return Err(format!("{e}{cleanup_suffix}"));
}
}
@ -399,6 +424,7 @@ mod tests {
use super::*;
use fabro_agent::sandbox::ExecResult;
use std::collections::HashMap;
use std::fs;
/// Minimal mock sandbox for asset_snapshot tests.
struct AssetMockSandbox {
@ -637,6 +663,15 @@ mod tests {
assert!(cmd.contains("-name '*.trace.zip'"));
}
#[test]
fn build_find_command_shell_quotes_root_and_globs() {
let globs = vec!["test result's/**".to_string(), "*.trace zip".to_string()];
let cmd = build_find_command("/workspace with spaces", "linux", &globs);
assert!(cmd.starts_with(&format!("find {}", shell_quote("/workspace with spaces"))));
assert!(cmd.contains(&format!("-path {}", shell_quote("*/test result's/*"))));
assert!(cmd.contains(&format!("-name {}", shell_quote("*.trace zip"))));
}
#[test]
fn build_find_command_darwin() {
let globs = vec!["test-results/**".to_string()];
@ -749,6 +784,42 @@ mod tests {
assert_eq!(summary.hash_errors, 0);
}
#[cfg(unix)]
#[test]
fn write_asset_manifest_failure_cleans_up_stage_dir() {
use std::fs::Permissions;
use std::os::unix::fs::PermissionsExt;
let parent = tempfile::tempdir().unwrap();
let stage_dir = parent.path().join("stage");
fs::create_dir_all(stage_dir.join("test-results")).unwrap();
fs::write(stage_dir.join("test-results/report.xml"), "<test/>").unwrap();
fs::set_permissions(&stage_dir, Permissions::from_mode(0o555)).unwrap();
let summary = AssetCollectionSummary {
files_copied: 1,
total_bytes: 7,
files_skipped: 0,
download_errors: 0,
hash_errors: 0,
captured_assets: vec![CapturedAssetInfo {
path: "test-results/report.xml".to_string(),
mime: "text/xml".to_string(),
content_md5: "f1430934c390c118ed2f148e1d44d36c".to_string(),
content_sha256: "28e51ddac37391b99c2b9053f1122d0bf84b02365e6fd8c6e8667378bd00f436"
.to_string(),
bytes: 7,
}],
};
let err = write_asset_manifest(&stage_dir, &summary).unwrap_err();
assert!(err.contains("failed to write"));
fs::set_permissions(&stage_dir, Permissions::from_mode(0o755)).unwrap();
cleanup_asset_stage_dir(&stage_dir).unwrap();
assert!(!stage_dir.exists());
}
#[test]
fn collect_asset_paths_from_manifests() {
let tmp = tempfile::tempdir().unwrap();

74
lib/crates/fabro-workflows/src/assets.rs
View file

@ -20,7 +20,11 @@ fn serialize_path<S: serde::Serializer>(path: &Path, serializer: S) -> Result<S:
}
/// Walk `{assets_dir}/*/retry_*/manifest.json`, stat each file, and return entries.
pub fn scan_assets(assets_dir: &Path, node_filter: Option<&str>) -> Result<Vec<AssetEntry>> {
pub fn scan_assets(
assets_dir: &Path,
node_filter: Option<&str>,
retry_filter: Option<u32>,
) -> Result<Vec<AssetEntry>> {
let Ok(nodes) = std::fs::read_dir(assets_dir) else {
return Ok(Vec::new());
};
@ -49,6 +53,12 @@ pub fn scan_assets(assets_dir: &Path, node_filter: Option<&str>) -> Result<Vec<A
.and_then(|value| value.parse().ok())
.unwrap_or(0);
if let Some(filter) = retry_filter {
if retry != filter {
continue;
}
}
let manifest = retry_dir.join("manifest.json");
let Ok(contents) = std::fs::read_to_string(&manifest) else {
continue;
@ -72,3 +82,65 @@ pub fn scan_assets(assets_dir: &Path, node_filter: Option<&str>) -> Result<Vec<A
Ok(entries)
}
#[cfg(test)]
mod tests {
use super::*;
use crate::asset_snapshot::{AssetCollectionSummary, CapturedAssetInfo};
#[test]
fn scan_assets_filters_by_node_and_retry() {
let tmp = tempfile::tempdir().unwrap();
let assets_dir = tmp.path().join("cache/artifacts/assets");
let retry_1 = assets_dir.join("work/retry_1");
std::fs::create_dir_all(&retry_1).unwrap();
std::fs::write(
retry_1.join("manifest.json"),
serde_json::to_string(&AssetCollectionSummary {
files_copied: 1,
total_bytes: 5,
files_skipped: 0,
download_errors: 0,
hash_errors: 0,
captured_assets: vec![CapturedAssetInfo {
path: "report.txt".to_string(),
mime: "text/plain".to_string(),
content_md5: "a".repeat(32),
content_sha256: "b".repeat(64),
bytes: 5,
}],
})
.unwrap(),
)
.unwrap();
let retry_2 = assets_dir.join("work/retry_2");
std::fs::create_dir_all(&retry_2).unwrap();
std::fs::write(
retry_2.join("manifest.json"),
serde_json::to_string(&AssetCollectionSummary {
files_copied: 1,
total_bytes: 6,
files_skipped: 0,
download_errors: 0,
hash_errors: 0,
captured_assets: vec![CapturedAssetInfo {
path: "report.txt".to_string(),
mime: "text/plain".to_string(),
content_md5: "c".repeat(32),
content_sha256: "d".repeat(64),
bytes: 6,
}],
})
.unwrap(),
)
.unwrap();
let entries = scan_assets(&assets_dir, Some("work"), Some(2)).unwrap();
assert_eq!(entries.len(), 1);
assert_eq!(entries[0].retry, 2);
assert_eq!(entries[0].relative_path, "report.txt");
assert_eq!(entries[0].size, 6);
}
}