🩹 harden file rotation failures

docs/api/file-sink-reopen-append.md

@@ -2,7 +2,7 @@
 name: file-sink-reopen-append
 group: api
 category: sink
-update-time: 20260613
+update-time: 20260717
 description: Reopen a FileSink in append mode.
 key-word:
     - file
@@ -37,6 +37,7 @@ Detailed rules explaining key parameters and behaviors
 - Reopen behavior is fixed to append mode.
 - The stored append policy is updated to `true` as part of the reopen path.
 - On failure, the sink remains unavailable and `open_failures` is incremented.
+- After a rotation failure, this is the explicit recovery path once the underlying remove, rename, or open condition has been resolved.
 
 ### How to Use
 

docs/api/file-sink-rotation-failures.md

@@ -2,7 +2,7 @@
 name: file-sink-rotation-failures
 group: api
 category: sink
-update-time: 20260707
+update-time: 20260717
 description: Read the number of rotation failures recorded by a FileSink.
 key-word:
     - file
@@ -36,6 +36,7 @@ Detailed rules explaining key parameters and behaviors
 - The sink reports its recorded rotation-failure count directly.
 - The counter increases when a rotation attempt cannot complete its critical file steps, such as removing the active file with zero backups, renaming a live file into `.1`, shifting an existing backup to a higher slot, or reopening the fresh active file afterward.
 - Missing optional backup slots do not count by themselves; only steps that fail while their source path still exists are treated as rotation failures.
+- When a critical rotation step fails, the triggering record is not written and the sink remains unavailable until an explicit reopen succeeds.
 - The counter is cumulative until reset.
 - This helper is observation-only and does not mutate sink state.
 
@@ -70,6 +71,8 @@ e.g.:
 
 - This counter alone does not say whether the sink is currently available.
 
+- After a recorded rotation failure, use `reopen_append()` only after the underlying filesystem condition has been resolved.
+
 ### Notes
 
 1. Use this helper when diagnosing incomplete direct file rotations.

src/BitLogger_wbtest.mbt

@@ -105,57 +105,3 @@ test "file sink can rotate on native backend" {
     inspect(sink.rotation_failures(), content="0")
   }
 }
-
-///|
-#cfg(platform="windows")
-test "file sink rotation failure counts partial backup-chain failure on native backend" {
-  let path = "logs/bitlogger-rotate-partial-failure.log"
-  ignore(@utils.remove_file_internal(path))
-  ignore(@utils.remove_file_internal(path + ".1"))
-  ignore(@utils.remove_file_internal(path + ".2"))
-
-  let seed = file_sink(
-    path,
-    auto_flush=true,
-    rotation=Some(file_rotation(20, max_backups=2)),
-    formatter=fn(rec) { rec.message },
-  )
-  if seed.is_available() {
-    seed.write(record(Level::Info, "1234567890"))
-    seed.write(record(Level::Info, "abcdefghij"))
-    inspect(seed.rotation_failures(), content="0")
-    inspect(seed.close(), content="true")
-
-    let locked = @utils.open_file_handle_internal(path + ".2", true)
-    inspect(locked is Some(_), content="true")
-
-    let sink = file_sink(
-      path,
-      auto_flush=true,
-      rotation=Some(file_rotation(20, max_backups=2)),
-      formatter=fn(rec) { rec.message },
-    )
-    inspect(sink.is_available(), content="true")
-    sink.write(record(Level::Info, "klmnopqrst"))
-
-    inspect(sink.rotation_failures(), content="1")
-    inspect(sink.state().rotation_failures, content="1")
-    inspect(sink.write_failures(), content="1")
-    inspect(sink.is_available(), content="false")
-
-    match locked {
-      None => ()
-      Some(handle) => ignore(@utils.close_file_handle_internal(handle))
-    }
-
-    inspect(sink.reopen_append(), content="true")
-    inspect(sink.is_available(), content="true")
-    inspect(sink.close(), content="true")
-
-    ignore(@utils.remove_file_internal(path))
-    ignore(@utils.remove_file_internal(path + ".1"))
-    ignore(@utils.remove_file_internal(path + ".2"))
-  } else {
-    inspect(native_files_supported(), content="false")
-  }
-}

src/file_runtime/file_sink.mbt

@@ -16,6 +16,24 @@ type Record = @core.Record
 ///|
 type RecordFormatter = @formatting.RecordFormatter
 
