Skip to content

Commit 582bb34

Browse files
committed
Fix deny TOML production value shapes
1 parent d21d399 commit 582bb34

11 files changed

Lines changed: 395 additions & 17 deletions

File tree

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
# Deny Production Shape Repair
2+
3+
## Goal
4+
5+
`aqc-deny-toml-engine` must write `deny.toml` values that `cargo deny` can parse.
6+
Shackles deny policy must use ordered threshold semantics for `licenses.confidence-threshold`.
7+
8+
## Current Failure
9+
10+
- Generated `maximum-db-staleness = "90d"` is rejected by `cargo deny`.
11+
- Generated `confidence-threshold = "0.8"` is rejected by `cargo deny`; it expects a float.
12+
- `DenyConfidenceThreshold` implements `ScalarValue::compare_for_order` as `None`, so `ScalarAssertion::AtLeast` cannot work for the field.
13+
14+
## Approach
15+
16+
- In `aqc-deny-toml-engine`, change `DenyConfidenceThreshold` from plain text semantics to a typed ratio:
17+
- parse constructor input like `"0.8"`
18+
- store a canonical text representation and an integer ordering key
19+
- implement `ScalarValue::compare_for_order` using the ordering key
20+
- parse TOML floats and write TOML floats
21+
- In `aqc-deny-toml-engine`, strengthen `DenyDuration` enough to reject the known wrong product default:
22+
- require constructor input to start with `P`
23+
- keep TOML representation as string
24+
- Add engine tests:
25+
- `AtLeast(0.8)` accepts `confidence-threshold = 0.9`
26+
- `AtLeast(0.8)` repairs `confidence-threshold = 0.7`
27+
- expected output writes `confidence-threshold = 0.8`, not a string
28+
- `DenyDuration::new("90d")` fails and `DenyDuration::new("P90D")` passes
29+
- Bump and publish `aqc-deny-toml-engine` to `0.1.1`.
30+
- In Shackles, update `shakrs-deny-policy`:
31+
- use `ScalarAssertion::AtLeast(DenyConfidenceThreshold::new("0.8"), ...)`
32+
- use `DenyDuration::new("P90D")`
33+
- update fixtures and spec/verifier expectations
34+
- bump and publish `shakrs-deny-policy` to `0.1.1`
35+
- bump and publish `shakrs` to `0.1.4`
36+
- Verify installed `shakrs` output with `cargo deny check`, not only `shakrs validate`.
37+
38+
## Files To Modify
39+
40+
- `packages/file-types/toml/aqc-deny-toml-engine/Cargo.toml`
41+
- `packages/file-types/toml/aqc-deny-toml-engine/src/requirement/value.rs`
42+
- `packages/file-types/toml/aqc-deny-toml-engine/src/requirement/value/value_impls/core.rs`
43+
- `packages/file-types/toml/aqc-deny-toml-engine/src/reconcile/scalar_value.rs`
44+
- `packages/file-types/toml/aqc-deny-toml-engine/tests/*`
45+
- Shackles deny policy, specs, fixtures, app lockfile, and release worklog
46+
47+
## Key Decisions
48+
49+
- Core already has `ScalarAssertion::AtLeast`; do not add another assertion type.
50+
- The field-specific fix belongs in the deny engine value type because only that type knows whether ordering is meaningful.
51+
- Do not make `DenyDuration` a generic duration parser here; this repair only prevents the rejected `90d` form and uses the cargo-deny `P90D` form.
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
# Deny Production Shape Repair
2+
3+
## Summary
4+
5+
Fixed `aqc-deny-toml-engine` so `licenses.confidence-threshold` is an ordered cargo-deny float and `advisories.maximum-db-staleness` rejects the known invalid non-cargo-deny duration form.
6+
Published `aqc-deny-toml-engine v0.1.1`.
7+
8+
## Decisions Made
9+
10+
- Kept `ScalarAssertion::AtLeast` in `aqc-file-engine-core`; no new core assertion was needed.
11+
- Changed `DenyConfidenceThreshold` semantics in the deny engine value type because that type owns the field's ordering and TOML representation.
12+
- Stored confidence thresholds with canonical text and an integer ordering key so merge uses deterministic ordering without float equality.
13+
- Made deny reconciliation parse and write confidence thresholds as TOML floats, matching cargo-deny.
14+
- Required `DenyDuration` strings to start with `P`, preventing the known rejected `90d` value from entering requirements.
15+
- Added tests for `AtLeast(0.8)` accepting `0.9`, repairing `0.7`, and writing `confidence-threshold = 0.8`.
16+
17+
## Key Files For Context
18+
19+
- `.plans/2026-07-07-141120-deny-production-shape-repair.md`
20+
- `packages/file-types/toml/aqc-deny-toml-engine/src/requirement/value.rs`
21+
- `packages/file-types/toml/aqc-deny-toml-engine/src/requirement/value/value_impls/core.rs`
22+
- `packages/file-types/toml/aqc-deny-toml-engine/src/reconcile/scalar_value.rs`
23+
- `packages/file-types/toml/aqc-deny-toml-engine/tests/reconcile.rs`
24+
- `packages/file-types/toml/aqc-deny-toml-engine/tests/merge.rs`
25+
- `specs/verifiers/verify_deny_toml_engine.py`
26+
27+
## Verification
28+
29+
- `cargo fmt --manifest-path packages/file-types/toml/aqc-deny-toml-engine/Cargo.toml`
30+
- `cargo test --manifest-path packages/file-types/toml/aqc-deny-toml-engine/Cargo.toml --all-targets`
31+
- `cargo deny --manifest-path packages/file-types/toml/aqc-deny-toml-engine/Cargo.toml check`
32+
- `specular lint specs/2026-07-07-103006-deny-toml-engine.spec.json`
33+
- `specular verify specs/2026-07-07-103006-deny-toml-engine.spec.json`
34+
- `cargo package --manifest-path packages/file-types/toml/aqc-deny-toml-engine/Cargo.toml --allow-dirty`
35+
- `cargo publish --manifest-path packages/file-types/toml/aqc-deny-toml-engine/Cargo.toml --allow-dirty`
36+
37+
## Next Steps
38+
39+
- Update Shackles deny policy to emit `ScalarAssertion::AtLeast(DenyConfidenceThreshold::new("0.8"), ...)`.
40+
- Update Shackles deny policy to use `DenyDuration::new("P90D")`.
41+
- Verify installed `shakrs` output with `cargo deny check`.

