Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 41 additions & 1 deletion crates/homeboy-cli/src/commands/bench/observation/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,53 @@ fn recorded_bench_artifact_reports_unreachable_public_viewer_url() {
artifact.observation_artifact_id.as_deref(),
Some("artifact-1")
);
assert!(artifact.public_url.is_some());
assert_eq!(artifact.public_url, None);
assert!(artifact.viewer_refs.viewer_links.is_empty());
assert_eq!(artifact.viewer_refs.viewer_url, None);
assert_eq!(diagnostic.class, "bench_public_artifact_url_unreachable");
assert_eq!(diagnostic.metadata["status_code"], 404);
}

#[test]
fn recorded_bench_artifact_preserves_legacy_public_viewer_url() {
let public_artifact_base = "https://artifacts.example.test/homeboy";
let _public_artifact_base = EnvGuard::set(
homeboy::core::artifacts::PUBLIC_ARTIFACT_BASE_URL_ENV,
public_artifact_base,
);
let mut artifact = BenchArtifact::default();
let record = ArtifactRecord {
id: "legacy-artifact".to_string(),
run_id: "run-1".to_string(),
kind: "bench_artifact".to_string(),
artifact_type: "file".to_string(),
path: "/tmp/blueprint.after.json".to_string(),
url: None,
public_url: None,
viewer_url: None,
viewer_links: Vec::new(),
sha256: None,
size_bytes: None,
mime: Some("application/json".to_string()),
metadata_json: serde_json::json!({
"viewer": crate::commands::runs::HOSTED_BLUEPRINT_VIEWER.to_metadata(None)
}),
created_at: "2026-06-12T00:00:00Z".to_string(),
};

let diagnostic = apply_recorded_bench_artifact_links(
"cold",
Some(0),
"blueprint.after",
&mut artifact,
&record,
);

assert_eq!(diagnostic, None);
assert!(artifact.public_url.is_some());
assert_eq!(artifact.viewer_refs.viewer_links.len(), 1);
}