+///|
+priv struct RotationFileOps {
+  file_exists : (String) -> Bool
+  remove_file : (String) -> Bool
+  rename_file : (String, String) -> Bool
+}
+
+///|
+fn default_rotation_file_ops() -> RotationFileOps {
+  {
+    file_exists: fn(path) { @utils.file_exists_internal(path) },
+    remove_file: fn(path) { @utils.remove_file_internal(path) },
+    rename_file: fn(from_path, to_path) {
+      @utils.rename_file_internal(from_path, to_path)
+    },
+  }
+}
+
 ///|
 pub struct FileSink {
   priv path : String
@@ -31,6 +49,7 @@ pub struct FileSink {
   priv write_failures : Ref[Int]
   priv flush_failures : Ref[Int]
   priv rotation_failures : Ref[Int]
+  priv rotation_file_ops : Ref[RotationFileOps]
 }
 
 ///|
@@ -61,6 +80,7 @@ pub fn file_sink(
     write_failures: Ref(0),
     flush_failures: Ref(0),
     rotation_failures: Ref(0),
+    rotation_file_ops: Ref(default_rotation_file_ops()),
   }
 }
 
@@ -291,6 +311,28 @@ fn rotated_file_path(path : String, index : Int) -> String {
   "\{path}.\{index}"
 }
 
