Skip to content

Commit 7239ace

Browse files
fix(rig): wire WorkloadSpec.lifecycle and report the lifecycle_snapshots drift class (#10553)
* feat(rig): give WorkloadSpec.lifecycle a reader and repair a snapshot class Closes the two residual gaps in #10317 and #10319 after the pipeline lifecycle step, snapshot state, and shared-path/service repair landed. #10317 — `WorkloadSpec.lifecycle` had zero readers anywhere in the workspace: no `.lifecycle()` call site, no `.lifecycle` field read outside its own accessor, no cross-crate use. It was parsed, defaulted in four test constructors, serialized, documented — and never executed. Wire it rather than delete it: `PipelineStep::Lifecycle` can now take its contract from a `workload` reference instead of an inline payload. Exactly one source is legal, and resolution is fail-closed — unknown extension, unmatched `path`, ambiguous selection, and a workload with no contract all fail before a phase runs. One declaration on the workload now governs every op the rig runs against it, instead of an `up` copy and a `down` copy drifting apart. Inline `lifecycle` becomes optional, which serde no longer rejects at load time, so `rig lint` gains the "exactly one contract source" check plus shape validation of the reference. #10319 — `repair` now reports the `lifecycle_snapshots` class it had no row for. Deliberately report-only: the handle is opaque, so deleting the record does not reap the environment behind it, it only strands one. A handle a declared step still owns is `skipped` with the op that reaps it; a handle no declared step owns is `blocked`. * test(rig): update lifecycle step spec tests for the optional contract body CI caught the one consumer my grep missed: I searched `crates/` for `PipelineStep::Lifecycle` but the `#[path]`-included test modules live under `tests/`, so `tests/core/rig/spec_test.rs` was never in scope. Also extends the round-trip coverage to the `workload` reference form: the reference carries no inline body, and does not grow an empty one on serialize. --------- Co-authored-by: chubes-bot <266378653+homeboy-ci[bot]@users.noreply.github.com>
1 parent 030cacc commit 7239ace

14 files changed

Lines changed: 1342 additions & 33 deletions

File tree

crates/homeboy-rig/src/lib.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -98,14 +98,14 @@ pub use spec::{
9898
DependencyMaterializationStepSpec, DiscoverSpec, ExecutableRequirementSpec,
9999
FilesystemAssertionKind, FilesystemAssertionSpec, LifecycleContract, LifecyclePhaseContract,
100100
LifecyclePhaseKind, LifecyclePhaseResult, LifecyclePhaseStatus, LifecycleResultMetadata,
101-
LifecycleSnapshotRef, NewerThanSpec, NormalizedDependencyMaterializationStep, PatchOp,
102-
PipelineStep, RigRequirementsSpec, RigResourceRetentionSpec, RigResourcesSpec, RigSpec,
103-
RunnerToolRequirementSpec, ServiceKind, ServiceSpec, SharedPathOp, SharedPathSpec, StackOp,
104-
SymlinkSpec, TimeSource, TraceConfig, TraceDependencySpec, TraceExperimentArtifactSpec,
105-
TraceExperimentCommandSpec, TraceExperimentSpec, TraceGuardrailSpec,
106-
TraceNativePublicPreviewSpec, TracePhaseTemplateSpec, TracePreviewAssetFanoutSpec,
107-
TraceProfileSpec, TracePublicPreviewMode, TracePublicPreviewSpec, TraceVariantSpec,
108-
WorkloadSpec, RIG_RESOURCE_CLASSES, RIG_RESOURCE_CLASS_EXCLUSIVE,
101+
LifecycleSnapshotRef, LifecycleWorkloadKind, LifecycleWorkloadRef, NewerThanSpec,
102+
NormalizedDependencyMaterializationStep, PatchOp, PipelineStep, RigRequirementsSpec,
103+
RigResourceRetentionSpec, RigResourcesSpec, RigSpec, RunnerToolRequirementSpec, ServiceKind,
104+
ServiceSpec, SharedPathOp, SharedPathSpec, StackOp, SymlinkSpec, TimeSource, TraceConfig,
105+
TraceDependencySpec, TraceExperimentArtifactSpec, TraceExperimentCommandSpec,
106+
TraceExperimentSpec, TraceGuardrailSpec, TraceNativePublicPreviewSpec, TracePhaseTemplateSpec,
107+
TracePreviewAssetFanoutSpec, TraceProfileSpec, TracePublicPreviewMode, TracePublicPreviewSpec,
108+
TraceVariantSpec, WorkloadSpec, RIG_RESOURCE_CLASSES, RIG_RESOURCE_CLASS_EXCLUSIVE,
109109
RIG_RESOURCE_CLASS_LIFECYCLE_SNAPSHOTS, RIG_RESOURCE_CLASS_PATHS, RIG_RESOURCE_CLASS_PORTS,
110110
RIG_RESOURCE_CLASS_PROCESS_PATTERNS,
111111
};
@@ -124,8 +124,8 @@ pub use workloads::{
124124
extension_workload_inputs, invocation_requirements_for_extension_workloads,
125125
required_component_id_for_workload, required_extension_ids_for_workload,
126126
runner_capabilities_for_extension, trace_dependencies_for_extension,
127-
workload_path_expansions_for_extension, workloads_for_extension, RigExtensionWorkloadInputs,
128-
RigWorkloadKind, RigWorkloadPathExpansion,
127+
workload_lifecycle_contract, workload_path_expansions_for_extension, workloads_for_extension,
128+
RigExtensionWorkloadInputs, RigWorkloadKind, RigWorkloadPathExpansion,
129129
};
130130

131131
use discovery::discover_rigs_for_install;

crates/homeboy-rig/src/lint.rs

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,7 @@ fn portability_failures(root: &Path, files: &[PathBuf]) -> Result<Vec<String>> {
522522
};
523523
failures.extend(shared_path_failures(root, file, &value));
524524
failures.extend(resource_retention_failures(root, file, &value));
525+
failures.extend(lifecycle_step_source_failures(root, file, &value));
525526
}
526527

