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 4

refactor(artifacts): remove obsolete artifact storage flag

Browse source 
Drop the dead artifact storage capability split from run records,
run.created events, and workflow/server create paths. Worker artifact
upload is now unconditional, and tests/snapshots no longer encode a
legacy object-backed distinction.
This commit is contained in:
Bryan Helmkamp 2026-04-07 23:01:33 -04:00
parent 77c0476b5d
commit 38cb2ec250
25 changed files with 35 additions and 114 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
lib/crates/fabro-checkpoint/src/metadata.rs
View file

@ -214,7 +214,6 @@ mod tests {
repo_origin_url: None,
base_branch: None,
labels: HashMap::new(),
artifact_storage: None,
provenance: None,
}
}

36
lib/crates/fabro-cli/src/commands/run/runner.rs
View file

@ -63,12 +63,8 @@ pub(crate) async fn execute(
.run
.as_ref()
.ok_or_else(|| anyhow!("Run {run_id} has no run record in store"))?;
let artifact_uploader = build_artifact_uploader(
run_id,
run_record,
client.clone_for_reuse(),
artifact_upload_token,
);
let artifact_uploader =
build_artifact_uploader(run_id, client.clone_for_reuse(), artifact_upload_token);
let interviewer = Arc::new(ControlInterviewer::new());
tokio::spawn(read_worker_control_stream(
io::stdin(),
@ -146,14 +142,9 @@ async fn apply_worker_control_line(interviewer: &ControlInterviewer, line: &str)
fn build_artifact_uploader(
run_id: RunId,
run_record: &fabro_types::RunRecord,
client: server_client::ServerStoreClient,
artifact_upload_token: Option<String>,
) -> Option<Arc<dyn StageArtifactUploader>> {
if !run_record.uses_object_backed_artifacts() {
return None;
}
let uploader: Arc<dyn StageArtifactUploader> = match artifact_upload_token {
Some(token) => Arc::new(HttpArtifactUploader {
run_id,
@ -484,8 +475,9 @@ mod tests {
use serde_json::json;
use super::{
WorkerTitlePhase, apply_worker_control_line, execute, initial_worker_title_phase,
read_worker_control_stream, worker_title, worker_title_phase_for_event,
WorkerTitlePhase, apply_worker_control_line, build_artifact_uploader, execute,
initial_worker_title_phase, read_worker_control_stream, worker_title,
worker_title_phase_for_event,
};
use crate::args::RunWorkerMode;
use fabro_interview::{AnswerValue, ControlInterviewer, Interviewer, Question, QuestionType};
@ -588,6 +580,24 @@ mod tests {
);
}
#[tokio::test]
async fn build_artifact_uploader_does_not_depend_on_run_capability_flag() {
let server = MockServer::start_async().await;
let uploader = build_artifact_uploader(
fixtures::RUN_1,
crate::server_client::connect_server_target_direct(&format!(
"{}/api/v1",
server.base_url()
))
.await
.unwrap(),
Some("token".to_string()),
);
assert!(uploader.is_some());
}
#[tokio::test]
async fn worker_bootstrap_loads_run_state_without_prefetching_run_events() {
let server = MockServer::start_async().await;

2
lib/crates/fabro-cli/src/commands/store/dump.rs
View file

@ -342,7 +342,6 @@ mod tests {
repo_origin_url: Some("https://github.com/fabro-sh/fabro".to_string()),
base_branch: Some("main".to_string()),
labels: HashMap::from([("team".to_string(), "infra".to_string())]),
artifact_storage: None,
provenance: None,
}
}
@ -488,7 +487,6 @@ mod tests {
base_branch: run_record.base_branch.clone(),
workflow_slug: run_record.workflow_slug.clone(),
db_prefix: None,
artifact_storage: run_record.artifact_storage,
provenance: run_record.provenance.clone(),
},
)

1
lib/crates/fabro-cli/tests/it/cmd/attach.rs
View file

@ -567,7 +567,6 @@ fn attach_json_errors_without_prompting_for_human_input() {
"event": "run.created",
"id": "[EVENT_ID]",
"properties": {
"artifact_storage": "object_store_v1",
"graph": {
"attrs": {
"goal": {

1
lib/crates/fabro-cli/tests/it/cmd/run.rs
View file

@ -646,7 +646,6 @@ fn json_run_implies_auto_approve_for_human_gates() {
"event": "run.created",
"id": "[EVENT_ID]",
"properties": {
"artifact_storage": "object_store_v1",
"graph": {
"attrs": {
"goal": {

1
lib/crates/fabro-server/src/run_manifest.rs
View file

@ -131,7 +131,6 @@ pub(crate) fn create_run_input(prepared: PreparedManifest) -> CreateRunInput {
.as_ref()
.map(|git| fabro_github::normalize_repo_origin_url(&git.origin_url)),
base_branch: prepared.git.as_ref().map(|git| git.branch.clone()),
artifact_storage: None,
provenance: None,
}
}

39
lib/crates/fabro-server/src/server.rs
View file

@ -34,9 +34,9 @@ use fabro_store::{
ArtifactStore, Database, EventEnvelope, EventPayload, PendingInterviewRecord, StageId,
};
use fabro_types::{
EventBody, InterviewQuestionRecord, InterviewQuestionType, RunArtifactStorage, RunBlobId,
RunClientProvenance, RunControlAction, RunEvent, RunId, RunProvenance, RunServerProvenance,
RunSubjectProvenance, Settings,
EventBody, InterviewQuestionRecord, InterviewQuestionType, RunBlobId, RunClientProvenance,
RunControlAction, RunEvent, RunId, RunProvenance, RunServerProvenance, RunSubjectProvenance,
Settings,
};
use fabro_util::redact::redact_jsonl_line;
use fabro_util::version::FABRO_VERSION;
@ -3097,7 +3097,6 @@ async fn create_run(
let mut create_input = run_manifest::create_run_input(prepared.clone());
create_input.run_id = Some(run_id);
create_input.artifact_storage = Some(RunArtifactStorage::ObjectStoreV1);
create_input.provenance = Some(run_provenance(&headers, &subject));
let created = match Box::pin(operations::create(state.store.as_ref(), create_input)).await {
@ -5856,7 +5855,7 @@ mod tests {
run_id
}
async fn create_legacy_run(state: &Arc<AppState>, settings: &Settings) -> RunId {
async fn create_direct_run(state: &Arc<AppState>, settings: &Settings) -> RunId {
operations::create(
state.store.as_ref(),
operations::CreateRunInput {
@ -5873,7 +5872,6 @@ mod tests {
host_repo_path: None,
repo_origin_url: None,
base_branch: None,
artifact_storage: None,
provenance: None,
},
)
@ -6580,7 +6578,7 @@ mod tests {
}
#[tokio::test]
async fn create_run_marks_object_backed_artifacts() {
async fn create_run_persists_run_record() {
let state = create_app_state();
let app = build_router(Arc::clone(&state), AuthMode::Disabled);
@ -6597,13 +6595,7 @@ mod tests {
.await
.unwrap();
assert!(
run_state
.run
.as_ref()
.unwrap()
.uses_object_backed_artifacts()
);
assert!(run_state.run.is_some());
}
#[tokio::test]
@ -6768,14 +6760,14 @@ mod tests {
}
#[tokio::test]
async fn legacy_runs_do_not_fallback_to_scratch_artifacts() {
async fn directly_created_runs_do_not_fallback_to_scratch_artifacts() {
let temp = tempfile::tempdir().unwrap();
let mut settings = dry_run_settings();
settings.storage_dir = Some(temp.path().join("storage"));
let state = create_app_state_with_options(settings.clone(), 5);
let app = build_router(Arc::clone(&state), AuthMode::Disabled);
let run_id = create_legacy_run(&state, &settings).await;
let run_id = create_direct_run(&state, &settings).await;
let artifact_path = Storage::new(settings.storage_dir())
.run_scratch(&run_id)
.artifact_files_dir()
@ -6784,21 +6776,6 @@ mod tests {
.join("src/lib.rs");
std::fs::create_dir_all(artifact_path.parent().unwrap()).unwrap();
std::fs::write(&artifact_path, "legacy scratch only").unwrap();
let run_state = state
.store
.open_run_reader(&run_id)
.await
.unwrap()
.state()
.await
.unwrap();
assert!(
!run_state
.run
.as_ref()
.unwrap()
.uses_object_backed_artifacts()
);
let req = Request::builder()
.method("GET")
.uri(api(&format!("/runs/{run_id}/stages/code@2/artifacts")))

1
lib/crates/fabro-store/src/run_state.rs
View file

@ -93,7 +93,6 @@ impl RunProjection {
repo_origin_url: props.repo_origin_url.clone(),
base_branch: props.base_branch.clone(),
labels,
artifact_storage: props.artifact_storage,
provenance: props.provenance.clone(),
});
self.graph_source.clone_from(&props.workflow_source);

1
lib/crates/fabro-store/src/slate/mod.rs
View file

@ -288,7 +288,6 @@ mod tests {
repo_origin_url: Some("https://github.com/fabro-sh/fabro".to_string()),
base_branch: Some("main".to_string()),
labels: std::collections::HashMap::from([("team".to_string(), "infra".to_string())]),
artifact_storage: None,
provenance: None,
}
}

4
lib/crates/fabro-types/src/lib.rs
View file

@ -45,8 +45,8 @@ pub use retro::{
OpenItemKind, Retro, RetroNarrative, SmoothnessRating, StageRetro,
};
pub use run::{
RunArtifactStorage, RunAuthMethod, RunClientProvenance, RunProvenance, RunRecord,
RunServerProvenance, RunSubjectProvenance,
RunAuthMethod, RunClientProvenance, RunProvenance, RunRecord, RunServerProvenance,
RunSubjectProvenance,
};
pub use run_blob_id::RunBlobId;
pub use run_event::{EventBody, RunEvent, RunNoticeLevel};

18
lib/crates/fabro-types/src/run.rs
View file

@ -48,12 +48,6 @@ pub struct RunProvenance {
pub subject: Option<RunSubjectProvenance>,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub enum RunArtifactStorage {
ObjectStoreV1,
}
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct RunRecord {
pub run_id: RunId,
@ -71,17 +65,5 @@ pub struct RunRecord {
#[serde(default, skip_serializing_if = "HashMap::is_empty")]
pub labels: HashMap<String, String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub artifact_storage: Option<RunArtifactStorage>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub provenance: Option<RunProvenance>,
}
impl RunRecord {
#[must_use]
pub fn uses_object_backed_artifacts(&self) -> bool {
matches!(
self.artifact_storage,
Some(RunArtifactStorage::ObjectStoreV1)
)
}
}

4
lib/crates/fabro-types/src/run_event/run.rs
View file

@ -2,7 +2,7 @@ use std::collections::BTreeMap;
use serde::{Deserialize, Serialize};
use crate::{Graph, RunArtifactStorage, RunControlAction, RunProvenance, Settings, StatusReason};
use crate::{Graph, RunControlAction, RunProvenance, Settings, StatusReason};
use super::{BilledTokenCounts, RunNoticeLevel};
@ -29,8 +29,6 @@ pub struct RunCreatedProps {
#[serde(default, skip_serializing_if = "Option::is_none")]
pub db_prefix: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub artifact_storage: Option<RunArtifactStorage>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub provenance: Option<RunProvenance>,
}

1
lib/crates/fabro-workflow/src/artifact_snapshot.rs
View file

@ -352,7 +352,6 @@ mod tests {
use super::*;
use fabro_agent::sandbox::ExecResult;
use std::collections::HashMap;
use std::fs;
/// Minimal mock sandbox for artifact_snapshot tests.
struct AssetMockSandbox {

4
lib/crates/fabro-workflow/src/event.rs
View file

@ -54,8 +54,6 @@ pub enum Event {
#[serde(default, skip_serializing_if = "Option::is_none")]
db_prefix: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
artifact_storage: Option<::fabro_types::RunArtifactStorage>,
#[serde(default, skip_serializing_if = "Option::is_none")]
provenance: Option<::fabro_types::RunProvenance>,
},
WorkflowRunStarted {
@ -1390,7 +1388,6 @@ fn event_body_from_event(event: &Event) -> EventBody {
base_branch,
workflow_slug,
db_prefix,
artifact_storage,
provenance,
..
} => EventBody::RunCreated(fabro_types::RunCreatedProps {
@ -1406,7 +1403,6 @@ fn event_body_from_event(event: &Event) -> EventBody {
base_branch: base_branch.clone(),
workflow_slug: workflow_slug.clone(),
db_prefix: db_prefix.clone(),
artifact_storage: *artifact_storage,
provenance: provenance.clone(),
}),
Event::WorkflowRunStarted {

15
lib/crates/fabro-workflow/src/operations/create.rs
View file

@ -3,7 +3,7 @@ use fabro_graphviz::graph::{AttrValue, Graph};
use fabro_model::{Catalog, Provider};
use fabro_sandbox::SandboxProvider;
use fabro_store::Database;
use fabro_types::{RunArtifactStorage, RunId, RunProvenance, Settings};
use fabro_types::{RunId, RunProvenance, Settings};
use std::collections::BTreeMap;
use std::collections::HashMap;
use std::path::{Path, PathBuf};
@ -35,7 +35,6 @@ pub struct CreateRunInput {
pub host_repo_path: Option<String>,
pub repo_origin_url: Option<String>,
pub base_branch: Option<String>,
pub artifact_storage: Option<RunArtifactStorage>,
pub provenance: Option<RunProvenance>,
}
@ -57,7 +56,6 @@ struct PersistCreateOptions {
working_directory: PathBuf,
host_repo_path: Option<String>,
repo_origin_url: Option<String>,
artifact_storage: Option<RunArtifactStorage>,
provenance: Option<RunProvenance>,
}
@ -85,7 +83,6 @@ pub async fn create(store: &Database, request: CreateRunInput) -> Result<Created
host_repo_path,
repo_origin_url,
base_branch,
artifact_storage,
provenance,
} = request;
@ -124,7 +121,6 @@ pub async fn create(store: &Database, request: CreateRunInput) -> Result<Created
working_directory,
host_repo_path,
repo_origin_url,
artifact_storage,
provenance,
},
current_dir,
@ -191,7 +187,6 @@ async fn persist_created_run(
base_branch: record.base_branch.clone(),
workflow_slug: record.workflow_slug.clone(),
db_prefix: None,
artifact_storage: record.artifact_storage,
provenance: record.provenance.clone(),
},
record.run_id.created_at(),
@ -324,7 +319,6 @@ fn persist_validated(
working_directory,
host_repo_path,
repo_origin_url,
artifact_storage,
provenance,
} = options;
@ -343,7 +337,6 @@ fn persist_validated(
repo_origin_url,
base_branch,
labels,
artifact_storage,
provenance,
};
@ -703,7 +696,6 @@ mod tests {
host_repo_path: None,
repo_origin_url: None,
base_branch: None,
artifact_storage: None,
provenance: None,
},
)
@ -752,7 +744,6 @@ mod tests {
host_repo_path: Some(dir.path().display().to_string()),
repo_origin_url: None,
base_branch: Some("main".to_string()),
artifact_storage: None,
provenance: None,
},
)
@ -833,7 +824,6 @@ mod tests {
host_repo_path: None,
repo_origin_url: None,
base_branch: None,
artifact_storage: None,
provenance: None,
},
)
@ -877,7 +867,6 @@ mod tests {
host_repo_path: None,
repo_origin_url: Some("https://github.com/acme/widgets".to_string()),
base_branch: None,
artifact_storage: None,
provenance: None,
},
)
@ -918,7 +907,6 @@ mod tests {
host_repo_path: None,
repo_origin_url: None,
base_branch: None,
artifact_storage: None,
provenance: None,
},
)
@ -961,7 +949,6 @@ mod tests {
host_repo_path: None,
repo_origin_url: None,
base_branch: None,
artifact_storage: None,
provenance: Some(fabro_types::RunProvenance {
server: Some(fabro_types::RunServerProvenance {
version: "0.9.0".to_string(),

2
lib/crates/fabro-workflow/src/operations/rebuild_meta.rs
View file

@ -378,7 +378,6 @@ mod tests {
repo_origin_url: None,
base_branch: None,
labels: HashMap::new(),
artifact_storage: None,
provenance: None,
}
}
@ -453,7 +452,6 @@ mod tests {
base_branch: run_record.base_branch.clone(),
workflow_slug: run_record.workflow_slug.clone(),
db_prefix: None,
artifact_storage: run_record.artifact_storage,
provenance: run_record.provenance.clone(),
},
)

1
lib/crates/fabro-workflow/src/operations/start.rs
View file

@ -846,7 +846,6 @@ mod tests {
host_repo_path: None,
repo_origin_url: None,
base_branch: None,
artifact_storage: None,
provenance: None,
},
)

1
lib/crates/fabro-workflow/src/pipeline/execute/tests.rs
View file

@ -144,7 +144,6 @@ fn persisted_workflow(graph: Graph, source: String, run_dir: &Path, run_id: RunI
repo_origin_url: None,
base_branch: Some("main".to_string()),
labels: HashMap::new(),
artifact_storage: None,
provenance: None,
},
)

1
lib/crates/fabro-workflow/src/pipeline/initialize.rs
View file

@ -749,7 +749,6 @@ mod tests {
repo_origin_url: None,
base_branch: Some("main".to_string()),
labels: HashMap::new(),
artifact_storage: None,
provenance: None,
},
)

2
lib/crates/fabro-workflow/src/pipeline/persist.rs
View file

@ -133,7 +133,6 @@ mod tests {
("env".to_string(), "test".to_string()),
("team".to_string(), "workflow".to_string()),
]),
artifact_storage: None,
provenance: None,
}
}
@ -158,7 +157,6 @@ mod tests {
base_branch: record.base_branch.clone(),
workflow_slug: record.workflow_slug.clone(),
db_prefix: None,
artifact_storage: record.artifact_storage,
provenance: record.provenance.clone(),
},
)

6
lib/crates/fabro-workflow/src/pipeline/pull_request.rs
View file

@ -1090,7 +1090,6 @@ mod tests {
repo_origin_url: None,
base_branch: Some("main".to_string()),
labels: HashMap::new(),
artifact_storage: None,
provenance: None,
};
append_event(
@ -1110,7 +1109,6 @@ mod tests {
base_branch: run_record.base_branch.clone(),
workflow_slug: run_record.workflow_slug.clone(),
db_prefix: None,
artifact_storage: run_record.artifact_storage,
provenance: run_record.provenance.clone(),
},
)
@ -1162,7 +1160,6 @@ mod tests {
repo_origin_url: None,
base_branch: Some("main".to_string()),
labels: HashMap::new(),
artifact_storage: None,
provenance: None,
};
append_event(
@ -1182,7 +1179,6 @@ mod tests {
base_branch: run_record.base_branch.clone(),
workflow_slug: run_record.workflow_slug.clone(),
db_prefix: None,
artifact_storage: run_record.artifact_storage,
provenance: run_record.provenance.clone(),
},
)
@ -1387,7 +1383,6 @@ mod tests {
repo_origin_url: None,
base_branch: None,
labels: std::collections::HashMap::new(),
artifact_storage: None,
provenance: None,
};
append_event(
@ -1407,7 +1402,6 @@ mod tests {
base_branch: None,
workflow_slug: None,
db_prefix: None,
artifact_storage: run_record.artifact_storage,
provenance: run_record.provenance.clone(),
},
)

2
lib/crates/fabro-workflow/src/pipeline/retro.rs
View file

@ -241,7 +241,6 @@ mod tests {
repo_origin_url: None,
base_branch: None,
labels: std::collections::HashMap::new(),
artifact_storage: None,
provenance: None,
};
append_event(
@ -261,7 +260,6 @@ mod tests {
base_branch: None,
workflow_slug: None,
db_prefix: None,
artifact_storage: run_record.artifact_storage,
provenance: run_record.provenance.clone(),
},
)

2
lib/crates/fabro-workflow/src/run_lookup.rs
View file

@ -423,7 +423,6 @@ mod tests {
repo_origin_url: None,
base_branch: Some("main".to_string()),
labels: HashMap::new(),
artifact_storage: None,
provenance: None,
}
}
@ -454,7 +453,6 @@ mod tests {
base_branch: run_record.base_branch.clone(),
workflow_slug: run_record.workflow_slug.clone(),
db_prefix: None,
artifact_storage: run_record.artifact_storage,
provenance: run_record.provenance.clone(),
},
)

2
lib/crates/fabro-workflow/src/runtime_store.rs
View file

@ -140,7 +140,6 @@ mod tests {
repo_origin_url: None,
base_branch: None,
labels: HashMap::new(),
artifact_storage: None,
provenance: None,
}
}
@ -166,7 +165,6 @@ mod tests {
base_branch: None,
workflow_slug: Some("test".to_string()),
db_prefix: None,
artifact_storage: None,
provenance: None,
},
)

1
lib/crates/fabro-workflow/src/test_support.rs
View file

@ -97,7 +97,6 @@ async fn initialized(
base_branch: run_options.base_branch.clone(),
workflow_slug: run_options.workflow_slug.clone(),
db_prefix: None,
artifact_storage: None,
provenance: None,
},
)