fix: preserve ebook download write failures

Author: DevanMetz

.github/workflows/release.yml

@@ -49,6 +49,8 @@ jobs:
         run: python3 tools/test_ebook_pages.py --compiler cc
       - name: Test ebook library
         run: python3 tools/test_ebook_library.py --compiler cc
+      - name: Test ebook downloads
+        run: python3 tools/test_ebook_download.py --compiler cc
       - name: Test capture writers
         run: python3 tools/test_capture_write.py --compiler cc
       - name: Test serial log writers
@@ -277,6 +279,22 @@ jobs:
           if-no-files-found: warn
           retention-days: 14
 
+      - name: Retain ebook download evidence
+        uses: actions/upload-artifact@v4
+        if: always()
+        with:
+          name: ebook-download-logs
+          path: |
+            build/ebook-download/compile.log
+            build/ebook-download/result.log
+            build/ebook-download/source.json
+            build/ebook-download/ebook_download.inc
+            build/ebook-download/ebook_download_types.inc
+            build/ebook-download/ebook_download_config.inc
+            build/ebook-download/storage_source.inc
+          if-no-files-found: warn
+          retention-days: 14
+
       - name: Retain capture writer evidence
         uses: actions/upload-artifact@v4
         if: always()

ROADMAP.md

@@ -69,6 +69,7 @@ Phase 1 software progress is also underway:
 - [x] Notes loads complete files up to 65,535 bytes, replacing a silent 2,047-byte prefix. Recovery/read/close errors, oversized files and NUL data disable Save; byte-length and SD checks run before storage changes. Its launcher leave hook clears editor ownership. Four preceding-source controls reproduce full-note truncation; 36 native file/fault cases and five real LVGL cases pass, including maximum ASCII/UTF-8 notes and 25 reopens without retained resources in the unchanged 96 KiB pool. Linux and Windows CI retain separate source-bound evidence. Physical SD/panel and power cuts remain open.
 - [x] Notes confirms directory/TMP preparation before opening a writer and preserves each load/write failure through cleanup. Missing errno uses EIO; a failed open cannot become an empty editable note, and a stream-flagged positive write cannot publish. All 63 native file/fault cases and 12 real LVGL cases pass; the preceding callbacks fail 18 and six controls respectively. Exact final/backup/TMP bytes, blocked stale Save events, accepted existing-directory/missing-TMP responses and 25 failure/retry pairs are checked with closed resources and unchanged LVGL allocation counts. Logical directory/mount responses, faults and PSRAM APIs are controlled. CI retains source hashes and logs; physical SD/panel and power cuts remain open.
 - [x] Ride history reports recovery/open/read/close errors as unavailable instead of showing zero or partial totals. Summary appends check SD availability and preparation errors, reject failed read chunks before copying, and retain the first error through cleanup. All 50 native file/fault cases pass; the preceding callbacks fail 24 controls. Exact final/backup/temporary bytes and 25 reopens are checked with closed modeled handles. Linux CI retains source hashes and logs, and the unflashed firmware builds with unchanged configuration. Physical SD/panel and power cuts remain open.
+- [x] Default ebook downloads capture their first write error independently of ignored SDK callback returns, stop later file writes and reject publication after failed chunks. Probe/TMP preparation failures preserve existing bytes; missing errno uses EIO and cleanup cannot replace the first cause. All 58 native callback/storage cases pass; the preceding callbacks fail 32 controls. All three default profiles, status/installation boundaries, retained TMPs, init/transport/status cleanup failures and 25 failure/retry/installed-recheck cycles are checked with closed controlled clients and native streams. Linux CI retains source hashes and logs. The unflashed firmware builds with the actual SDK; worker/timer scheduling, Wi-Fi/TLS, physical SD/panel and power cuts remain open.
 - [x] Ride history and summary writers now isolate errno at every I/O boundary. Failed opens cannot become empty history or replace an existing index; flagged positive-count reads/writes stop before later parsing, writes or publication, preserving the first cause through cleanup. All 95 history and 41 recording cases pass; the preceding history callbacks fail 32 controls and the Stop flow fails one additional missing-errno source-open control. Exact generations, retained TMPs, four 25-pair failure/retry paths and successful reloads are checked with closed resources. A published ride remains saved when its index update fails, and the failure is reported separately. Logical directory/mount responses, UI, BLE data, time and faults are controlled. The unflashed firmware builds; physical SD/panel and power cuts remain open.
 - [x] Ride Start checks both directories, stops on noncollision creation errors and preserves their first cause. Rejected headers/rows, periodic sync failures and known SD loss retain the named TMP and clear recording ownership without retrying publication. All 39 native recording/history/storage cases pass; the preceding source fails 27 controls. Exact CSV/index/backup bytes, 100-name limits, tick wrap and 25 same-second restart pairs are checked with closed streams/descriptors. Linux CI retains source hashes and logs. Physical SD, BLE, panel and power cuts remain open.
 - [x] Ring logging commits its saved timestamp only after tail recovery and a complete, successfully closed read. Failed loads retain queued samples and defer history requests until a successful retry; known SD unavailability also blocks a cached checkpoint. Appends check directory errors and preserve the first seek/tell/header/row error through close and repair. All 59 native file/queue/fault cases pass; the preceding callbacks fail 32 controls. Checks verify exact bytes, retry/deduplication, timeout handling while blocked and 25 checkpoint reopens with closed modeled handles. Linux CI retains source hashes and logs. Physical SD, BLE, panel and power cuts remain open.

