fix(merge): gate error-code emission and never let a code outrank 401/403

crates/lance-context-api/src/lib.rs

@@ -1517,9 +1517,36 @@ impl MaintenanceErrorCode {
         }
     }
 
-    /// Render `message` with this code attached.
+    /// Render `message` with this code attached, if code emission is
+    /// enabled for this process; otherwise return `message` unchanged.
+    ///
+    /// Emission is gated because older masters classify these errors by
+    /// exact text and would treat a tagged message as a permanent error
+    /// with a one-hour cooldown. Deploy readers (every master and executor
+    /// understanding `parse`) first, then set `MAINTENANCE_ERROR_CODES=1`.
+    /// The default flips to on once no pre-code binary remains supported.
     pub fn tag(self, message: &str) -> String {
-        format!("[{}] {message}", self.as_str())
+        if Self::emission_enabled() {
+            format!("{message} [{}]", self.as_str())
+        } else {
+            message.to_string()
+        }
+    }
+
+    /// Render with the code regardless of the process gate. For tests and
+    /// for callers that have verified every reader in the fleet.
+    pub fn tag_forced(self, message: &str) -> String {
+        format!("{message} [{}]", self.as_str())
+    }
+
+    pub fn emission_enabled() -> bool {
+        use std::sync::OnceLock;
+        static ENABLED: OnceLock<bool> = OnceLock::new();
+        *ENABLED.get_or_init(|| {
+            std::env::var("MAINTENANCE_ERROR_CODES")
+                .map(|v| matches!(v.trim(), "1" | "true" | "on" | "yes"))
+                .unwrap_or(false)
+        })
     }
 
     /// Find the first code tag in an error string, regardless of wrapping.
