From 34688c7476898c244cdd97ef58de02d0c8567581 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 25 Sep 2026 11:29:45 +0000 Subject: [PATCH 1/3] fix(config): a dotted array row is droppable, so a handler outlives its schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `[[hook.handler]]` fell through BOTH prune granularities, and the row is where the stale-binary detector lives — so the first schema addition a running binary predated unregistered the detector for exactly that staleness. A detector declared in the artifact it polices cannot police that artifact's own vocabulary. `Header::is_row` equated *droppable* with *dotless*, so `owning_row` declined and the row arm never ran. `drop_key` then asked the top-level table for a key spelled `hook.handler`, found `hook`, and declined too. Neither granularity could name the unit, so the whole file was refused — every rule off at once, which is the disposition CLOUD-1677 argues against at length. `is_row` now means what it says: any `[[name]]`. WHICH of the two dotted shapes a row is cannot be read off the header, because it is a fact about the document. `[[hook.handler]]` sits under a plain table and is reachable as the path `hook.handler`; `[[provision.env]]` sits inside whichever `[[provision]]` ELEMENT precedes it, and no path from the root names it. `addressable_row` asks the text whether the root is itself an array row, and the nested case still charges the element that owns it. Misreading it costs a wider unit, never a wrong blank. A section is then a PATH rather than a key: the array is resolved one table at a time, and an emptied section is removed from the table that holds it, up to the root — a file whose only `hook` headers were those rows parses, once they are all blanked, to a document with no `hook` key, not to `hook = {}`. Leaving the husk would fail the loop's own exactness guard and abandon a prune that was right. `deny_unknown_fields` STAYS. The key is still reported — `config show` names the dropped row and `doctor` fails `config-rows-dropped` — because a partial load that went silent would trade one failure mode for a quieter one, and that is the row's own second Done criterion. The cases go in `config_skew.rs` rather than the `mediator_skew.rs` the row names: `engine-config` already binds its suite at `config.rs`, a second `#MUTANT-SUITE` in one file is ignored, and a case elsewhere would carry no mutation. Three rows: restoring the dotless requirement, reading the section as a flat key, and making every dotted row droppable — the last is the one that keeps the fix from over-reaching, and the nested-provision case is what refutes it. Refs: CLOUD-1775 --- crates/batten/src/config.rs | 128 +++++++++++++++++++-- crates/batten/tests/it/config_skew.rs | 154 ++++++++++++++++++++++++++ 2 files changed, 274 insertions(+), 8 deletions(-) diff --git a/crates/batten/src/config.rs b/crates/batten/src/config.rs index aaea17f54..e11a62ab9 100644 --- a/crates/batten/src/config.rs +++ b/crates/batten/src/config.rs @@ -2465,9 +2465,26 @@ impl<'a> Header<'a> { self.name().split('.').next().unwrap_or(self.name()) } - /// Whether this header is a dotless `[[name]]` — the only droppable shape. + /// Whether this header is a `[[name]]` — an array-of-tables element. + /// + /// **Dotted or not (CLOUD-1775).** This asked `!name.contains('.')` and so + /// equated *droppable* with *dotless*, which is true of one dotted shape and + /// false of the other. `[[hook.handler]]` is an array under a plain table, + /// addressable as the path `hook.handler` and droppable a row at a time; + /// `[[provision.env]]` is an array inside the last `[[provision]]` ELEMENT, + /// where the droppable unit is that element. Conflating them cost the whole + /// file: `owning_row` declined the handler, so the row arm never ran, and + /// `drop_key` then looked for a top-level key spelled `hook.handler` and found + /// `hook`, so the key arm declined too. A `[[hook.handler]]` is where the + /// stale-binary detector lives, so the first schema addition a running binary + /// predates unregistered the detector for exactly that staleness. + /// + /// Which of the two shapes a dotted row is cannot be read off the header + /// alone — it is a fact about the DOCUMENT, namely whether the root is itself + /// an array row — so that question lives in [`owning_row`], which has the text. + //MUTANT dotted-row-not-droppable|s@ matches!(self, Self::Row(_))@ matches!(self, Self::Row(name) if !name.contains('.'))@|an_unknown_key_in_a_dotted_row_costs_the_row_not_the_file fn is_row(self) -> bool { - matches!(self, Self::Row(name) if !name.contains('.')) + matches!(self, Self::Row(_)) } } @@ -2808,6 +2825,54 @@ fn key_extent(text: &str, at: usize) -> (usize, usize) { /// counts this section's rows as the author wrote them, so two rows dropped in /// one load still report distinct pointers. Reading it off the working copy is /// how two `[[verb]]` rows both reported `#0`, caught in review. +/// Walk a dotted path of plain tables, from `table` down. +/// +/// `None` the moment a segment is missing or is not a table, which is the +/// fail-closed direction: the caller abandons the drop rather than guessing +/// where the row it is charging actually lives. +fn table_at<'a>(table: &'a mut toml::Table, path: &str) -> Option<&'a mut toml::Table> { + let mut at = table; + for segment in path.split('.') { + at = at.get_mut(segment)?.as_table_mut()?; + } + Some(at) +} + +/// Whether `header` names a row that can be addressed — and therefore dropped — +/// as a path from the top-level table. +/// +/// A dotless `[[name]]` always can. A dotted one can only when its root is NOT +/// itself an array row in this document: `[[hook.handler]]` sits under a plain +/// `hook` table and is reachable as `hook.handler`, while `[[provision.env]]` +/// sits inside whichever `[[provision]]` ELEMENT precedes it, and no path from +/// the root names it — the element that holds it is the unit, which is what the +/// caller's walk resolves to. +/// +/// **The answer is a property of the document, not of the header**, which is why +/// [`Header::is_row`] cannot decide it. It is also the fail-closed direction: a +/// root that is a row anywhere makes the dotted header un-droppable and the +/// caller falls back to the owning element, so a misread here costs a wider unit +/// rather than a wrong blank. +//MUTANT every-dotted-row-droppable|s@ !nests_in_a_row(text, header.root())@ true@|a_dotted_row_nested_in_an_array_element_still_charges_its_owner +fn addressable_row(text: &str, header: Header<'_>) -> bool { + if !header.is_row() { + return false; + } + if !header.name().contains('.') { + return true; + } + !nests_in_a_row(text, header.root()) +} + +/// Whether `root` is itself declared as an array-of-tables anywhere in `text`. +/// +/// Named rather than inlined for the `//MUTANT` row above's reason: a mutation +/// row is `id|script|case` split on `|`, so an anchor carrying a closure yields +/// five fields and the declaration can only ever report `malformed-row`. +fn nests_in_a_row(text: &str, root: &str) -> bool { + headers(text).any(|(_, other)| other.is_row() && other.name() == root) +} + fn owning_row(text: &str, at: usize) -> Option<(String, usize, usize)> { // The nearest header at or before `at`, then back through any dotted // headers it nests under. @@ -2817,7 +2882,7 @@ fn owning_row(text: &str, at: usize) -> Option<(String, usize, usize)> { if offset > at { continue; } - if header.is_row() && want.is_none_or(|root| root == header.name()) { + if addressable_row(text, header) && want.is_none_or(|root| root == header.name()) { found = Some((offset, header.name())); break; } @@ -2988,6 +3053,13 @@ fn drop_key( // A key under a `[[row]]` header that `owning_row` declined for some other // reason is not this arm's business: dropping one key out of a row would // leave that row enforcing something nobody wrote. + // + // DOTTED OR NOT (CLOUD-1775). This guard read `is_row` when that predicate + // meant *dotless* row, so a key inside a `[[hook.handler]]` reached the + // paragraph below and asked the top-level table for a key spelled + // `hook.handler` — which decided nothing, since the answer was `Declined` + // either way. It is a guard rather than an accident now: the row arm owns + // every `[[name]]`, and key granularity never reaches one. if header.is_row() { return KeyDrop::Declined; } @@ -3197,10 +3269,24 @@ fn prune_unresolvable(source: &str, behind: bool }) .count(); let mut expected = current.clone(); - let Some(rows) = expected - .get_mut(§ion) - .and_then(toml::Value::as_array_mut) - else { + // A SECTION IS A PATH, NOT A KEY (CLOUD-1775). `hook.handler` is a key + // `handler` inside the table `hook`; asking the top-level table for it + // finds nothing, which is half of why the handler rows fell through both + // granularities. A dotless section still resolves in one step, so this is + // the same lookup for every row that already worked. + let (holder_path, leaf) = section + .rsplit_once('.') + .map_or((None, section.as_str()), |(holder, leaf)| { + (Some(holder), leaf) + }); + let Some(holder) = (match holder_path { + Some(path) => table_at(&mut expected, path), + None => Some(&mut expected), + }) else { + break; + }; + //MUTANT dotted-section-read-as-a-key|s@ let Some(rows) = holder.get_mut(leaf).and_then(toml::Value::as_array_mut) else {@ let Some(rows) = holder.get_mut(section.as_str()).and_then(toml::Value::as_array_mut) else {@|an_unknown_key_in_a_dotted_row_costs_the_row_not_the_file + let Some(rows) = holder.get_mut(leaf).and_then(toml::Value::as_array_mut) else { break; }; let Some(position) = index.checked_sub(shift).filter(|at| *at < rows.len()) else { @@ -3209,8 +3295,34 @@ fn prune_unresolvable(source: &str, behind: bool rows.remove(position); // A section whose last row goes leaves no header at all, so the parsed // document loses the key rather than keeping an empty array. + // + // Under a DOTTED section that reaches one table further (CLOUD-1775): a + // file whose only `hook` headers were `[[hook.handler]]` rows parses, + // once they are all blanked, to a document with no `hook` key at all — + // not to `hook = {}`. So an emptied holder is removed from ITS holder in + // turn, up to the root. Leaving the husk behind would fail this loop's + // own exactness guard and abandon a prune that was otherwise right. if rows.is_empty() { - expected.remove(§ion); + holder.remove(leaf); + let mut path = holder_path; + while let Some(above) = path { + let (next, key) = above + .rsplit_once('.') + .map_or((None, above), |(next, key)| (Some(next), key)); + let emptied = match next { + Some(next) => table_at(&mut expected, next), + None => Some(&mut expected), + } + .filter(|holder| { + holder + .get(key) + .and_then(toml::Value::as_table) + .is_some_and(toml::Table::is_empty) + }); + let Some(emptied) = emptied else { break }; + emptied.remove(key); + path = next; + } } let mut candidate = text.clone(); diff --git a/crates/batten/tests/it/config_skew.rs b/crates/batten/tests/it/config_skew.rs index eec2daa7c..7f8a1f413 100644 --- a/crates/batten/tests/it/config_skew.rs +++ b/crates/batten/tests/it/config_skew.rs @@ -140,6 +140,160 @@ fn a_malformed_config_does_not_mention_a_rebuild() { ); } +/// Two `[[hook.handler]]` rows, the second carrying a key no build has. +/// +/// `[hook]` is never declared as a header of its own, which is the shape +/// `batten.toml` itself has: the only `hook` headers in the authority are the +/// sixteen `[[hook.handler]]` rows. That is what makes the section addressable +/// as a dotted path rather than as a nest inside some `[[hook]]` element. +const HANDLERS: &str = r#"version = 1 + +[[hook.handler]] +id = "readable-handler" +on = "user-prompt-submit" +run = ["true"] +owner = "CLOUD-1775" +expires = "2027-02-28" + +[[hook.handler]] +id = "from-a-newer-schema" +on = "user-prompt-submit" +run = ["true"] +owner = "CLOUD-1775" +expires = "2027-02-28" +this_key_does_not_exist_in_any_version = true +"#; + +/// THE CASE THE DEFECT FAILS (CLOUD-1775), and the one the bootstrap turns on. +/// +/// `[[hook.handler]]` fell through BOTH prune granularities. `Header::is_row` +/// equated droppable with dotless, so `owning_row` declined and the row arm never +/// ran; `drop_key` then asked the top-level table for a key spelled +/// `hook.handler`, found `hook` instead, and declined too. The file was refused +/// whole. +/// +/// That is the detector disabled by the staleness it detects: the skew handler is +/// declared in the artifact whose vocabulary it polices, so the first schema +/// addition a running binary predates unregisters it. Measured 2026-09-10 on +/// `command_matcher`, three hours after the same shape on `[wiring]`; between +/// them, eight `land` laps at ~900s of local `verify` failed for reasons that had +/// nothing to do with the branch. +/// +/// Fails by: `dotted-row-not-droppable`, which restores the dotless requirement. +#[test] +fn an_unknown_key_in_a_dotted_row_costs_the_row_not_the_file() { + let dir = scratch("dotted-row", HANDLERS); + + let output = check(&dir); + let said = stderr(&output); + assert_eq!( + output.status.code(), + Some(0), + "the rows this build CAN read must still load, or the detector is \ + disabled by exactly the skew it exists to report: {said}" + ); + assert!( + !said.contains("invalid config"), + "one unreadable row is not an unreadable file: {said}" + ); +} + +/// THE ANTI-VACUITY HALF. A partial load must not become a silent one. +/// +/// Without this, the case above is satisfied by a build that drops the key and +/// says nothing — which is the permissive fallback CLOUD-251 names, and would let +/// a typo switch a handler off with no reader ever learning of it. It is also the +/// second of the row's own Done criteria, stated in its words: *the unknown key +/// is still REPORTED*. +/// +/// The good row is asserted present in the same breath, because a report naming +/// the dropped row would also be produced by a build that dropped both. +#[test] +fn the_dropped_handler_is_named_and_its_neighbour_survives() { + let dir = scratch("dotted-row-named", HANDLERS); + + let out = batten() + .args(["config", "show"]) + .current_dir(&dir) + .env_remove("BATTEN_STRICTNESS") + .env_remove("BATTEN_CONFIG_FROM") + .output() + .expect("run batten config show"); + let said = format!("{}{}", String::from_utf8_lossy(&out.stdout), stderr(&out)); + assert!( + said.contains("from-a-newer-schema"), + "a dropped row is a handler that is NOT running, and must be reported by \ + id: {said}" + ); + // THE SURVIVOR IS COUNTED, NOT NAMED, and the count is the whole assertion. + // `config show` is pointer-only (rule 4), so it renders `hook ` rather + // than each handler's id — which is what makes the number load-bearing here: + // `2` is a build that dropped nothing, `0` is a prune that took the section + // instead of the row, and only `1` is the repair. + let counted = said + .lines() + .filter_map(|line| { + let mut fields = line.split_whitespace(); + (fields.next() == Some("hook")).then(|| fields.next())? + }) + .next(); + assert_eq!( + counted, + Some("1"), + "exactly the readable handler survives — 2 is no prune at all and 0 is \ + the section taken rather than the row: {said}" + ); + // POINTER, NEVER THE PAYLOAD (rule 4): the report names the row, never the + // bytes that could not be read. + assert!( + !said.contains("this_key_does_not_exist_in_any_version"), + "the report is a pointer, not the file's contents: {said}" + ); +} + +/// A DOTTED ROW NESTED INSIDE AN ARRAY ELEMENT IS STILL NOT THE DROPPABLE UNIT. +/// +/// `[[provision.env]]` is an array inside the last `[[provision]]` element, so +/// the row that owns a key under it is the `[[provision]]` row — dropping the +/// `env` block alone would leave a provision row provisioning something nobody +/// wrote. The discriminator is whether the dotted header's ROOT is itself an +/// array row in the same document; `hook` is not, `provision` is. +/// +/// This is the mirror that keeps the fix from becoming "every dotted header is +/// droppable", which would pass the two cases above and silently change what a +/// `[[provision]]` row provisions. +#[test] +fn a_dotted_row_nested_in_an_array_element_still_charges_its_owner() { + let dir = scratch( + "dotted-row-nested", + r#"version = 1 + +[[provision]] +id = "readable-provision" +run = ["true"] + +[[provision.env]] +name = "SOME_VAR" +value = "x" +this_key_does_not_exist_in_any_version = true +"#, + ); + + let out = batten() + .args(["config", "show"]) + .current_dir(&dir) + .env_remove("BATTEN_STRICTNESS") + .env_remove("BATTEN_CONFIG_FROM") + .output() + .expect("run batten config show"); + let said = format!("{}{}", String::from_utf8_lossy(&out.stdout), stderr(&out)); + assert!( + said.contains("readable-provision"), + "the unit charged is the [[provision]] row that owns the nested block, \ + named by its own id: {said}" + ); +} + /// The wording this predicate matches on is serde's, and nothing types it. /// /// `config_error` discriminates on the rendered message because serde exposes no From 4eaf76e3e3e5b4277859fa8eca99925619768ca8 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 25 Sep 2026 13:46:24 +0000 Subject: [PATCH 2/3] fix(config): an action row's unknown key costs the row, named rather than silent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cli::an_unknown_key_in_an_action_row_stays_a_hard_config_error` read acceptance (d) — *a mistyped key must never be SILENTLY dropped* — as "the whole file is refused", and until the previous commit that was also what happened: `[[hook.action]]` is a dotted header, `is_row` equated droppable with dotless, and neither prune granularity could name the unit, so the only granularity left was the file. CLOUD-1428 had already overruled that trade for `[[rule]]`: one row off and named beats every row off and silent, and only the first is a posture a reader can act on. A dotted row is a row, so the action row takes the same disposition. The concern (d) states is unchanged and is asserted as a PAIR now, neither half being the criterion alone: the file loads, AND `config show` names the dropped action by id. A build that dropped the key quietly satisfies the first and fails the second, which is the reading (d) exists to refuse. `deny_unknown_fields` stays on the row — it is what makes the key cost the row at all rather than be ignored. Admits: 735971f575e2751ed5b3ba3577ac1c761e48634ce3092164e094cdf9a5d651b3 Admits-rule: turn mint ahead Admits-verdict: receipt read other Admits-subject: receipt read other Admits-anchor: call:9d84d2fb8d24ad8706a17b3a5b7a415360647f18 Admits-epoch: d1fd73a23483c258a5b6a3f362913e32b3b452ca3ce9c776f90a9f6b9308f6ef Admits-author: alec@wenzowski.com Admits-prev: - Admits-answer-lost: the write that repairs it is the edit to crates/batten/tests/it/cli.rs, which is the mediated write this class refuses Admits-answer-precondition: verify is red on head 9d84d2fb because cli::an_unknown_key_in_an_action_row_stays_a_hard_config_error asserts exit 1 for an unknown key in a [[hook.action]] row, and this branch's CLOUD-1775 change makes that row droppable so the file now loads at exit 0; the only repair is editing that case Admits-answer-rejected-route: re-running verify cannot change its answer on this head — the case is red for a content reason the run will reproduce identically, and a verify receipt for this head is unobtainable until the case is edited Refs: CLOUD-1775 --- crates/batten/tests/it/cli.rs | 43 +++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/crates/batten/tests/it/cli.rs b/crates/batten/tests/it/cli.rs index 72d8de4b9..44d6d4ea7 100644 --- a/crates/batten/tests/it/cli.rs +++ b/crates/batten/tests/it/cli.rs @@ -9974,10 +9974,27 @@ fn an_action_on_the_adjudicated_event_is_a_config_error() { } #[test] -fn an_unknown_key_in_an_action_row_stays_a_hard_config_error() { - // Acceptance (d). `deny_unknown_fields` on the row, so a mistyped key is - // refused rather than silently dropped — an action nobody notices is - // half-declared is a side effect that never fires. +fn an_unknown_key_in_an_action_row_costs_the_row_and_is_reported() { + // Acceptance (d), AND THE SUBJECT MOVED UNDER IT — the concern is unchanged + // and what answers it is not. + // + // (d) asks that a mistyped key never be SILENTLY dropped, because an action + // nobody notices is half-declared is a side effect that never fires. This + // case read that as "the whole file is refused", and until CLOUD-1775 that + // was also what happened: `[[hook.action]]` is a dotted header, `is_row` + // equated droppable with dotless, and neither prune granularity could name + // the unit — so the only granularity left was the file. + // + // CLOUD-1428 had already overruled that trade for `[[rule]]`: one row off and + // named beats every row off and silent, and only the first is a posture a + // reader can act on. A dotted row is a row, so the action row takes the same + // disposition — and `deny_unknown_fields` STAYS on the row, which is what + // makes the key cost it at all rather than being ignored. + // + // So the assertion is the PAIR, and neither half alone is the criterion: the + // file loads, AND the drop is reported by id. A build that dropped the key + // quietly satisfies the first and fails the second, which is exactly the + // reading (d) exists to refuse. let dir = repo_with_config( "action-unknown-key", "version = 1\n\n[[hook.action]]\nid = \"probe\"\non = \"stop\"\nrun = [\"true\"]\nwhen = \"always\"\n", @@ -9988,7 +10005,23 @@ fn an_unknown_key_in_an_action_row_stays_a_hard_config_error() { &serde_json::json!({ "hook_event_name": "Stop" }).to_string(), false, ); - assert_eq!(output.status.code(), Some(1), "a usage error, never a deny"); + assert_eq!( + output.status.code(), + Some(0), + "one unreadable row is not an unreadable file: {}", + common::stderr(&output) + ); + + let shown = common::batten() + .args(["config", "show"]) + .current_dir(&dir) + .output() + .expect("run batten config show"); + let said = format!("{}{}", common::stdout(&shown), common::stderr(&shown)); + assert!( + said.contains("probe"), + "the dropped action is named by id, or the drop is the silence (d) refuses: {said}" + ); } /// CLOUD-437's fixture: two rows, one declaring its own hatch and one not. From fb693e7ef86de691a0ed1ec7c47022902ec4cc00 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 25 Sep 2026 22:28:30 +0000 Subject: [PATCH 3/3] fix(config): name the ancestor sweep, so the prune loop stays under the line lint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CLOUD-1775 cleanup grew `prune_unresolvable` to 113 lines against a ceiling of 100, and `spawn add other` refuses an `#[allow]` in engine source — correctly, since the escape would hide the growth rather than answer it. `drop_empty_ancestors` is the extraction, and the split falls where the reader wants it: the loop is what a reader comes to `prune_unresolvable` for, and the husk sweep is a detail of one branch of it. Its doc says what the caller's comment said and adds the caution the inline form left implicit — it stops at the first table that is not empty, so a section still holding something the author wrote is never removed because a sibling array emptied. The test's `filter_map(..).next()` is spelled `find_map`, which is the same lint pass asking for the shorter form of something this branch added. Both writes were refused by `turn mint ahead` and taken through the articulation route CLOUD-1823 declared — the route CLOUD-1889 made reachable, exercised here for the first time outside its own suite. Admits: 7becebbbe268d64c9ba93fa41744b89e2202e2ab8cead73d6a0c4cd1c68aa5d5 Admits-rule: turn mint ahead Admits-verdict: receipt read other Admits-subject: receipt read other Admits-anchor: call:c7f328c790bb067e4c94a32f917d9ba009cc0f0d Admits-epoch: d1fd73a23483c258a5b6a3f362913e32b3b452ca3ce9c776f90a9f6b9308f6ef Admits-author: alec@wenzowski.com Admits-prev: 3edf39af063ee53390fe12f38509c08f56c00b36c19083f9c64a6b9926bd712c Admits-answer-lost: the write that repairs it is the edit to crates/batten/src/config.rs, which is the mediated write this class refuses Admits-answer-precondition: verify is red on head c7f328c7 because clippy refuses prune_unresolvable at 113/100 lines — the CLOUD-1775 ancestor-cleanup loop grew it — and the repair is extracting that loop into a named function Admits-answer-rejected-route: re-running verify cannot change its answer on this head — the line count is a property of the bytes this head carries and the run will reproduce it identically Admits: 9bcff3292556d88a8e3efa2a2b18f771177e96e89f9887d0748b39e622285daa Admits-rule: turn mint ahead Admits-verdict: receipt read other Admits-subject: receipt read other Admits-anchor: call:c7f328c790bb067e4c94a32f917d9ba009cc0f0d Admits-epoch: d1fd73a23483c258a5b6a3f362913e32b3b452ca3ce9c776f90a9f6b9308f6ef Admits-author: alec@wenzowski.com Admits-prev: 7becebbbe268d64c9ba93fa41744b89e2202e2ab8cead73d6a0c4cd1c68aa5d5 Admits-answer-lost: the write that repairs it is the edit to crates/batten/tests/it/config_skew.rs, which is the mediated write this class refuses Admits-answer-precondition: verify is red on this head: clippy refuses filter_map(..).next() at crates/batten/tests/it/config_skew.rs:233, a lint on a test this branch added, and the repair is spelling it find_map Admits-answer-rejected-route: re-running verify cannot change its answer on this head — the lint is a property of the bytes this head carries and the run will reproduce it identically Refs: CLOUD-1775 --- crates/batten/src/config.rs | 53 +++++++++++++++++---------- crates/batten/tests/it/config_skew.rs | 11 ++---- 2 files changed, 38 insertions(+), 26 deletions(-) diff --git a/crates/batten/src/config.rs b/crates/batten/src/config.rs index e11a62ab9..a5107ea3d 100644 --- a/crates/batten/src/config.rs +++ b/crates/batten/src/config.rs @@ -2838,6 +2838,39 @@ fn table_at<'a>(table: &'a mut toml::Table, path: &str) -> Option<&'a mut toml:: Some(at) } +/// Remove every table along `path` that the drop above left empty, root-ward. +/// +/// A file whose only `hook` headers were `[[hook.handler]]` rows parses, once +/// they are all blanked, to a document with no `hook` key at all — not to +/// `hook = {}`. So the husk goes too, and its own holder is asked the same +/// question in turn. Leaving one behind would fail [`prune_unresolvable`]'s +/// exactness guard and abandon a prune that was otherwise right. +/// +/// It stops at the first table that is NOT empty, which is the whole of its +/// caution: a section that still holds something the author wrote is never +/// removed because a sibling array emptied. +fn drop_empty_ancestors(table: &mut toml::Table, path: Option<&str>) { + let mut path = path; + while let Some(above) = path { + let (next, key) = above + .rsplit_once('.') + .map_or((None, above), |(next, key)| (Some(next), key)); + let emptied = match next { + Some(next) => table_at(table, next), + None => Some(&mut *table), + } + .filter(|holder| { + holder + .get(key) + .and_then(toml::Value::as_table) + .is_some_and(toml::Table::is_empty) + }); + let Some(emptied) = emptied else { break }; + emptied.remove(key); + path = next; + } +} + /// Whether `header` names a row that can be addressed — and therefore dropped — /// as a path from the top-level table. /// @@ -3304,25 +3337,7 @@ fn prune_unresolvable(source: &str, behind: bool // own exactness guard and abandon a prune that was otherwise right. if rows.is_empty() { holder.remove(leaf); - let mut path = holder_path; - while let Some(above) = path { - let (next, key) = above - .rsplit_once('.') - .map_or((None, above), |(next, key)| (Some(next), key)); - let emptied = match next { - Some(next) => table_at(&mut expected, next), - None => Some(&mut expected), - } - .filter(|holder| { - holder - .get(key) - .and_then(toml::Value::as_table) - .is_some_and(toml::Table::is_empty) - }); - let Some(emptied) = emptied else { break }; - emptied.remove(key); - path = next; - } + drop_empty_ancestors(&mut expected, holder_path); } let mut candidate = text.clone(); diff --git a/crates/batten/tests/it/config_skew.rs b/crates/batten/tests/it/config_skew.rs index 7f8a1f413..a1ff65126 100644 --- a/crates/batten/tests/it/config_skew.rs +++ b/crates/batten/tests/it/config_skew.rs @@ -230,13 +230,10 @@ fn the_dropped_handler_is_named_and_its_neighbour_survives() { // than each handler's id — which is what makes the number load-bearing here: // `2` is a build that dropped nothing, `0` is a prune that took the section // instead of the row, and only `1` is the repair. - let counted = said - .lines() - .filter_map(|line| { - let mut fields = line.split_whitespace(); - (fields.next() == Some("hook")).then(|| fields.next())? - }) - .next(); + let counted = said.lines().find_map(|line| { + let mut fields = line.split_whitespace(); + (fields.next() == Some("hook")).then(|| fields.next())? + }); assert_eq!( counted, Some("1"),