docs/architecture.md

@@ -109,6 +109,8 @@ Files directory browsing resets errno before each directory read so per-entry `s
 
 The ebook library verifies regular-file metadata before offering a `.txt` entry, excluding matching directories and unconfirmed paths. Directory setup/open failures show their cause; read/stat/close failures keep confirmed rows visible with an incomplete-list warning. Fresh errno checks preserve the first failure, and failed or skipped listings cannot claim the library is empty. The existing 64-entry and 256-byte path bounds are disclosed. Native checks use an existing BOOKS directory with controlled UI, timer, SD, download and fault boundaries; welcome creation and physical storage remain separate checks.
 
+Default ebook downloads keep the file pointer and first write error in a per-download stack context passed to the HTTP callback. The pinned SDK body parser ignores callback return codes, so this context stops later file writes and gates publication independently of the client result. Fresh errno at probe/preparation/write/close boundaries prevents stale causes; close/removal cannot overwrite a captured write error. Confirmed 2xx downloads still use the existing new-file commit helper. Callback state lives until SDK cleanup finishes, then file ownership is closed or consumed by storage. Native file tests control HTTP events and faults; the unflashed build checks the actual SDK interface. Worker/timer scheduling, Wi-Fi/TLS, physical SD and panel behavior remain separate checks.
+
 Scope captures the first rejected header/row write before cleanup and closes an invalid temporary stream without attempting publication. I2C uses a shared finish path for explicit Stop and error cleanup: known row/flush/sync/SD failures close and release recording ownership, preserve the first cause, and report once; only a healthy Stop attempts publication. This prevents a successful retry during cleanup from turning an earlier failure into a Saved result. Native bytes and closed handles are checked with controlled UI/data/time/fault boundaries; sampling, drivers and physical SD behavior remain separate gates.
 
 Scope and I2C constructors clear errno before each directory/metadata/open/stream boundary and stop on a non-collision failure. Scope closes the descriptor, attempts to remove its new empty TMP and reports a failed stream conversion without another filename attempt. I2C retains that empty file, and clears capture paths/ownership after a rejected header. Its watch flag is enabled only after an accepted header. Native constructor checks verify both 100-name bounds, retained-TMP restart, first errors and closed handles; existing path buffers, sample data, CSV formats and watch timing remain unchanged.

docs/data-formats.md

@@ -41,6 +41,8 @@ Choose a record from the current page and tap **COPY ROW** to put its original 1
 
 The ebook library accepts case-insensitive `.txt` names only after confirming regular-file metadata, and skips directories or entries with failed metadata. It stores at most 64 paths of 256 bytes, disclosing filled slots and overlong names. Setup/open errors show their cause; read/stat/close errors mark retained confirmed entries incomplete and reach the SD callback with the first failure. The copy-books hint appears only after an error-free listing with no accepted or overlong entries. Native existing-directory checks cover these transitions with controlled UI, timer, mount and error boundaries; welcome creation and physical SD/panel behavior remain separate checks.
 
+Default ebook downloads stop before opening a writer when the installation probe or old-TMP removal fails. Each write checks its own errno and stream flag, using EIO when a failure supplies no cause. The first failed chunk stops later file writes and prevents publication even if the HTTP client later reports success. A failed download attempts to close/remove its TMP and preserves the first storage error through cleanup; failed removal can retain partial bytes. Healthy 2xx downloads use the existing new-file sync/close/publication helper, which retains an unpublished TMP on commit failure and never overwrites an existing final. Native file checks use controlled HTTP events and faults; physical SD/TLS and power-loss behavior remain open.
+
 Scope and I2C capture failures never retry publication during cleanup. Rejected header/row writes and failed I2C periodic flush/sync stop recording into the current TMP, close it, preserve the first cause and report failure. Known SD unavailability also closes an active I2C capture before adding another row. Healthy explicit saves still use the existing flush/sync/close/rename helper and CSV formats. A failed TMP can contain partial or complete-looking bytes; check the named file before recovery. Native byte/fault checks do not establish physical durability or power-loss recovery.
 
 Scope and I2C creation advance through the 100 same-second names only on an existing CSV or TMP collision. Directory/stat/open/stream-creation failures stop the attempt, preserve the first cause through cleanup and use `EIO` when errno is absent. Scope attempts to remove its newly created empty TMP after a failed `fdopen`; I2C retains and names that empty file. A rejected I2C header retains its partial TMP and leaves capture ownership clear without enabling watch. Restart skips retained files and preserves their bytes; exhausting all names reports `EEXIST`.

docs/testing.md

@@ -17,6 +17,7 @@ Python 3 and Node.js are needed for the quick checks. The configuration checks a
 | Ring heart-rate logging, saved timestamps or queue processing | [Ring history storage](#ring-history-storage) and a firmware build. |
 | Ebook page loading or navigation | [Ebook pages](#ebook-pages) and a firmware build. |
 | Ebook library enumeration | [Ebook library](#ebook-library) and a firmware build. |
+| Default ebook downloads or their file publication | [Ebook downloads](#ebook-downloads) and a firmware build. |
 | Scope/I2C CSV writes or publication | [Capture writers](#capture-writers) and a firmware build. |
 | Scope/I2C capture or saved serial CSV viewers | [CSV viewer storage and UI](#csv-viewer-storage-and-ui), existing serial-copy checks and a firmware build. |
 | Files fallback text previews | [Files text previews](#files-text-previews) and a firmware build. |
@@ -207,6 +208,18 @@ Windows Find APIs and POSIX directory APIs supply the entries; native dot entrie
 
 The library retains 64 stored paths of 256 bytes. The entry-limit message means those slots filled; no extra read proves that more entries exist. Generated callback/config source, full main/function/test hashes and compile/result logs stay under `build/ebook-library/`. Linux CI retains `ebook-library-logs`, including available failure diagnostics. These 28 checks are separate from the Windows native total.
 
+## Ebook downloads
+
+```powershell
+python tools/test_ebook_download.py --compiler 'C:/path/to/clang.exe'
+```
+
+The portable runner compiles the actual `ebook_http_event`, `ebook_default_installed` and `ebook_download_default` functions, their actual book/download types and default profiles, and the complete `storage_io.c`. Its 58 native-file cases check all three default profiles, the existing 1,024-byte installation boundary, 200/299 success and 199/300/404 rejection, first-probe failures with existing or absent files, TMP removal/open failures, client-init/transport failures, rejected and full-count stream-flagged writes, flush/sync/close/publication errors, missing errno and compounded cleanup failures. Every case checks exact final/TMP bytes and no remaining controlled clients or file handles. Twenty-five rejected-write/retry/installed-recheck cycles verify recovery without a redundant client.
+
+The preceding callbacks fail 32 controls. The transport adapter delivers all three data chunks even after the callback returns failure, matching the ignored data-callback return in the pinned SDK's body parser. The download keeps its first write cause in per-download state, stops later file writes and rejects publication even if the modeled client returns success. Cleanup cannot replace that cause; failed removal can leave a partial TMP. Separate init/transport/status cleanup controls report close/removal failures with their own cause, preserving the first close failure when removal also fails. Probe/preparation failures stop before a writer/client is opened, preserving existing files. Successful commit behavior, default URLs, TLS bundle, User-Agent, 30-second I/O timeout and 1,024-byte SDK buffer stay the same. An existing small final is preserved and leaves the completed download as an unpublished TMP when new-file publication refuses the collision.
+
+Files are native; SDK types, events, status/transport results, SD error reporting and I/O faults are controlled. This fixture does not execute the SDK client, the download worker/timer, LVGL, Wi-Fi/TLS, physical SD or power-loss behavior. The unflashed firmware build checks the callback/config interface against the actual SDK. Generated callbacks/types/config/storage source, full input/executable hashes and compile/result logs stay under `build/ebook-download/`. Linux CI retains `ebook-download-logs`, including available failure diagnostics. These 58 cases are separate from the Windows native total.
+
 ## Capture writers
 
 ```powershell

