Skip to content

Commit 28cb73e

Browse files
author
Chris Huber
committed
fix(runs): keep terminal artifact reads local
Terminal artifact and evidence readers now use only persisted controller state, while generated public URLs require recorded reachability before emission. AI assistance: OpenAI gpt-5.6-sol via OpenCode was used to trace the artifact-reader and publication paths, implement the bounded-read invariant, and run focused verification. Chris Huber remains responsible for every line.
1 parent 010df5d commit 28cb73e

4 files changed

Lines changed: 46 additions & 27 deletions

File tree

crates/homeboy-cli/src/commands/runs/tests/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1524,7 +1524,7 @@ fn artifacts_command_suppresses_viewer_links_when_public_url_is_unreachable() {
15241524
.find(|artifact| artifact.kind == "bench_artifact")
15251525
.expect("bench artifact");
15261526

1527-
assert!(artifact.public_url.is_some());
1527+
assert_eq!(artifact.public_url, None);
15281528
assert!(artifact.viewer_links.is_empty());
15291529
assert_eq!(artifact.viewer_url, None);
15301530
assert_eq!(

crates/homeboy-core/src/observation/runs_service/artifact_links.rs

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,24 @@ use super::*;
22

33
/// Enrich a single artifact record with public/viewer link metadata.
44
///
5-
/// Mirrors the original CLI helper exactly: derive a public URL (from
6-
/// stored artifact metadata or by treating the artifact path as the URL
7-
/// for `url`-typed artifacts), then resolve any cached viewer links.
5+
/// Derive a public URL from stored artifact metadata or, for explicitly
6+
/// URL-typed artifacts, from the recorded URL. Generated public URLs are only
7+
/// emitted after the persistence-time probe verified them, so a successful
8+
/// terminal handoff never advertises an already-broken tunnel URL.
89
pub(crate) fn enrich_artifact_link(mut artifact: ArtifactRecord) -> ArtifactRecord {
910
let public_url =
1011
public_artifact_url(&artifact).or_else(|| public_url_for_url_artifact(&artifact));
1112
if let Some(url) = public_url.clone() {
13+
if artifact.artifact_type != "url"
14+
&& artifact
15+
.metadata_json
16+
.get("public_url_validation")
17+
.and_then(|validation| validation.get("reachable"))
18+
.and_then(Value::as_bool)
19+
!= Some(true)
20+
{
21+
return artifact;
22+
}
1223
artifact.public_url = Some(url.clone());
1324
artifact.viewer_links = cached_validated_viewer_links(&artifact, &url);
1425
artifact.viewer_url = artifact.viewer_links.first().map(|link| link.url.clone());
@@ -51,13 +62,10 @@ pub fn related_lab_artifacts_for_runner_job(
5162
if run.kind != "runner-exec" {
5263
return Ok(Vec::new());
5364
}
54-
// Resolve the runner-exec run's Lab job id. Prefer authoritative runner
55-
// evidence; fall back to the persisted metadata in either shape.
56-
let Some(job_id) =
57-
runner_evidence::with_runner_evidence(|p| p.mirrored_runner_job_identity(run))
58-
.map(|(_runner_id, job_id)| job_id)
59-
.or_else(|| lab_remote_job_id(run).map(str::to_string))
60-
else {
65+
// The terminal envelope persists the Lab identity. Do not ask a live
66+
// runner from a durable reader: the runner can be gone precisely when its
67+
// already-recorded evidence is needed for review.
68+
let Some(job_id) = lab_remote_job_id(run).map(str::to_string) else {
6169
return Ok(Vec::new());
6270
};
6371
let mut artifacts = Vec::new();
@@ -83,15 +91,15 @@ pub fn related_lab_artifacts_for_runner_job(
8391

8492
/// List the enriched artifact records attached to a run.
8593
///
86-
/// Side-effect ordering matches the CLI: refresh mirrored daemon evidence,
87-
/// then index nested publication artifact refs, then list and enrich.
94+
/// This reader only consults the durable local store. Runner reconciliation and
95+
/// remote manifest indexing are explicit operations, so a disconnected or
96+
/// stuck runner cannot block an artifact receipt, evidence report, or terminal
97+
/// handoff.
8898
pub fn list_artifacts_for_run(
8999
store: &ObservationStore,
90100
run_id: &str,
91101
) -> Result<Vec<ArtifactRecord>> {
92102
let run = require_run(store, run_id)?;
93-
refresh_mirrored_daemon_evidence_best_effort(&run.id);
94-
crate::artifacts::index_remote_published_artifact_refs_for_run(store, &run.id)?;
95103
let artifacts = store.list_artifacts(&run.id)?;
96104
Ok(enrich_artifact_links(artifacts))
97105
}

crates/homeboy-core/src/observation/runs_service/tests.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,11 @@ fn require_run_reads_terminal_lab_review_alias_while_runner_probe_is_stalled() {
690690
store
691691
.import_run(&run)
692692
.expect("persist terminal Lab review run");
693+
let artifact_path = _home.path().join("terminal-evidence.json");
694+
std::fs::write(&artifact_path, br#"{"ok":true}"#).expect("artifact bytes");
695+
store
696+
.record_artifact(&run.id, "terminal_evidence", &artifact_path)
697+
.expect("persist terminal evidence");
693698

694699
let (entered_tx, entered_rx) = mpsc::channel();
695700
let (release_tx, release_rx) = mpsc::channel();
@@ -707,6 +712,10 @@ fn require_run_reads_terminal_lab_review_alias_while_runner_probe_is_stalled() {
707712
let resolved = require_run(&store, label).expect("durable local terminal record");
708713
assert_eq!(resolved.id, run.id);
709714
assert_eq!(resolved.status, RunStatus::Fail.as_str());
715+
let artifacts = list_artifacts_for_run(&store, &run.id)
716+
.expect("durable artifact reader must not wait for the runner");
717+
assert_eq!(artifacts.len(), 1);
718+
assert_eq!(artifacts[0].kind, "terminal_evidence");
710719
assert!(
711720
!stalled.is_finished(),
712721
"lookup completed before probe release"

crates/homeboy-extension/src/bench/artifact_persistence.rs

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -353,25 +353,27 @@ pub fn apply_recorded_bench_artifact_links(
353353
) -> Option<BenchDiagnostic> {
354354
artifact.observation_artifact_id = Some(record.id.clone());
355355
let public_url = artifact_links::public_artifact_url(record)?;
356-
artifact.public_url = Some(public_url.clone());
357-
artifact.viewer_refs.viewer_links =
358-
artifact_links::cached_validated_viewer_links(record, &public_url);
359-
artifact.viewer_refs.viewer_url = artifact
360-
.viewer_refs
361-
.viewer_links
362-
.first()
363-
.map(|link| link.url.clone());
364356
let validation = record.metadata_json.get("public_url_validation")?;
365357
let reachable = validation
366358
.get("reachable")
367359
.and_then(serde_json::Value::as_bool)
368360
.unwrap_or(false);
369-
(!reachable).then(|| {
361+
if reachable {
362+
artifact.public_url = Some(public_url.clone());
363+
artifact.viewer_refs.viewer_links =
364+
artifact_links::cached_validated_viewer_links(record, &public_url);
365+
artifact.viewer_refs.viewer_url = artifact
366+
.viewer_refs
367+
.viewer_links
368+
.first()
369+
.map(|link| link.url.clone());
370+
None
371+
} else {
370372
let error = validation
371373
.get("error")
372374
.and_then(serde_json::Value::as_str)
373375
.unwrap_or("public artifact URL was not reachable");
374-
bench_artifact_diagnostic(
376+
Some(bench_artifact_diagnostic(
375377
scenario_id,
376378
run_index,
377379
name,
@@ -384,8 +386,8 @@ pub fn apply_recorded_bench_artifact_links(
384386
"status_code": validation.get("status_code").cloned().unwrap_or(serde_json::Value::Null),
385387
"error": validation.get("error").cloned().unwrap_or(serde_json::Value::Null),
386388
}),
387-
)
388-
})
389+
))
390+
}
389391
}
390392

391393
fn bench_artifact_metadata(

0 commit comments

Comments
 (0)