527528
Ok(failures)
@@ -584,6 +585,87 @@ fn shared_path_failures(root: &Path, file: &Path, rig: &serde_json::Value) -> Ve
584585
failures
585586
}
586587

588+
/// A `kind: "lifecycle"` step takes its contract from exactly one source:
589+
/// an inline `lifecycle` payload, or a `workload` reference.
590+
///
591+
/// Both fields are optional in the schema so either shape parses, which means
592+
/// serde no longer rejects a step that declares neither. The executor still
593+
/// fails closed at run time, but a rig author should not have to run `rig up`
594+
/// to find out — that is what this lint is for.
595+
fn lifecycle_step_source_failures(
596+
root: &Path,
597+
file: &Path,
598+
rig: &serde_json::Value,
599+
) -> Vec<String> {
600+
let rel = display_relative(root, file);
601+
let Some(pipelines) = rig.get("pipeline").and_then(|value| value.as_object()) else {
602+
return Vec::new();
603+
};
604+
605+
let mut failures = Vec::new();
606+
for (name, steps) in pipelines {
607+
let Some(steps) = steps.as_array() else {
608+
continue;
609+
};
610+
for (index, step) in steps.iter().enumerate() {
611+
let Some(step) = step.as_object() else {
612+
continue;
613+
};
614+
if step.get("kind").and_then(|value| value.as_str()) != Some("lifecycle") {
615+
continue;
616+
}
617+
let inline = step.contains_key("lifecycle");
618+
let reference = step.contains_key("workload");
619+
let location = format!("{rel}: pipeline.{name}[{index}] lifecycle step");
620+
match (inline, reference) {
621+
(true, true) => failures.push(format!(
622+
"{location} declares both `lifecycle` and `workload`; use exactly one contract source"
623+
)),
624+
(false, false) => failures.push(format!(
625+
"{location} declares neither `lifecycle` nor `workload`; use exactly one contract source"
626+
)),
627+
_ => {}
628+
}
629+
630+
if let Some(workload) = step.get("workload") {
631+
failures.extend(lifecycle_workload_ref_failures(&location, workload));
632+
}
633+
}
634+
}
635+
failures
636+
}
637+
638+
/// Shape checks for a `workload` reference. Resolution against the declared
639+
/// workload maps stays at run time — lint only rejects a reference that could
640+
/// never resolve for anyone.
641+
fn lifecycle_workload_ref_failures(location: &str, workload: &serde_json::Value) -> Vec<String> {
642+
let Some(workload) = workload.as_object() else {
643+
return vec![format!("{location} `workload` must be an object")];
644+
};
645+
646+
let mut failures = Vec::new();
647+
match workload.get("kind").and_then(|value| value.as_str()) {
648+
Some("bench" | "fuzz" | "trace") => {}
649+
Some(other) => failures.push(format!(
650+
"{location} `workload.kind` must be one of bench, fuzz, trace (found '{other}')"
651+
)),
652+
None => failures.push(format!(
653+
"{location} `workload.kind` is required and must be one of bench, fuzz, trace"
654+
)),
655+
}
656+
let extension = workload
657+
.get("extension")
658+
.and_then(|value| value.as_str())
659+
.unwrap_or("")
660+
.trim();
661+
if extension.is_empty() {
662+
failures.push(format!(
663+
"{location} `workload.extension` is required and must be a non-empty extension id"
664+
));
665+
}
666+
failures
667+
}
668+
587669
/// `delete_after_ttl` without a `ttl` is contract-invalid: the lifecycle record
588670
/// would fail validation and the run's whole lifecycle index would be dropped.
589671
/// Catch it at lint time instead of losing the artifact at runtime.
@@ -1439,4 +1521,134 @@ mod tests {
14391521
assert!(error.contains("fuzz_profile profile missing references write"));
14401522
assert!(error.contains("bench_profile profile missing references slow"));
14411523
}
1524+
1525+
#[test]
1526+
fn package_lint_reports_lifecycle_step_without_a_contract_source() {
1527+
let temp = tempfile::TempDir::new().expect("temp package");
1528+
let rig_dir = temp.path().join("rigs").join("lifecycle");
1529+
fs::create_dir_all(&rig_dir).expect("rig dir");
1530+
fs::write(
1531+
rig_dir.join("rig.json"),
1532+
r#"{
1533+
"id": "lifecycle",
1534+
"pipeline": {
1535+
"up": [{ "kind": "lifecycle", "op": "prepare" }]
1536+
}
1537+
}"#,
1538+
)
1539+
.expect("write rig");
1540+
1541+
let outcome = run_package_lint_at(temp.path()).expect("lint package");
1542+
let step = portability_step(&outcome);
1543+
1544+
assert_eq!(step.status, "fail");
1545+
let error = step.error.as_ref().expect("error");
1546+
assert!(
1547+
error.contains("declares neither `lifecycle` nor `workload`"),
1548+
"{error}"
1549+
);
1550+
assert!(error.contains("pipeline.up[0]"), "{error}");
1551+
}
1552+
1553+
#[test]
1554+
fn package_lint_reports_lifecycle_step_with_two_contract_sources() {
1555+
let temp = tempfile::TempDir::new().expect("temp package");
1556+
let rig_dir = temp.path().join("rigs").join("lifecycle");
1557+
fs::create_dir_all(&rig_dir).expect("rig dir");
1558+
fs::write(
1559+
rig_dir.join("rig.json"),
1560+
r#"{
1561+
"id": "lifecycle",
1562+
"pipeline": {
1563+
"up": [{
1564+
"kind": "lifecycle",
1565+
"lifecycle": { "phases": [] },
1566+
"workload": { "kind": "fuzz", "extension": "generic" }
1567+
}]
1568+
}
1569+
}"#,
1570+
)
1571+
.expect("write rig");
1572+
1573+
let outcome = run_package_lint_at(temp.path()).expect("lint package");
1574+
let step = portability_step(&outcome);
1575+
1576+
assert_eq!(step.status, "fail");
1577+
let error = step.error.as_ref().expect("error");
1578+
assert!(
1579+
error.contains("declares both `lifecycle` and `workload`"),
1580+
"{error}"
1581+
);
1582+
}
1583+
1584+
#[test]
1585+
fn package_lint_reports_malformed_lifecycle_workload_reference() {
1586+
let temp = tempfile::TempDir::new().expect("temp package");
1587+
let rig_dir = temp.path().join("rigs").join("lifecycle");
1588+
fs::create_dir_all(&rig_dir).expect("rig dir");
1589+
fs::write(
1590+
rig_dir.join("rig.json"),
1591+
r#"{
1592+
"id": "lifecycle",
1593+
"pipeline": {
1594+
"up": [{
1595+
"kind": "lifecycle",
1596+
"workload": { "kind": "sandbox", "extension": " " }
1597+
}]
1598+
}
1599+
}"#,
1600+
)
1601+
.expect("write rig");
1602+
1603+
let outcome = run_package_lint_at(temp.path()).expect("lint package");
1604+
let step = portability_step(&outcome);
1605+
1606+
assert_eq!(step.status, "fail");
1607+
let error = step.error.as_ref().expect("error");
1608+
assert!(error.contains("`workload.kind` must be one of"), "{error}");
1609+
assert!(
1610+
error.contains("`workload.extension` is required"),
1611+
"{error}"
1612+
);
1613+
}
1614+
1615+
#[test]
1616+
fn package_lint_accepts_both_lifecycle_contract_sources_used_alone() {
1617+
let temp = tempfile::TempDir::new().expect("temp package");
1618+
let rig_dir = temp.path().join("rigs").join("lifecycle");
1619+
fs::create_dir_all(&rig_dir).expect("rig dir");
1620+
fs::write(
1621+
rig_dir.join("rig.json"),
1622+
r#"{
1623+
"id": "lifecycle",
1624+
"fuzz_workloads": {
1625+
"generic": [{
1626+
"path": "fuzz/a.workload.json",
1627+
"lifecycle": {
1628+
"phases": [{ "id": "make", "phase": "prepare", "command": "true" }]
1629+
}
1630+
}]
1631+
},
1632+
"pipeline": {
1633+
"up": [{
1634+
"kind": "lifecycle",
1635+
"workload": { "kind": "fuzz", "extension": "generic" }
1636+
}],
1637+
"down": [{
1638+
"kind": "lifecycle",
1639+
"op": "teardown",
1640+
"lifecycle": {
1641+
"phases": [{ "id": "reap", "phase": "teardown", "command": "true" }]
1642+
}
1643+
}]
1644+
}
1645+
}"#,
1646+
)
1647+
.expect("write rig");
1648+
1649+
let outcome = run_package_lint_at(temp.path()).expect("lint package");
1650+
let step = portability_step(&outcome);
1651+
1652+
assert_eq!(step.status, "pass", "{:?}", step.error);
1653+
}
14421654
}