@@ -1834,11 +1861,18 @@ mod tests {
     #[test]
     fn maintenance_error_code_round_trips_through_wrappers() {
         let code = MaintenanceErrorCode::StalePreparation;
-        let tagged = code.tag("index metadata changed; reprepare");
+        let tagged = code.tag_forced("index metadata changed; reprepare");
         assert_eq!(
             tagged,
-            "[LC_STALE_PREPARATION] index metadata changed; reprepare"
+            "index metadata changed; reprepare [LC_STALE_PREPARATION]"
         );
+        if std::env::var("MAINTENANCE_ERROR_CODES").is_err() {
+            assert_eq!(
+                code.tag("index metadata changed; reprepare"),
+                "index metadata changed; reprepare",
+                "emission is off by default for mixed-version safety"
+            );
+        }
         let wrapped = format!("Invalid user input: {tagged}, crates/x/src/y.rs:1:2");
         assert_eq!(MaintenanceErrorCode::parse(&wrapped), Some(code));
         assert_eq!(

crates/lance-context-merge/src/failure.rs

@@ -33,13 +33,17 @@ pub fn now_ms() -> u64 {
 }
 
 pub fn classify(error: &str) -> FailureClass {
-    // A stable code wins over any prose, path or wrapper format. Ownership
-    // is checked first because an unresolved owner must never be retried
-    // as an ordinary conflict.
+    // Priority: unresolved ownership, then explicit permanent signals that
+    // must never be laundered by a code (auth failures), then the stable
+    // code, then legacy text. A code beats prose, paths and wrapper formats
+    // but never beats a 401/403 that wraps it.
     let lowered = error.to_ascii_lowercase();
     if lowered.contains("ownership unresolved") {
         return FailureClass::OwnershipUnresolved;
     }
+    if has_http_status(&lowered, &["401", "403"]) {
+        return FailureClass::DataOrConfiguration;
+    }
     if let Some(code) = MaintenanceErrorCode::parse(error) {
         return match code {
             MaintenanceErrorCode::StalePreparation => FailureClass::Retryable,
@@ -83,6 +87,17 @@ pub fn classify(error: &str) -> FailureClass {
 // Do not exempt arbitrary errors merely containing "reprepare".
 fn is_preparation_conflict(error: &str) -> bool {
     let message = error.strip_prefix("invalid user input: ").unwrap_or(error);
+    // Tolerate a trailing lowercase code suffix on the message segment so a
+    // tagged string is also recognised by the text path (defence in depth).
+    let message = message
+        .split_once(" [lc_stale_preparation]")
+        .map_or(message, |(head, tail)| {
+            // Re-join head with the location tail if the code sat before it.
+            if tail.starts_with(", crates/") {
+                return head;
+            }
+            message
+        });
     let message = match message.split_once(", crates/lance-context-core/src/store_base.rs:") {
         Some((message, location)) => {
             let Some((line, column)) = location.split_once(':') else {
@@ -169,10 +184,11 @@ impl ShardFailure {
     // moving the failure timestamp, or mutating ownership/durable records.
     // The next fenced completion persists the normal retry result.
     fn corrected(mut self) -> Self {
+        // One rule set: whatever `classify` says today is the effective
+        // class. Only a DataOrConfiguration record can be softened; a stored
+        // Retryable is never hardened on read.
         if self.class == FailureClass::DataOrConfiguration
-            && (MaintenanceErrorCode::parse(&self.last_error)
-                == Some(MaintenanceErrorCode::StalePreparation)
-                || is_preparation_conflict(&self.last_error.to_ascii_lowercase()))
+            && classify(&self.last_error) == FailureClass::Retryable
         {
             self.class = FailureClass::Retryable;
             let (attention, delay) = Self::retry_policy(&self.class, self.consecutive_attempts);
@@ -418,7 +434,7 @@ mod tests {
     #[test]
     fn coded_preparation_conflicts_classify_regardless_of_wrapper_or_path() {
         let tagged = MaintenanceErrorCode::StalePreparation
-            .tag("compaction source fragment changed; reprepare");
+            .tag_forced("compaction source fragment changed; reprepare");
         for error in [
             format!(
                 "Invalid user input: {tagged}, crates/lance-context-core/src/store_base.rs:1529:24"
@@ -430,11 +446,39 @@ mod tests {
         ] {
             assert_eq!(classify(&error), FailureClass::Retryable, "{error}");
         }
-        // Ownership still wins, and the code does not launder a 403.
+        // Ownership still wins, and the code does not launder a 401/403
+        // regardless of where the status appears relative to the code.
         assert_eq!(
             classify(&format!("ownership unresolved: {tagged}")),
             FailureClass::OwnershipUnresolved
         );
+        for error in [
+            format!("HTTP 403: {tagged}"),
+            format!("{tagged}; status 403"),
+            format!("status code 401 {tagged}"),
+        ] {
+            assert_eq!(
+                classify(&error),
+                FailureClass::DataOrConfiguration,
+                "{error}"
+            );
+            // And corrected() must not soften such a record either.
+            let old = ShardFailure {
+                target: "table".into(),
+                endpoint: "master:compact".into(),
+                class: FailureClass::DataOrConfiguration,
+                consecutive_attempts: 1,
+                last_failure_ms: 2,
+                next_retry_ms: 3_602_000,
+                needs_attention: true,
+                last_error: error.clone(),
+            };
+            assert_eq!(
+                old.corrected().class,
+                FailureClass::DataOrConfiguration,
+                "{error}"
+            );
+        }
         // An old DataOrConfiguration record carrying a code is corrected too.
         let old = ShardFailure {
             target: "table".into(),
@@ -452,6 +496,36 @@ mod tests {
         assert_eq!(corrected.last_failure_ms, 2);
     }
 
+    /// Mixed-version rollout: a master older than the code reader classifies
+    /// by exact text. Emission must therefore be off by default so new
+    /// executors keep producing the exact legacy string, and the legacy text
+    /// path must still recognise a tagged string once emission is enabled.
+    #[test]
+    fn code_emission_is_off_by_default_and_text_path_tolerates_the_code() {
+        // Default: no code in the string (unless the test env sets the var).
+        if std::env::var("MAINTENANCE_ERROR_CODES").is_err() {
+            assert_eq!(
+                MaintenanceErrorCode::StalePreparation.tag("index metadata changed; reprepare"),
+                "index metadata changed; reprepare"
+            );
+        }
+        // Legacy exact matcher, byte-for-byte, as an old master sees a
+        // message produced with emission off.
+        let plain = "Invalid user input: index metadata changed; reprepare, crates/lance-context-core/src/store_base.rs:1529:24";
+        assert!(is_preparation_conflict(&plain.to_ascii_lowercase()));
+        // With emission on, the text path alone (no code parsing) still
+        // recognises it, so a reader with only the text path is not fooled.
+        let tagged = format!(
+            "Invalid user input: {}, crates/lance-context-core/src/store_base.rs:1529:24",
+            MaintenanceErrorCode::StalePreparation.tag_forced("index metadata changed; reprepare")
+        );
+        assert!(
+            is_preparation_conflict(&tagged.to_ascii_lowercase()),
+            "{tagged}"
+        );
+        assert_eq!(classify(&tagged), FailureClass::Retryable);
+    }
+
     #[test]
     fn old_conflict_records_recover_without_resetting_attempts_or_clock() {
         for attempts in [1, 4, 15, u32::MAX] {

docs/concurrent-indexing.md

@@ -36,11 +36,16 @@ the original snapshot read version, allowing Lance to check intervening commits
 as well. Newly appended fragments remain unindexed and query planning scans them;
 index publication must preserve merge watermarks and newly appended rows.
 
-Stale-preparation validation failures carry the stable code
-`[LC_STALE_PREPARATION]` in their message. The failure classifier recognises the
-code regardless of how Lance wraps the error or where the check lives in the
-source tree; the older exact-text matcher remains only for failure records
-written by binaries that predate the code. These failures use the ordinary
+Stale-preparation validation failures can carry the stable code
+`[LC_STALE_PREPARATION]` at the end of their message. The failure classifier
+recognises the code regardless of how Lance wraps the error or where the check
+lives in the source tree, but never above an explicit HTTP 401/403 or an
+unresolved-ownership prefix. Emission is **off by default**
+(`MAINTENANCE_ERROR_CODES=1` enables it): a master older than the code reader
+classifies by exact text and would treat a tagged message as a permanent error
+with a one-hour cooldown. Roll out readers to every master and executor first,
+then enable emission. The exact-text matcher stays for records from older
+binaries and also tolerates the code suffix. These failures use the ordinary
 bounded retry budget. The first two failed
 attempts wait two seconds; repeated failures back off and eventually require
 attention. Existing records incorrectly classified as data/configuration errors