‎packages/file-types/toml/aqc-deny-toml-engine/Cargo.lock‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/file-types/toml/aqc-deny-toml-engine/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "aqc-deny-toml-engine"
3-
version = "0.1.0"
3+
version = "0.1.1"
44
edition = "2024"
55
license = "MIT OR Apache-2.0"
66
rust-version = "1.85"

‎packages/file-types/toml/aqc-deny-toml-engine/src/reconcile/scalar_value.rs‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,23 @@ macro_rules! impl_string_scalar {
6363

6464
impl_string_scalar!(DenyNonEmptyString);
6565
impl_string_scalar!(DenyDuration);
66-
impl_string_scalar!(DenyConfidenceThreshold);
66+
67+
impl DenyTomlScalar for DenyConfidenceThreshold {
68+
fn parse_item(item: &Item) -> Option<Self> {
69+
item.as_float()
70+
.map(|value| value.to_string())
71+
.or_else(|| item.as_integer().map(|value| value.to_string()))
72+
.and_then(|value| Self::new(value).ok())
73+
}
74+
75+
fn write_item(value: &Self) -> Item {
76+
toml_edit::value(value.as_f64())
77+
}
78+
79+
fn render_value(value: &Self) -> String {
80+
value.as_str().to_owned()
81+
}
82+
}
6783

6884
macro_rules! impl_enum_scalar {
6985
($type_name:ty) => {

‎packages/file-types/toml/aqc-deny-toml-engine/src/requirement/value.rs‎

Lines changed: 151 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//! Deny TOML scalar and item value types.
22
3+
use std::cmp::Ordering;
34
use std::collections::BTreeSet;
45

56
use serde::{Deserialize, Serialize};
@@ -8,9 +9,22 @@ mod value_impls;
89

910
#[derive(Debug, Clone, PartialEq, Eq)]
1011
pub enum DenyTomlValueError {
11-
Empty { field: &'static str },
12-
UnknownEnum { field: &'static str, value: String },
13-
OverlappingFeatures { package: String, feature: String },
12+
Empty {
13+
field: &'static str,
14+
},
15+
Invalid {
16+
field: &'static str,
17+
value: String,
18+
reason: &'static str,
19+
},
20+
UnknownEnum {
21+
field: &'static str,
22+
value: String,
23+
},
24+
OverlappingFeatures {
25+
package: String,
26+
feature: String,
27+
},
1428
}
1529

1630
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
@@ -52,8 +66,32 @@ pub struct DenyPackageSpec(String);
5266
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
5367
pub struct DenyDuration(String);
5468

55-
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
56-
pub struct DenyConfidenceThreshold(String);
69+
#[derive(Debug, Clone, Serialize, Deserialize)]
70+
#[serde(try_from = "String", into = "String")]
71+
pub struct DenyConfidenceThreshold {
72+
text: String,
73+
millionths: u32,
74+
}
75+
76+
impl PartialEq for DenyConfidenceThreshold {
77+
fn eq(&self, other: &Self) -> bool {
78+
self.millionths == other.millionths
79+
}
80+
}
81+
82+
impl Eq for DenyConfidenceThreshold {}
83+
84+
impl PartialOrd for DenyConfidenceThreshold {
85+
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
86+
Some(self.cmp(other))
87+
}
88+
}
89+
90+
impl Ord for DenyConfidenceThreshold {
91+
fn cmp(&self, other: &Self) -> Ordering {
92+
self.millionths.cmp(&other.millionths)
93+
}
94+
}
5795

5896
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
5997
pub struct DenyGraphTargetSpec {
@@ -200,5 +238,111 @@ macro_rules! impl_text_wrapper {
200238

201239
impl_text_wrapper!(DenyNonEmptyString, "text");
202240
impl_text_wrapper!(DenyPackageSpec, "package");
203-
impl_text_wrapper!(DenyDuration, "duration");
204-
impl_text_wrapper!(DenyConfidenceThreshold, "confidence-threshold");
241+
242+
impl DenyDuration {
243+
pub fn new(value: impl Into<String>) -> Result<Self, DenyTomlValueError> {
244+
let value = value.into();
245+
if value.is_empty() {
246+
return Err(DenyTomlValueError::Empty { field: "duration" });
247+
}
248+
if !value.starts_with('P') {
249+
return Err(DenyTomlValueError::Invalid {
250+
field: "duration",
251+
value,
252+
reason: "duration must use cargo-deny ISO-8601 form such as P90D",
253+
});
254+
}
255+
Ok(Self(value))
256+
}
257+
258+
#[must_use]
259+
pub fn as_str(&self) -> &str {
260+
&self.0
261+
}
262+
}
263+
264+
impl DenyConfidenceThreshold {
265+
pub fn new(value: impl Into<String>) -> Result<Self, DenyTomlValueError> {
266+
let value = value.into();
267+
if value.is_empty() {
268+
return Err(DenyTomlValueError::Empty {
269+
field: "confidence-threshold",
270+
});
271+
}
272+
let millionths = parse_confidence_millionths(&value)?;
273+
Ok(Self {
274+
text: canonical_confidence_text(&value),
275+
millionths,
276+
})
277+
}
278+
279+
#[must_use]
280+
pub fn as_str(&self) -> &str {
281+
&self.text
282+
}
283+
284+
#[must_use]
285+
pub fn as_f64(&self) -> f64 {
286+
self.text.parse::<f64>().unwrap_or(0.0)
287+
}
288+
}
289+
290+
impl TryFrom<String> for DenyConfidenceThreshold {
291+
type Error = DenyTomlValueError;
292+
293+
fn try_from(value: String) -> Result<Self, Self::Error> {
294+
Self::new(value)
295+
}
296+
}
297+
298+
impl From<DenyConfidenceThreshold> for String {
299+
fn from(value: DenyConfidenceThreshold) -> Self {
300+
value.text
301+
}
302+
}
303+
304+
fn parse_confidence_millionths(value: &str) -> Result<u32, DenyTomlValueError> {
305+
let Some((whole, fraction)) = value.split_once('.') else {
306+
return match value {
307+
"0" => Ok(0),
308+
"1" => Ok(1_000_000),
309+
_ => Err(invalid_confidence(value)),
310+
};
311+
};
312+
if fraction.is_empty()
313+
|| fraction.len() > 6
314+
|| !fraction.bytes().all(|byte| byte.is_ascii_digit())
315+
{
316+
return Err(invalid_confidence(value));
317+
}
318+
let padded = format!("{fraction:0<6}");
319+
let fraction_value = padded
320+
.parse::<u32>()
321+
.map_err(|_| invalid_confidence(value))?;
322+
match whole {
323+
"0" => Ok(fraction_value),
324+
"1" if fraction_value == 0 => Ok(1_000_000),
325+
_ => Err(invalid_confidence(value)),
326+
}
327+
}
328+
329+
fn canonical_confidence_text(value: &str) -> String {
330+
let mut out = value.to_owned();
331+
if out.contains('.') {
332+
while out.ends_with('0') {
333+
let _ = out.pop();
334+
}
335+
if out.ends_with('.') {
336+
out.push('0');
337+
}
338+
}
339+
out
340+
}
341+
342+
fn invalid_confidence(value: &str) -> DenyTomlValueError {
343+
DenyTomlValueError::Invalid {
344+
field: "confidence-threshold",
345+
value: value.to_owned(),
346+
reason: "confidence-threshold must be a number from 0.0 through 1.0",
347+
}
348+
}

‎packages/file-types/toml/aqc-deny-toml-engine/src/requirement/value/value_impls/core.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,10 @@ impl ScalarValue for value::DenyDuration {
7474

7575
impl ScalarValue for value::DenyConfidenceThreshold {
7676
fn render(&self) -> String {
77-
self.0.clone()
77+
self.as_str().to_owned()
7878
}
79-
fn compare_for_order(&self, _other: &Self) -> Option<Ordering> {
80-
None
79+
fn compare_for_order(&self, other: &Self) -> Option<Ordering> {
80+
Some(self.cmp(other))
8181
}
8282
}
8383

‎packages/file-types/toml/aqc-deny-toml-engine/src/requirement/value/value_impls/error.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ impl fmt::Display for DenyTomlValueError {
88
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
99
match self {
1010
Self::Empty { field } => write!(f, "{field} must not be empty"),
11+
Self::Invalid {
12+
field,
13+
value,
14+
reason,
15+
} => write!(f, "invalid {field} value {value}: {reason}"),
1116
Self::UnknownEnum { field, value } => {
1217
write!(f, "unknown {field} value {value}")
1318
}

0 commit comments

Comments
 (0)