+///|
+fn remove_file_if_present(ops : RotationFileOps, path : String) -> Bool {
+  if !(ops.file_exists)(path) {
+    true
+  } else {
+    (ops.remove_file)(path) || !(ops.file_exists)(path)
+  }
+}
+
+///|
+fn rename_file_if_present(
+  ops : RotationFileOps,
+  from_path : String,
+  to_path : String,
+) -> Bool {
+  if !(ops.file_exists)(from_path) {
+    true
+  } else {
+    (ops.rename_file)(from_path, to_path) || !(ops.file_exists)(from_path)
+  }
+}
+
 ///|
 fn rotate_file_sink_internal(sink : FileSink, rotation : FileRotation) -> Bool {
   let closed = match sink.handle.val {
@@ -304,23 +346,27 @@ fn rotate_file_sink_internal(sink : FileSink, rotation : FileRotation) -> Bool {
   if !closed {
     return false
   }
+  let ops = sink.rotation_file_ops.val
   if rotation.max_backups > 0 {
-    ignore(
+    if !remove_file_if_present(
-      @utils.remove_file_internal(
+        ops,
         rotated_file_path(sink.path, rotation.max_backups),
-      ),
+      ) {
-    )
+      return false
+    }
     for index = rotation.max_backups - 1; index >= 1; {
       let from_path = rotated_file_path(sink.path, index)
       let to_path = rotated_file_path(sink.path, index + 1)
-      ignore(@utils.rename_file_internal(from_path, to_path))
+      if !rename_file_if_present(ops, from_path, to_path) {
+        return false
+      }
       continue index - 1
     }
-    ignore(
+    if !rename_file_if_present(ops, sink.path, rotated_file_path(sink.path, 1)) {
-      @utils.rename_file_internal(sink.path, rotated_file_path(sink.path, 1)),
+      return false
-    )
+    }
-  } else {
+  } else if !remove_file_if_present(ops, sink.path) {
-    ignore(@utils.remove_file_internal(sink.path))
+    return false
   }
   sink.handle.val = @utils.open_file_handle_internal(sink.path, false)
   sink.handle.val is Some(_)

src/file_runtime/file_sink_rotation_wbtest.mbt

@@ -0,0 +1,111 @@
+///|
+test "file sink counts an injected partial backup-chain failure" {
+  let test_path = "logs/file-runtime-rotate-partial-failure.log"
+  ignore(@utils.remove_file_internal(test_path))
+  ignore(@utils.remove_file_internal(test_path + ".1"))
+  ignore(@utils.remove_file_internal(test_path + ".2"))
+
+  let rotation = @file_model.file_rotation(20, max_backups=2)
+  let seed = file_sink(test_path, auto_flush=true, rotation=Some(rotation), formatter=fn(
+    rec,
+  ) {
+    rec.message
+  })
+  if seed.is_available() {
+    seed.write(@core.Record::new(@core.Level::Info, "1234567890"))
+    seed.write(@core.Record::new(@core.Level::Info, "abcdefghij"))
+    inspect(seed.rotation_failures(), content="0")
+    inspect(seed.close(), content="true")
+
+    let sink = file_sink(test_path, auto_flush=true, rotation=Some(rotation), formatter=fn(
+      rec,
+    ) {
+      rec.message
+    })
+    sink.rotation_file_ops.val = {
+      file_exists: fn(candidate) { @utils.file_exists_internal(candidate) },
+      remove_file: fn(candidate) { @utils.remove_file_internal(candidate) },
+      rename_file: fn(from_path, to_path) {
+        if from_path == test_path + ".1" && to_path == test_path + ".2" {
+          false
+        } else {
+          @utils.rename_file_internal(from_path, to_path)
+        }
+      },
+    }
+    sink.write(@core.Record::new(@core.Level::Info, "klmnopqrst"))
+
+    inspect(sink.rotation_failures(), content="1")
+    inspect(sink.state().rotation_failures, content="1")
+    inspect(sink.write_failures(), content="1")
+    inspect(sink.is_available(), content="false")
+    inspect(sink.reopen_append(), content="true")
+    inspect(sink.is_available(), content="true")
+    inspect(sink.close(), content="true")
+
+    ignore(@utils.remove_file_internal(test_path))
+    ignore(@utils.remove_file_internal(test_path + ".1"))
+    ignore(@utils.remove_file_internal(test_path + ".2"))
+  } else {
+    inspect(native_files_supported(), content="false")
+  }
+}
+
+///|
+test "file sink counts an injected oldest-backup removal failure" {
+  let test_path = "logs/file-runtime-rotate-removal-failure.log"
+  ignore(@utils.remove_file_internal(test_path))
+  ignore(@utils.remove_file_internal(test_path + ".1"))
+  ignore(@utils.remove_file_internal(test_path + ".2"))
+
+  let rotation = @file_model.file_rotation(20, max_backups=2)
+  let seed = file_sink(test_path, auto_flush=true, rotation=Some(rotation), formatter=fn(
+    rec,
+  ) {
+    rec.message
+  })
+  if seed.is_available() {
+    seed.write(@core.Record::new(@core.Level::Info, "1234567890"))
+    seed.write(@core.Record::new(@core.Level::Info, "abcdefghij"))
+    inspect(seed.close(), content="true")
+
+    let oldest = @utils.open_file_handle_internal(test_path + ".2", true)
+    inspect(oldest is Some(_), content="true")
+    match oldest {
+      None => ()
+      Some(handle) =>
+        inspect(@utils.close_file_handle_internal(handle), content="true")
+    }
+
+    let sink = file_sink(test_path, auto_flush=true, rotation=Some(rotation), formatter=fn(
+      rec,
+    ) {
+      rec.message
+    })
+    sink.rotation_file_ops.val = {
+      file_exists: fn(candidate) { @utils.file_exists_internal(candidate) },
+      remove_file: fn(candidate) {
+        if candidate == test_path + ".2" {
+          false
+        } else {
+          @utils.remove_file_internal(candidate)
+        }
+      },
+      rename_file: fn(from_path, to_path) {
+        @utils.rename_file_internal(from_path, to_path)
+      },
+    }
+    sink.write(@core.Record::new(@core.Level::Info, "klmnopqrst"))
+
+    inspect(sink.rotation_failures(), content="1")
+    inspect(sink.write_failures(), content="1")
+    inspect(sink.is_available(), content="false")
+    inspect(sink.close(), content="false")
+
+    ignore(@utils.remove_file_internal(test_path))
+    ignore(@utils.remove_file_internal(test_path + ".1"))
+    ignore(@utils.remove_file_internal(test_path + ".2"))
+  } else {
+    inspect(native_files_supported(), content="false")
+  }
+}

src/utils/file_backend_stub.mbt

@@ -12,6 +12,12 @@ pub fn open_file_handle_internal(path : String, append : Bool) -> FileHandle? {
   None
 }
 
+///|
+pub fn file_exists_internal(path : String) -> Bool {
+  ignore(path)
+  false
+}
+
 ///|
 pub fn write_file_handle_internal(
   handle : FileHandle,

src/utils/pkg.generated.mbti

@@ -25,6 +25,8 @@ pub fn default_style_tag_registry() -> @formatting.StyleTagRegistry
 
 pub fn field_equals(String, String) -> (@core.Record) -> Bool
 
+pub fn file_exists_internal(String) -> Bool
+
 pub fn file_size_i64_internal(FileHandle) -> Int64
 
 pub fn flush_file_handle_internal(FileHandle) -> Bool