pub(super) fn bench_results(component_id: &str, scenario_id: &str, p95: f64) -> BenchResults {
serde_json::from_value(serde_json::json!({
"component_id": component_id,
Expand Down
2 changes: 1 addition & 1 deletion crates/homeboy-cli/src/commands/runs/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1524,7 +1524,7 @@ fn artifacts_command_suppresses_viewer_links_when_public_url_is_unreachable() {
.find(|artifact| artifact.kind == "bench_artifact")
.expect("bench artifact");

assert!(artifact.public_url.is_some());
assert_eq!(artifact.public_url, None);
assert!(artifact.viewer_links.is_empty());
assert_eq!(artifact.viewer_url, None);
assert_eq!(
Expand Down
44 changes: 34 additions & 10 deletions crates/homeboy-core/src/artifact_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,26 +250,29 @@ pub fn cached_validated_viewer_links(
if links.is_empty() {
return links;
}
if artifact
.metadata_json
.get("public_url_validation")
.and_then(|validation| validation.get("reachable"))
.and_then(Value::as_bool)
== Some(true)
{
if public_artifact_url_is_reachable_or_legacy(artifact) {
links
} else {
Vec::new()
}
}

/// Return whether a persisted public URL may be presented to a reviewer.
///
/// Records written before validation was introduced have no validation metadata
/// and retain their persisted links. New records carry validation metadata, so
/// malformed or failed validation suppresses their generated links.
pub fn public_artifact_url_is_reachable_or_legacy(artifact: &ArtifactRecord) -> bool {
match artifact.metadata_json.get("public_url_validation") {
None => true,
Some(validation) => validation.get("reachable").and_then(Value::as_bool) == Some(true),
}
}

pub fn annotate_public_artifact_url_validation(
artifact: &mut ArtifactRecord,
) -> Option<PublicArtifactUrlValidation> {
let public_url = public_artifact_url(artifact)?;
if viewer_links(artifact, Some(&public_url)).is_empty() {
return None;
}
let validation = validate_public_artifact_url(&public_url);
artifact.metadata_json["public_url_validation"] =
public_artifact_url_validation_json(&validation);
Expand Down Expand Up @@ -544,6 +547,27 @@ mod tests {
}
}

#[test]
fn public_url_validation_preserves_legacy_records_and_gates_new_records() {
let legacy = viewer_artifact();
let reachable = ArtifactRecord {
metadata_json: serde_json::json!({
"public_url_validation": { "reachable": true }
}),
..viewer_artifact()
};
let unreachable = ArtifactRecord {
metadata_json: serde_json::json!({
"public_url_validation": { "reachable": false }
}),
..viewer_artifact()
};

assert!(public_artifact_url_is_reachable_or_legacy(&legacy));
assert!(public_artifact_url_is_reachable_or_legacy(&reachable));
assert!(!public_artifact_url_is_reachable_or_legacy(&unreachable));
}

struct EnvGuard {
key: &'static str,
prior: Option<String>,
Expand Down
2 changes: 1 addition & 1 deletion crates/homeboy-core/src/artifacts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub use super::artifact_dom_boxes::{
};
pub use super::artifact_links::{
cached_validated_viewer_links, public_artifact_path_url, public_artifact_url,
PUBLIC_ARTIFACT_BASE_URL_ENV,
public_artifact_url_is_reachable_or_legacy, PUBLIC_ARTIFACT_BASE_URL_ENV,
};
pub use super::artifact_manifest::{
ArtifactManifest, ARTIFACT_MANIFEST_SCHEMA, RUNTIME_AGENT_ARTIFACT_PATHS_SCHEMA,
Expand Down
31 changes: 17 additions & 14 deletions crates/homeboy-core/src/observation/runs_service/artifact_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,19 @@ use super::*;

/// Enrich a single artifact record with public/viewer link metadata.
///
/// Mirrors the original CLI helper exactly: derive a public URL (from
/// stored artifact metadata or by treating the artifact path as the URL
/// for `url`-typed artifacts), then resolve any cached viewer links.
/// Derive a public URL from stored artifact metadata or, for explicitly
/// URL-typed artifacts, from the recorded URL. Generated public URLs are only
/// emitted after the persistence-time probe verified them, so a successful
/// terminal handoff never advertises an already-broken tunnel URL.
pub(crate) fn enrich_artifact_link(mut artifact: ArtifactRecord) -> ArtifactRecord {
let public_url =
public_artifact_url(&artifact).or_else(|| public_url_for_url_artifact(&artifact));
if let Some(url) = public_url.clone() {
if artifact.artifact_type != "url"
&& !crate::artifact_links::public_artifact_url_is_reachable_or_legacy(&artifact)
{
return artifact;
}
artifact.public_url = Some(url.clone());
artifact.viewer_links = cached_validated_viewer_links(&artifact, &url);
artifact.viewer_url = artifact.viewer_links.first().map(|link| link.url.clone());
Expand Down Expand Up @@ -51,13 +57,10 @@ pub fn related_lab_artifacts_for_runner_job(
if run.kind != "runner-exec" {
return Ok(Vec::new());
}
// Resolve the runner-exec run's Lab job id. Prefer authoritative runner
// evidence; fall back to the persisted metadata in either shape.
let Some(job_id) =
runner_evidence::with_runner_evidence(|p| p.mirrored_runner_job_identity(run))
.map(|(_runner_id, job_id)| job_id)
.or_else(|| lab_remote_job_id(run).map(str::to_string))
else {
// The terminal envelope persists the Lab identity. Do not ask a live
// runner from a durable reader: the runner can be gone precisely when its
// already-recorded evidence is needed for review.
let Some(job_id) = lab_remote_job_id(run).map(str::to_string) else {
return Ok(Vec::new());
};
let mut artifacts = Vec::new();
Expand All @@ -83,15 +86,15 @@ pub fn related_lab_artifacts_for_runner_job(

/// List the enriched artifact records attached to a run.
///
/// Side-effect ordering matches the CLI: refresh mirrored daemon evidence,
/// then index nested publication artifact refs, then list and enrich.
/// This reader only consults the durable local store. Runner reconciliation and
/// remote manifest indexing are explicit operations, so a disconnected or
/// stuck runner cannot block an artifact receipt, evidence report, or terminal
/// handoff.
pub fn list_artifacts_for_run(
store: &ObservationStore,
run_id: &str,
) -> Result<Vec<ArtifactRecord>> {
let run = require_run(store, run_id)?;
refresh_mirrored_daemon_evidence_best_effort(&run.id);
crate::artifacts::index_remote_published_artifact_refs_for_run(store, &run.id)?;
let artifacts = store.list_artifacts(&run.id)?;
Ok(enrich_artifact_links(artifacts))
}
Expand Down
9 changes: 9 additions & 0 deletions crates/homeboy-core/src/observation/runs_service/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,11 @@ fn require_run_reads_terminal_lab_review_alias_while_runner_probe_is_stalled() {
store
.import_run(&run)
.expect("persist terminal Lab review run");
let artifact_path = _home.path().join("terminal-evidence.json");
std::fs::write(&artifact_path, br#"{"ok":true}"#).expect("artifact bytes");
store
.record_artifact(&run.id, "terminal_evidence", &artifact_path)
.expect("persist terminal evidence");

let (entered_tx, entered_rx) = mpsc::channel();
let (release_tx, release_rx) = mpsc::channel();
Expand All @@ -707,6 +712,10 @@ fn require_run_reads_terminal_lab_review_alias_while_runner_probe_is_stalled() {
let resolved = require_run(&store, label).expect("durable local terminal record");
assert_eq!(resolved.id, run.id);
assert_eq!(resolved.status, RunStatus::Fail.as_str());
let artifacts = list_artifacts_for_run(&store, &run.id)
.expect("durable artifact reader must not wait for the runner");
assert_eq!(artifacts.len(), 1);
assert_eq!(artifacts[0].kind, "terminal_evidence");
assert!(
!stalled.is_finished(),
"lookup completed before probe release"
Expand Down
35 changes: 18 additions & 17 deletions crates/homeboy-extension/src/bench/artifact_persistence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,25 +353,26 @@ pub fn apply_recorded_bench_artifact_links(
) -> Option<BenchDiagnostic> {
artifact.observation_artifact_id = Some(record.id.clone());
let public_url = artifact_links::public_artifact_url(record)?;
artifact.public_url = Some(public_url.clone());
artifact.viewer_refs.viewer_links =
artifact_links::cached_validated_viewer_links(record, &public_url);
artifact.viewer_refs.viewer_url = artifact
.viewer_refs
.viewer_links
.first()
.map(|link| link.url.clone());
let validation = record.metadata_json.get("public_url_validation")?;
let reachable = validation
.get("reachable")
.and_then(serde_json::Value::as_bool)
.unwrap_or(false);
(!reachable).then(|| {
if artifact_links::public_artifact_url_is_reachable_or_legacy(record) {
artifact.public_url = Some(public_url.clone());
artifact.viewer_refs.viewer_links =
artifact_links::cached_validated_viewer_links(record, &public_url);
artifact.viewer_refs.viewer_url = artifact
.viewer_refs
.viewer_links
.first()
.map(|link| link.url.clone());
None
} else {
let validation = record
.metadata_json
.get("public_url_validation")
.expect("unreachable new artifact has validation metadata");
let error = validation
.get("error")
.and_then(serde_json::Value::as_str)
.unwrap_or("public artifact URL was not reachable");
bench_artifact_diagnostic(
Some(bench_artifact_diagnostic(
scenario_id,
run_index,
name,
Expand All @@ -384,8 +385,8 @@ pub fn apply_recorded_bench_artifact_links(
"status_code": validation.get("status_code").cloned().unwrap_or(serde_json::Value::Null),
"error": validation.get("error").cloned().unwrap_or(serde_json::Value::Null),
}),
)
})
))
}
}

fn bench_artifact_metadata(
Expand Down
2 changes: 2 additions & 0 deletions tests/self_checks_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ fn lint_args(root: &Path) -> LintArgs {
force: false,
setting_args: SettingArgs::default(),
baseline_args: BaselineArgs::default(),
release_readiness_source: None,
json_summary: false,
}
}
Expand All @@ -69,6 +70,7 @@ fn test_args(root: &Path) -> TestArgs {
changed: LabChangedScopeArgs::default(),
ci_job: None,
setting_args: SettingArgs::default(),
release_readiness_source: None,
args: Vec::new(),
json_summary: false,
restore_checkout: false,
Expand Down
Loading