main/main.c

@@ -208,6 +208,11 @@ typedef struct {
     const char *url;
 } ebook_default_t;
 
+typedef struct {
+    FILE *file;
+    int error;
+} ebook_download_state_t;
+
 typedef struct {
     gpio_num_t pin;
     const char *port;
@@ -3293,9 +3298,15 @@ static void weather_task(void *argument)
 
 static esp_err_t ebook_http_event(esp_http_client_event_t *event)
 {
-    FILE *file = event->user_data;
-    if (event->event_id != HTTP_EVENT_ON_DATA || !file) return ESP_OK;
-    return fwrite(event->data, 1, event->data_len, file) == (size_t)event->data_len ? ESP_OK : ESP_FAIL;
+    ebook_download_state_t *download = event->user_data;
+    if (event->event_id != HTTP_EVENT_ON_DATA || !download || !download->file) return ESP_OK;
+    if (download->error) return ESP_FAIL;
+    errno = 0;
+    if (fwrite(event->data, 1, event->data_len, download->file) != (size_t)event->data_len || ferror(download->file)) {
+        download->error = errno ? errno : EIO;
+        return ESP_FAIL;
+    }
+    return ESP_OK;
 }
 
 static bool start_voice_mic(void)
@@ -4395,59 +4406,82 @@ static bool ebook_default_installed(const ebook_default_t *book)
     char path[256];
     snprintf(path, sizeof(path), SD_PATH "/BOOKS/%s", book->filename);
     struct stat info;
-    return stat(path, &info) == 0 && info.st_size > 1024;
+    errno = 0;
+    if (stat(path, &info) != 0) {
+        if (!errno) errno = EIO;
+        return false;
+    }
+    errno = 0;
+    return info.st_size > 1024;
 }
 
 static bool ebook_download_default(const ebook_default_t *book)
 {
     if (ebook_default_installed(book)) return true;
+    if (errno != 0 && errno != ENOENT) {
+        int error = errno;
+        sd_record_error(error);
+        ESP_LOGE("tab5-os", "Could not inspect %s: %s", book->filename, strerror(error));
+        return false;
+    }
     char path[256];
     char temporary[256];
     snprintf(path, sizeof(path), SD_PATH "/BOOKS/%s", book->filename);
     snprintf(temporary, sizeof(temporary), "%s", path);
     snprintf(strrchr(temporary, '.'), 5, ".TMP");
+    errno = 0;
     if (remove(temporary) != 0 && errno != ENOENT) {
         int remove_error = errno ? errno : EIO;
         sd_record_error(remove_error);
         ESP_LOGE("tab5-os", "Could not clear %s: %s", temporary, strerror(remove_error));
         return false;
     }
+    errno = 0;
     FILE *file = fopen(temporary, "wb");
     if (!file) {
         int open_error = errno ? errno : EIO;
         sd_record_error(open_error);
         ESP_LOGE("tab5-os", "Could not create %s: %s", temporary, strerror(open_error));
         return false;
     }
+    ebook_download_state_t download = {.file = file};
     esp_http_client_config_t config = {
         .url = book->url,
         .event_handler = ebook_http_event,
-        .user_data = file,
+        .user_data = &download,
         .crt_bundle_attach = esp_crt_bundle_attach,
         .timeout_ms = 30000,
         .buffer_size = 1024,
     };
     esp_http_client_handle_t client = esp_http_client_init(&config);
     if (!client) {
-        fclose(file);
-        remove(temporary);
+        int stream_error = 0;
+        errno = 0;
+        if (fclose(download.file) != 0) stream_error = errno ? errno : EIO;
+        download.file = NULL;
+        errno = 0;
+        if (remove(temporary) != 0 && errno != ENOENT && !stream_error) stream_error = errno ? errno : EIO;
+        if (stream_error) sd_record_error(stream_error);
         ESP_LOGE("tab5-os", "Could not start download for %s", book->filename);
         return false;
     }
     esp_http_client_set_header(client, "User-Agent", "Tab5OS/1.0");
     esp_err_t error = esp_http_client_perform(client);
     int status = esp_http_client_get_status_code(client);
     esp_http_client_cleanup(client);
-    bool downloaded = error == ESP_OK && status >= 200 && status < 300;
+    bool downloaded = error == ESP_OK && status >= 200 && status < 300 && !download.error;
     bool saved = false;
     if (downloaded) {
-        saved = storage_commit_new_file(&file, temporary, path) == 0;
+        errno = 0;
+        saved = storage_commit_new_file(&download.file, temporary, path) == 0;
         if (!saved) sd_record_error(errno ? errno : EIO);
     } else {
-        int stream_error = ferror(file) ? (errno ? errno : EIO) : 0;
-        if (fclose(file) != 0 && !stream_error) stream_error = errno ? errno : EIO;
-        file = NULL;
-        remove(temporary);
+        int stream_error = download.error;
+        errno = 0;
+        if (fclose(download.file) != 0 && !stream_error) stream_error = errno ? errno : EIO;
+        download.file = NULL;
+        errno = 0;
+        if (remove(temporary) != 0 && errno != ENOENT && !stream_error) stream_error = errno ? errno : EIO;
         if (stream_error) sd_record_error(stream_error);
     }
     ESP_LOGI("tab5-os", "Default ebook %s: %s (%d)", book->filename, saved ? "saved" : "failed", status);