From 6ea97492137693fe63c4dd975e4d0650aeff1e60 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Fri, 28 Aug 2026 17:36:48 +0800 Subject: [PATCH 1/3] fix(settings): fail closed on uncertain targets Refs #111 --- crates/workshop-rs/src/settings/schema.rs | 72 +++++++++++++++++-- crates/workshop-rs/src/settings/table.rs | 39 ++-------- crates/workshop-rs/tests/settings_pipeline.rs | 46 ++++++++++++ 3 files changed, 119 insertions(+), 38 deletions(-) diff --git a/crates/workshop-rs/src/settings/schema.rs b/crates/workshop-rs/src/settings/schema.rs index 880baef..3dc3711 100644 --- a/crates/workshop-rs/src/settings/schema.rs +++ b/crates/workshop-rs/src/settings/schema.rs @@ -237,6 +237,15 @@ pub enum SettingOperationError { setting: SettingId, target: SettingTarget, }, + ApplicabilityUnknown { + setting: SettingId, + target: Box, + }, + TargetShapeMismatch { + setting: SettingId, + definition: SettingTargetKind, + target: Box, + }, WrongValueKind { setting: SettingId, expected: &'static str, @@ -265,6 +274,18 @@ impl fmt::Display for SettingOperationError { "setting {setting} was not found for target {target:?}" ) } + Self::ApplicabilityUnknown { setting, target } => write!( + formatter, + "applicability of setting {setting} is unknown for target {target:?}" + ), + Self::TargetShapeMismatch { + setting, + definition, + target, + } => write!( + formatter, + "setting {setting} has target shape {definition:?} and cannot write target {target:?}" + ), Self::WrongValueKind { setting, expected, @@ -538,10 +559,15 @@ impl SettingDefinition { None => Applicability::Unknown, Some(false) => Applicability::NotApplicable, Some(true) => { - match table::hero_setting_applicability(actual_hero.as_str(), self.key) { - Some(true) => Applicability::Applicable, - Some(false) => Applicability::NotApplicable, - None => Applicability::Unknown, + if is_common_hero_setting(self.key) { + Applicability::Applicable + } else { + match table::hero_setting_applicability(actual_hero.as_str(), self.key) + { + Some(true) => Applicability::Applicable, + Some(false) => Applicability::NotApplicable, + None => Applicability::Unknown, + } } } } @@ -594,6 +620,13 @@ impl SettingDefinition { ) -> Result<(), SettingOperationError> { let id = self.operation_id()?; self.ensure_target(target)?; + if !self.write_target_shape_matches(target) { + return Err(SettingOperationError::TargetShapeMismatch { + setting: id, + definition: self.target_kind(), + target: Box::new(target.clone()), + }); + } let path = self.concrete_path(target); let node = find_node_mut(&mut settings.children, &path).ok_or_else(|| { SettingOperationError::NotFound { @@ -619,10 +652,25 @@ impl SettingDefinition { setting: id, target: target.clone(), }), - Applicability::Applicable | Applicability::Unknown => Ok(()), + Applicability::Unknown => Err(SettingOperationError::ApplicabilityUnknown { + setting: id, + target: Box::new(target.clone()), + }), + Applicability::Applicable => Ok(()), } } + fn write_target_shape_matches(&self, target: &SettingTarget) -> bool { + !matches!( + (&self.target, target), + (TargetPattern::Team(_), SettingTarget::Hero { .. }) + | ( + TargetPattern::TeamAbility { .. }, + SettingTarget::HeroAbility { .. } + ) + ) + } + fn operation_id(&self) -> Result { self.id() .cloned() @@ -670,6 +718,20 @@ fn target_hero(target: &SettingTarget) -> String { } } +fn is_common_hero_setting(key: &str) -> bool { + matches!( + key, + "health%" + | "enablePrimaryFire" + | "enableSecondaryFire" + | "enableAbility1" + | "enableAbility2" + | "enableAbility3" + | "combatUltGen%" + | "passiveUltGen%" + ) +} + fn find_node<'a>(children: &'a [SettingsNode], path: &[String]) -> Option<&'a SettingsNode> { let (name, rest) = path.split_first()?; let node = children.iter().find(|node| node.name() == name)?; diff --git a/crates/workshop-rs/src/settings/table.rs b/crates/workshop-rs/src/settings/table.rs index b661aa9..d593d02 100644 --- a/crates/workshop-rs/src/settings/table.rs +++ b/crates/workshop-rs/src/settings/table.rs @@ -1050,40 +1050,13 @@ pub fn hero_setting_name(hero: &str, key: &str, locale: &str) -> Option<&'static /// applicability for this catalog surface. pub fn hero_setting_applicability(hero: &str, key: &str) -> Option { hero_name(hero)?; - // These common controls are represented by the Workshop hero settings - // table for every topology-valid hero, while the export only carries - // localized labels for a subset. Do not turn that presentation gap into a - // false negative for typed queries. - if matches!( - key, - "health%" - | "enablePrimaryFire" - | "enableSecondaryFire" - | "enableAbility1" - | "enableAbility2" - | "enableAbility3" - | "combatUltGen%" - | "passiveUltGen%" - ) { - return None; + // This is a reviewed semantic exception, independent of localized + // presentation data. All other hero-specific applicability remains + // Unknown until an equivalent semantic fact is reviewed. + match key { + "ability1EnemyKb%" => Some(hero == "ashe"), + _ => None, } - let evidenced = GENERATED_HERO_SETTING_NAMES.iter().any(|entry| { - entry.key == key - && entry - .locales - .iter() - .any(|(_, value)| !value.trim().is_empty()) - }); - evidenced.then(|| { - GENERATED_HERO_SETTING_NAMES.iter().any(|entry| { - entry.hero == hero - && entry.key == key - && entry - .locales - .iter() - .any(|(_, value)| !value.trim().is_empty()) - }) - }) } #[derive(Deserialize)] diff --git a/crates/workshop-rs/tests/settings_pipeline.rs b/crates/workshop-rs/tests/settings_pipeline.rs index ae035df..75a2725 100644 --- a/crates/workshop-rs/tests/settings_pipeline.rs +++ b/crates/workshop-rs/tests/settings_pipeline.rs @@ -814,3 +814,49 @@ fn typed_settings_errors_reject_invalid_members_and_non_applicable_targets() { .expect_err("non-applicable hero setting must be rejected"); assert!(matches!(error, SettingOperationError::NotApplicable { .. })); } + +#[test] +fn typed_settings_writes_fail_closed_for_unknown_applicability_and_widening() { + let health = definitions() + .find(|definition| { + definition.path().ends_with("health%") + && definition.target_kind() == SettingTargetKind::Hero + }) + .expect("hero health definition"); + let mut settings = workshop_rs::settings::Settings { + span: None, + children: Vec::new(), + }; + let unknown_error = health + .write( + &mut settings, + &SettingTarget::Hero { + team: None, + hero: HeroId::new("futureHero"), + }, + SettingValue::Percent(100.0), + ) + .expect_err("unknown applicability must refuse writes"); + assert!(matches!( + unknown_error, + SettingOperationError::ApplicabilityUnknown { .. } + )); + + let team_definition = definitions() + .find(|definition| definition.target_kind() == SettingTargetKind::Team) + .expect("team definition"); + let widening_error = team_definition + .write( + &mut settings, + &SettingTarget::Hero { + team: None, + hero: HeroId::from(hero_ids::ANA), + }, + SettingValue::Boolean(false), + ) + .expect_err("team path must not be widened to a hero write"); + assert!(matches!( + widening_error, + SettingOperationError::TargetShapeMismatch { .. } + )); +} From 12f467d32af39df379d2e84b305c66d203857a6f Mon Sep 17 00:00:00 2001 From: Teakowa Date: Fri, 28 Aug 2026 18:39:53 +0800 Subject: [PATCH 2/3] fix(settings): preserve uncertain target inspection --- crates/workshop-rs/src/settings/schema.rs | 50 ++++++++----------- crates/workshop-rs/src/settings/table.rs | 15 ------ crates/workshop-rs/tests/settings_pipeline.rs | 42 ++++++++-------- 3 files changed, 42 insertions(+), 65 deletions(-) diff --git a/crates/workshop-rs/src/settings/schema.rs b/crates/workshop-rs/src/settings/schema.rs index 3dc3711..4b7777b 100644 --- a/crates/workshop-rs/src/settings/schema.rs +++ b/crates/workshop-rs/src/settings/schema.rs @@ -558,18 +558,7 @@ impl SettingDefinition { match hero_ability_exists(actual_hero, actual_slot, target_variant(target))? { None => Applicability::Unknown, Some(false) => Applicability::NotApplicable, - Some(true) => { - if is_common_hero_setting(self.key) { - Applicability::Applicable - } else { - match table::hero_setting_applicability(actual_hero.as_str(), self.key) - { - Some(true) => Applicability::Applicable, - Some(false) => Applicability::NotApplicable, - None => Applicability::Unknown, - } - } - } + Some(true) => Applicability::Unknown, } } (TargetPattern::Unknown, _) => Applicability::Unknown, @@ -589,7 +578,7 @@ impl SettingDefinition { target: &SettingTarget, ) -> Result { let id = self.operation_id()?; - self.ensure_target(target)?; + self.ensure_read_target(target)?; let path = self.concrete_path(target); let node = find_node(&settings.children, &path).ok_or_else(|| { SettingOperationError::NotFound { @@ -619,7 +608,7 @@ impl SettingDefinition { value: SettingValue, ) -> Result<(), SettingOperationError> { let id = self.operation_id()?; - self.ensure_target(target)?; + self.ensure_write_target(target)?; if !self.write_target_shape_matches(target) { return Err(SettingOperationError::TargetShapeMismatch { setting: id, @@ -639,7 +628,24 @@ impl SettingDefinition { apply_value(node, &id, value) } - fn ensure_target(&self, target: &SettingTarget) -> Result<(), SettingOperationError> { + fn ensure_read_target(&self, target: &SettingTarget) -> Result<(), SettingOperationError> { + let id = self.operation_id()?; + match self + .applicability(target) + .map_err(|error| SettingOperationError::InvalidValue { + setting: id.clone(), + message: error.to_string(), + span: None, + })? { + Applicability::NotApplicable => Err(SettingOperationError::NotApplicable { + setting: id, + target: target.clone(), + }), + Applicability::Applicable | Applicability::Unknown => Ok(()), + } + } + + fn ensure_write_target(&self, target: &SettingTarget) -> Result<(), SettingOperationError> { let id = self.operation_id()?; match self .applicability(target) @@ -718,20 +724,6 @@ fn target_hero(target: &SettingTarget) -> String { } } -fn is_common_hero_setting(key: &str) -> bool { - matches!( - key, - "health%" - | "enablePrimaryFire" - | "enableSecondaryFire" - | "enableAbility1" - | "enableAbility2" - | "enableAbility3" - | "combatUltGen%" - | "passiveUltGen%" - ) -} - fn find_node<'a>(children: &'a [SettingsNode], path: &[String]) -> Option<&'a SettingsNode> { let (name, rest) = path.split_first()?; let node = children.iter().find(|node| node.name() == name)?; diff --git a/crates/workshop-rs/src/settings/table.rs b/crates/workshop-rs/src/settings/table.rs index d593d02..23d6c83 100644 --- a/crates/workshop-rs/src/settings/table.rs +++ b/crates/workshop-rs/src/settings/table.rs @@ -1044,21 +1044,6 @@ pub fn hero_setting_name(hero: &str, key: &str, locale: &str) -> Option<&'static }) } -/// Return explicit applicability evidence for a hero setting from the -/// reviewed hero-setting export. `None` means the hero is not in the reviewed -/// roster; otherwise the exported presence/absence is the effective setting -/// applicability for this catalog surface. -pub fn hero_setting_applicability(hero: &str, key: &str) -> Option { - hero_name(hero)?; - // This is a reviewed semantic exception, independent of localized - // presentation data. All other hero-specific applicability remains - // Unknown until an equivalent semantic fact is reviewed. - match key { - "ability1EnemyKb%" => Some(hero == "ashe"), - _ => None, - } -} - #[derive(Deserialize)] struct HeroSettingAlias { hero: String, diff --git a/crates/workshop-rs/tests/settings_pipeline.rs b/crates/workshop-rs/tests/settings_pipeline.rs index 75a2725..7678262 100644 --- a/crates/workshop-rs/tests/settings_pipeline.rs +++ b/crates/workshop-rs/tests/settings_pipeline.rs @@ -422,10 +422,10 @@ fn settings_schema_distinguishes_applicability_and_unknown_hero_evidence() { Applicability::Unknown ); - let ashe_only = definitions + let ability1_enemy_kb = definitions .iter() .find(|definition| definition.path().ends_with("ability1EnemyKb%")) - .expect("Ashe-only ability setting"); + .expect("ability 1 enemy knockback setting"); let ana_ability1 = SettingTarget::HeroAbility { team: None, hero: HeroId::from(hero_ids::ANA), @@ -433,10 +433,10 @@ fn settings_schema_distinguishes_applicability_and_unknown_hero_evidence() { variant: None, }; assert_eq!( - ashe_only + ability1_enemy_kb .applicability(&ana_ability1) .expect("applicability"), - Applicability::NotApplicable + Applicability::Unknown ); let ability1 = definitions @@ -745,31 +745,28 @@ fn typed_settings_read_and_write_preserve_unrelated_structure() { slot: LogicalSlot::from(slots::PRIMARY_FIRE), variant: None, }; - primary - .write( - program.settings.as_mut().expect("settings"), - &target, - SettingValue::Boolean(false), - ) - .expect("hero typed write"); + assert_eq!( + primary.applicability(&target).expect("applicability"), + Applicability::Unknown + ); assert!(matches!( primary .read(program.settings.as_ref().expect("settings"), &target) - .expect("hero typed read") + .expect("typed reads preserve evidence-insufficient occurrences") .authored, - SettingValue::Boolean(false) + SettingValue::Boolean(true) )); let error = primary .write( program.settings.as_mut().expect("settings"), &target, - SettingValue::Number(1.0), + SettingValue::Boolean(false), ) - .expect_err("wrong kind must be rejected"); + .expect_err("unknown applicability must reject writes"); assert!(matches!( error, - SettingOperationError::WrongValueKind { span: Some(_), .. } + SettingOperationError::ApplicabilityUnknown { .. } )); } @@ -797,10 +794,10 @@ fn typed_settings_errors_reject_invalid_members_and_non_applicable_targets() { SettingOperationError::InvalidValue { span: Some(_), .. } )); - let ashe_only = definitions() + let ability1_enemy_kb = definitions() .find(|definition| definition.path().ends_with("ability1EnemyKb%")) - .expect("Ashe-only setting"); - let error = ashe_only + .expect("ability 1 enemy knockback setting"); + let error = ability1_enemy_kb .write( program.settings.as_mut().expect("settings"), &SettingTarget::HeroAbility { @@ -811,8 +808,11 @@ fn typed_settings_errors_reject_invalid_members_and_non_applicable_targets() { }, SettingValue::Percent(10.0), ) - .expect_err("non-applicable hero setting must be rejected"); - assert!(matches!(error, SettingOperationError::NotApplicable { .. })); + .expect_err("uncertain hero setting must be rejected for writes"); + assert!(matches!( + error, + SettingOperationError::ApplicabilityUnknown { .. } + )); } #[test] From af74d1c6c5177b2be00a7a4cf176eeaa19669cf4 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Fri, 28 Aug 2026 20:09:30 +0800 Subject: [PATCH 3/3] fix(settings): preserve wildcard target uncertainty --- crates/workshop-rs/src/settings/schema.rs | 35 ++----------------- crates/workshop-rs/tests/settings_pipeline.rs | 14 ++++---- 2 files changed, 9 insertions(+), 40 deletions(-) diff --git a/crates/workshop-rs/src/settings/schema.rs b/crates/workshop-rs/src/settings/schema.rs index 4b7777b..526bc3c 100644 --- a/crates/workshop-rs/src/settings/schema.rs +++ b/crates/workshop-rs/src/settings/schema.rs @@ -241,11 +241,6 @@ pub enum SettingOperationError { setting: SettingId, target: Box, }, - TargetShapeMismatch { - setting: SettingId, - definition: SettingTargetKind, - target: Box, - }, WrongValueKind { setting: SettingId, expected: &'static str, @@ -278,14 +273,6 @@ impl fmt::Display for SettingOperationError { formatter, "applicability of setting {setting} is unknown for target {target:?}" ), - Self::TargetShapeMismatch { - setting, - definition, - target, - } => write!( - formatter, - "setting {setting} has target shape {definition:?} and cannot write target {target:?}" - ), Self::WrongValueKind { setting, expected, @@ -457,7 +444,7 @@ impl SettingDefinition { } (TargetPattern::Team(expected), SettingTarget::Hero { team, .. }) => { if team_matches(expected.as_deref(), team.as_ref()) { - Applicability::Applicable + Applicability::Unknown } else { Applicability::NotApplicable } @@ -507,7 +494,7 @@ impl SettingDefinition { Applicability::NotApplicable } else { match hero_ability_exists(actual_hero, actual_slot, actual_variant.as_ref())? { - Some(true) => Applicability::Applicable, + Some(true) => Applicability::Unknown, Some(false) => Applicability::NotApplicable, None => Applicability::Unknown, } @@ -609,13 +596,6 @@ impl SettingDefinition { ) -> Result<(), SettingOperationError> { let id = self.operation_id()?; self.ensure_write_target(target)?; - if !self.write_target_shape_matches(target) { - return Err(SettingOperationError::TargetShapeMismatch { - setting: id, - definition: self.target_kind(), - target: Box::new(target.clone()), - }); - } let path = self.concrete_path(target); let node = find_node_mut(&mut settings.children, &path).ok_or_else(|| { SettingOperationError::NotFound { @@ -666,17 +646,6 @@ impl SettingDefinition { } } - fn write_target_shape_matches(&self, target: &SettingTarget) -> bool { - !matches!( - (&self.target, target), - (TargetPattern::Team(_), SettingTarget::Hero { .. }) - | ( - TargetPattern::TeamAbility { .. }, - SettingTarget::HeroAbility { .. } - ) - ) - } - fn operation_id(&self) -> Result { self.id() .cloned() diff --git a/crates/workshop-rs/tests/settings_pipeline.rs b/crates/workshop-rs/tests/settings_pipeline.rs index 7678262..cc66a3b 100644 --- a/crates/workshop-rs/tests/settings_pipeline.rs +++ b/crates/workshop-rs/tests/settings_pipeline.rs @@ -480,7 +480,7 @@ fn settings_schema_distinguishes_applicability_and_unknown_hero_evidence() { variant: None, }) .expect("applicability"), - Applicability::Applicable + Applicability::Unknown ); let health = definitions @@ -616,7 +616,7 @@ fn settings_schema_normalizes_concept_ids_and_group_targets() { variant: Some(AbilityVariant::new("mech")), }) .expect("applicability"), - Applicability::Applicable + Applicability::Unknown ); assert_eq!( team_primary @@ -627,7 +627,7 @@ fn settings_schema_normalizes_concept_ids_and_group_targets() { variant: Some(AbilityVariant::new("pilot")), }) .expect("applicability"), - Applicability::Applicable + Applicability::Unknown ); assert_eq!( team_primary @@ -654,7 +654,7 @@ fn settings_schema_normalizes_concept_ids_and_group_targets() { hero: HeroId::from(hero_ids::ANA), }) .expect("applicability"), - Applicability::Applicable + Applicability::Unknown ); } @@ -816,7 +816,7 @@ fn typed_settings_errors_reject_invalid_members_and_non_applicable_targets() { } #[test] -fn typed_settings_writes_fail_closed_for_unknown_applicability_and_widening() { +fn typed_settings_writes_fail_closed_for_unknown_applicability() { let health = definitions() .find(|definition| { definition.path().ends_with("health%") @@ -854,9 +854,9 @@ fn typed_settings_writes_fail_closed_for_unknown_applicability_and_widening() { }, SettingValue::Boolean(false), ) - .expect_err("team path must not be widened to a hero write"); + .expect_err("team-to-hero applicability without evidence must refuse writes"); assert!(matches!( widening_error, - SettingOperationError::TargetShapeMismatch { .. } + SettingOperationError::ApplicabilityUnknown { .. } )); }