Skip to content

Commit 46303d7

Browse files
committed
refactor(hm-pipeline-ir): timeout_seconds uses Option<u32> where 0 is a meaningless / footgun value
1 parent 904721f commit 46303d7

2 files changed

Lines changed: 27 additions & 6 deletions

File tree

crates/hm-pipeline-ir/src/graph.rs

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::collections::BTreeMap;
2+
use std::num::NonZeroU32;
23

34
use daggy::Dag;
45

@@ -28,8 +29,10 @@ pub struct CommandStep {
2829
#[serde(default)]
2930
pub env: Option<BTreeMap<String, String>>,
3031
/// Maximum wall-clock seconds before the step is killed.
32+
///
33+
/// `NonZeroU32`: a `0`-second budget is rejected at the wire boundary.
3134
#[serde(default)]
32-
pub timeout_seconds: Option<u32>,
35+
pub timeout_seconds: Option<NonZeroU32>,
3336
/// Cache configuration for this step's committed snapshot.
3437
#[serde(default)]
3538
pub cache: Option<Cache>,
@@ -88,8 +91,11 @@ pub struct PipelineGraph {
8891
default_image: Option<String>,
8992
/// Whole-build wall-clock budget in seconds. When set, the local
9093
/// orchestrator kills the run and fails it once this elapses.
94+
///
95+
/// `NonZeroU32` makes a `0`-second budget (kill immediately) an
96+
/// unrepresentable, wire-rejected value rather than a runtime footgun.
9197
#[serde(default, skip_serializing_if = "Option::is_none")]
92-
timeout_seconds: Option<u32>,
98+
timeout_seconds: Option<NonZeroU32>,
9399
#[serde(rename = "graph")]
94100
inner: Dag<Transition, EdgeKind>,
95101
}
@@ -112,8 +118,11 @@ impl PipelineGraph {
112118
}
113119

114120
/// Whole-build wall-clock budget in seconds, if the author set one.
121+
///
122+
/// The returned value is positive by construction (`0` is rejected at
123+
/// the wire boundary), so consumers need no `> 0` guard.
115124
#[must_use]
116-
pub const fn timeout_seconds(&self) -> Option<u32> {
125+
pub const fn timeout_seconds(&self) -> Option<NonZeroU32> {
117126
self.timeout_seconds
118127
}
119128

@@ -128,6 +137,8 @@ impl PipelineGraph {
128137
mod timeout_tests {
129138
#![allow(clippy::unwrap_used, clippy::expect_used)]
130139

140+
use std::num::NonZeroU32;
141+
131142
use super::PipelineGraph;
132143

133144
#[test]
@@ -138,7 +149,17 @@ mod timeout_tests {
138149
"graph": {"nodes": [], "node_holes": [], "edge_property": "directed", "edges": []}
139150
}"#;
140151
let g: PipelineGraph = serde_json::from_str(json).unwrap();
141-
assert_eq!(g.timeout_seconds(), Some(1800));
152+
assert_eq!(g.timeout_seconds(), NonZeroU32::new(1800));
153+
}
154+
155+
#[test]
156+
fn rejects_zero_pipeline_timeout_seconds() {
157+
let json = r#"{
158+
"version": "0",
159+
"timeout_seconds": 0,
160+
"graph": {"nodes": [], "node_holes": [], "edge_property": "directed", "edges": []}
161+
}"#;
162+
assert!(serde_json::from_str::<PipelineGraph>(json).is_err());
142163
}
143164

144165
#[test]

crates/hm-pipeline-ir/tests/snapshots/schema_snapshot__command_step_schema_is_stable.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,14 @@ expression: schema
4848
}
4949
},
5050
"timeout_seconds": {
51-
"description": "Maximum wall-clock seconds before the step is killed.",
51+
"description": "Maximum wall-clock seconds before the step is killed.\n\n`NonZeroU32`: a `0`-second budget is rejected at the wire boundary.",
5252
"default": null,
5353
"type": [
5454
"integer",
5555
"null"
5656
],
5757
"format": "uint32",
58-
"minimum": 0.0
58+
"minimum": 1.0
5959
},
6060
"cache": {
6161
"description": "Cache configuration for this step's committed snapshot.",

0 commit comments

Comments
 (0)