crates/homeboy-rig/src/pipeline/lifecycle_step.rs

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ use std::time::Duration;
1616
use super::super::expand::{expand_vars, settings_env};
1717
use super::super::spec::{
1818
LifecycleContract, LifecyclePhaseContract, LifecyclePhaseKind, LifecyclePhaseResult,
19-
LifecyclePhaseStatus, LifecycleResultMetadata, LifecycleSnapshotRef, RigSpec,
19+
LifecyclePhaseStatus, LifecycleResultMetadata, LifecycleSnapshotRef, LifecycleWorkloadRef,
20+
RigSpec,
2021
};
2122
use super::super::state::{now_rfc3339, LifecycleSnapshotState, RigState};
2223
use super::super::toolchain;
@@ -35,14 +36,17 @@ use homeboy_core::server::{
3536
/// names the thing it is holding a handle to.
3637
const DEFAULT_SNAPSHOT_KIND: &str = "lifecycle_snapshot";
3738

39+
#[allow(clippy::too_many_arguments)]
3840
pub(super) fn run_lifecycle_step(
3941
rig: &RigSpec,
4042
step_id: Option<&str>,
4143
component: Option<&str>,
42-
contract: &LifecycleContract,
44+
contract: Option<&LifecycleContract>,
45+
workload: Option<&LifecycleWorkloadRef>,
4346
op: LifecyclePhaseKind,
4447
settings: &[(String, String)],
4548
) -> Result<()> {
49+
let contract = resolve_contract(rig, contract, workload)?;
4650
let (result, failure) = execute_lifecycle_phases(rig, component, contract, op, settings)?;
4751

4852
// Persist before propagating a phase failure: a handle captured before the
@@ -56,8 +60,36 @@ pub(super) fn run_lifecycle_step(
5660
}
5761
}
5862

63+
/// Pick the contract this step executes: the inline payload, or the one a
64+
/// rig-owned workload declares.
65+
///
66+
/// Exactly one source is legal. Declaring both is a spec bug that would leave
67+
/// the reader guessing which contract governs, and declaring neither leaves
68+
/// nothing to run — both fail here, before any phase executes and before any
69+
/// state is touched.
70+
fn resolve_contract<'a>(
71+
rig: &'a RigSpec,
72+
contract: Option<&'a LifecycleContract>,
73+
workload: Option<&LifecycleWorkloadRef>,
74+
) -> Result<&'a LifecycleContract> {
75+
match (contract, workload) {
76+
(Some(contract), None) => Ok(contract),
77+
(None, Some(reference)) => {
78+
super::super::workloads::workload_lifecycle_contract(rig, reference)
79+
}
80+
(Some(_), Some(_)) => Err(step_error(
81+
rig,
82+
"lifecycle step declares both `lifecycle` and `workload`; declare the contract inline or reference a workload, not both",
83+
)),
84+
(None, None) => Err(step_error(
85+
rig,
86+
"lifecycle step declares neither `lifecycle` nor `workload`; provide an inline `homeboy/lifecycle-contract/v1` payload or reference a workload that declares one",
87+
)),
88+
}
89+
}
90+
5991
/// Stable ownership key for the handles a step captures.
60-
fn step_key(step_id: Option<&str>, component: Option<&str>) -> String {
92+
pub(crate) fn step_key(step_id: Option<&str>, component: Option<&str>) -> String {
6193
step_id
6294
.map(str::trim)
6395
.filter(|value| !value.is_empty())

crates/homeboy-rig/src/pipeline/mod.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ use homeboy_core::error::Result;
2828
pub use outcome::{PipelineOutcome, PipelineStepOutcome};
2929

3030
pub(super) use command_step::run_command_step;
31+
/// Ownership key a `lifecycle` step's captured handles are recorded under.
32+
/// Exported so `repair` classifies live handles with the same key the executor
33+
/// writes, instead of a second copy that can drift.
34+
pub(super) use lifecycle_step::step_key as lifecycle_step_key;
3135

3236
pub fn run_pipeline(rig: &RigSpec, name: &str, fail_fast: bool) -> Result<PipelineOutcome> {
3337
run_pipeline_with_settings(rig, name, fail_fast, &[])
@@ -339,13 +343,15 @@ fn run_step(
339343
step_id,
340344
component,
341345
lifecycle,
346+
workload,
342347
op,
343348
..
344349
} => lifecycle_step::run_lifecycle_step(
345350
rig,
346351
step_id.as_deref(),
347352
component.as_deref(),
348-
lifecycle,
353+
lifecycle.as_ref(),
354+
workload.as_ref(),
349355
*op,
350356
settings,
351357
),

0 commit comments

Comments
 (0)