fix(settings): fail closed on uncertain targets

crates/workshop-rs/src/settings/schema.rs

@@ -237,6 +237,10 @@ pub enum SettingOperationError {
         setting: SettingId,
         target: SettingTarget,
     },
+    ApplicabilityUnknown {
+        setting: SettingId,
+        target: Box<SettingTarget>,
+    },
     WrongValueKind {
         setting: SettingId,
         expected: &'static str,
@@ -265,6 +269,10 @@ 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::WrongValueKind {
                 setting,
                 expected,
@@ -436,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
                 }
@@ -486,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,
                     }
@@ -537,13 +545,7 @@ impl SettingDefinition {
                 match hero_ability_exists(actual_hero, actual_slot, target_variant(target))? {
                     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,
-                        }
-                    }
+                    Some(true) => Applicability::Unknown,
                 }
             }
             (TargetPattern::Unknown, _) => Applicability::Unknown,
@@ -563,7 +565,7 @@ impl SettingDefinition {
         target: &SettingTarget,
     ) -> Result<SettingOccurrence, SettingOperationError> {
         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 {
@@ -593,7 +595,7 @@ impl SettingDefinition {
         value: SettingValue,
     ) -> Result<(), SettingOperationError> {
         let id = self.operation_id()?;
-        self.ensure_target(target)?;
+        self.ensure_write_target(target)?;
         let path = self.concrete_path(target);
         let node = find_node_mut(&mut settings.children, &path).ok_or_else(|| {
             SettingOperationError::NotFound {
@@ -606,7 +608,7 @@ 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)
@@ -623,6 +625,27 @@ impl SettingDefinition {
         }
     }
 
+    fn ensure_write_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::Unknown => Err(SettingOperationError::ApplicabilityUnknown {
+                setting: id,
+                target: Box::new(target.clone()),
+            }),
+            Applicability::Applicable => Ok(()),
+        }
+    }
+
     fn operation_id(&self) -> Result<SettingId, SettingOperationError> {
         self.id()
             .cloned()

crates/workshop-rs/src/settings/table.rs

@@ -1044,48 +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<bool> {
-    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;
-    }
-    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)]
 struct HeroSettingAlias {
     hero: String,

crates/workshop-rs/tests/settings_pipeline.rs

@@ -422,21 +422,21 @@ 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),
         slot: LogicalSlot::from(slots::ABILITY_1),
         variant: None,
     };
     assert_eq!(
-        ashe_only
+        ability1_enemy_kb
             .applicability(&ana_ability1)
             .expect("applicability"),
-        Applicability::NotApplicable
+        Applicability::Unknown
     );
 
     let ability1 = definitions
@@ -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
     );
 }
 
@@ -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,6 +808,55 @@ 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]
+fn typed_settings_writes_fail_closed_for_unknown_applicability() {
+    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-to-hero applicability without evidence must refuse writes");
+    assert!(matches!(
+        widening_error,
+        SettingOperationError::ApplicabilityUnknown { .. }
+    ));
 }