From a48ad3cd61e030c9f2881bffb8252c4b75896d33 Mon Sep 17 00:00:00 2001
From: Zach Nelson <zach@zdnelson.com>
Date: Fri, 8 May 2026 12:29:21 -0500
Subject: [PATCH] chore: Updated docs to reflect DESTRUCTOR_CLOSES_FILE=1
 (#1878)

## Summary

Follow-up to 23aad213 which removed redundant FsFile close() calls from
the codebase. This updates CLAUDE.md so AI agents and contributors stop
reintroducing them. Adds the build flag to the Critical Build Flags
section with a clear "do not add file.close() on local FsFile variables"
rule, enumerates the three cases where explicit close is still required
(close before Storage.remove, close before reopening the same variable,
and member variables that outlive function scope), and updates the
HalStorage example, RAII guidance, and Activity Lifecycle sections to be
consistent with the new policy.

---

### AI Usage

While CrossPoint doesn't have restrictions on AI tools in contributing,
please be transparent about their usage as it
helps set the right context for reviewers.

Did you use AI tools to help write this code? _**YES**_
---
 .skills/SKILL.md | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/.skills/SKILL.md b/.skills/SKILL.md
index 90a00dd7..8e5e8f8f 100644
--- a/.skills/SKILL.md
+++ b/.skills/SKILL.md
@@ -104,8 +104,17 @@ These flags in `platformio.ini` fundamentally affect firmware behavior:
 -DUSE_UTF8_LONG_NAMES=1              // SD card long filename support
 -DMINIZ_NO_ZLIB_COMPATIBLE_NAMES=1   // Avoid zlib name conflicts
 -DXML_GE=0                           // Disable XML general entities (security)
+-DDESTRUCTOR_CLOSES_FILE=1           // FsFile destructor auto-closes (SdFat)
 ```
 
+**DESTRUCTOR_CLOSES_FILE implications**:
+- SdFat's `FsBaseFile` destructor calls `close()` automatically when the object goes out of scope
+- **Do NOT add explicit `file.close()` calls** for local `FsFile` variables — the destructor handles it
+- Explicit `close()` is still required in these cases:
+  1. **Close before delete**: Must close before `Storage.remove()` on the same path
+  2. **Close before reopen**: Must close before reopening the same `FsFile` variable (e.g., write then reopen for read, or rewrite the same path)
+  3. **Member variables**: `FsFile` members persist beyond any single function scope, so close at the intended release point (e.g., in `onExit()`)
+
 **SINGLE_BUFFER_MODE implications**:
 - Only ONE framebuffer exists (not double-buffered)
 - Grayscale rendering requires temporary buffer allocation (`renderer.storeBwBuffer()`)
@@ -145,11 +154,11 @@ These flags in `platformio.ini` fundamentally affect firmware behavior:
 FsFile file;
 if (Storage.openFileForRead("MODULE", "/path/to/file.bin", file)) {
   // Read from file
-  file.close();  // Explicit close required
+  // No file.close() needed — DESTRUCTOR_CLOSES_FILE=1 handles it at scope exit
 }
 ```
 
-**Usage**: See example above. Uses `FsFile` (SdFat), NOT Arduino `File`.
+**Usage**: See example above. Uses `FsFile` (SdFat), NOT Arduino `File`. Do NOT add `file.close()` for local variables (see DESTRUCTOR_CLOSES_FILE above).
 
 ---
 
@@ -167,7 +176,7 @@ if (Storage.openFileForRead("MODULE", "/path/to/file.bin", file)) {
 
 ### Memory Safety and RAII
 * Smart Pointers: Prefer std::unique_ptr. Avoid std::shared_ptr (unnecessary atomic overhead for a single-core RISC-V).
-* RAII: Use destructors for cleanup, but call file.close() or vTaskDelete() explicitly for deterministic resource release.
+* RAII: Use destructors for cleanup. Call `vTaskDelete()` explicitly for deterministic task release. Do NOT call `file.close()` on local `FsFile` variables — `DESTRUCTOR_CLOSES_FILE=1` handles it at scope exit (see Critical Build Flags).
 
 ### ESP32-C3 Platform Pitfalls
 
@@ -376,13 +385,13 @@ void enterNewActivity(Activity* activity) {
 - Activity navigation = `delete` old activity + `new` create next activity
 - Any memory allocated in `onEnter()` MUST be freed in `onExit()`
 - FreeRTOS tasks MUST be deleted in `onExit()` before activity destruction
-- File handles MUST be closed in `onExit()`
+- Member `FsFile` handles MUST be closed in `onExit()` (local `FsFile` variables auto-close via destructor)
 
 **Activity Pattern**:
 ```cpp
 void onEnter()  { Activity::onEnter(); /* alloc: buffer, tasks */ render(); }
 void loop()     { mappedInput.update(); /* handle input */ }
-void onExit()   { /* free: vTaskDelete, free buffer, close files */ Activity::onExit(); }
+void onExit()   { /* free: vTaskDelete, free buffer, close member FsFiles */ Activity::onExit(); }
 ```
 
 **Critical**: Free resources in reverse order. Delete tasks BEFORE activity destruction.