# V-426: independent verification of PR #426

- PR: https://github.com/chhoumann/PodNotes/pull/426, `fix(persistence): retry pending saves after read failures` (follow-up to #425: F1, N1, N2)
- Head: `cb8d43ea94cc47c21f5a9a615e0e5e09f55ba2a4`, based on master `48566ab`. The base is current origin/master, and the merge state is `CLEAN` / `MERGEABLE`.
- Worktree: `/home/christian/.t3/worktrees/PodNotes/verify-426`, left detached and clean at the head with no ignored files.
- Verifier: Claude Opus 5.5 (claude-opus-5-5). Inspect only: no tracked file changed and no GitHub writes.
- Evidence: `/home/christian/.claude/orchestrate/podnotes/evidence/V-426/`, in `gates/`, `tests/`, `live/` and `scripts/`.

## Verdict

**PASS (live-ui-verified).** No blocking findings.

All three claims hold in code, unit tests, mutation runs and real Obsidian 1.14.4. Known limit 1 is worse than the PR author describes: it loses later edits, not just leaves a stale Notice. It is still a residue of master's bug, not a regression. On master the same scenarios lose everything, and on the head only a narrower path does. I file it as follow-up F1 below, with a fix. Limit 2 loses no data (follow-up F2).

## 1. Behavior

The production diff is `src/persistence/writer.ts` only, +20/-6 lines:

- **`persist(warnOnFailure)`.** `save()`, the strict path used by settings import and API-key selection, passes `false`. Background saves, checkpoints and retries pass `true`.
- **`reportFailure`.** It is computed before the read (`writer.ts:187`), so a read that settles an earlier failed write does not restart the streak.
- **Read retry.** A failed pre-write read now calls `scheduleCheckpoint()` when a failed write or unsaved edits are pending (`writer.ts:191-196`).
- **Skip removed.** dd7ce70's skip-after-settle was removed in cb8d43e. `settleFailedWrite` is `void` again, and the merged write always runs.

Callers I read:

- `main.ts`:
  - `createWriter`'s `onWriteFailed` shows "PodNotes could not save your changes."
  - `saveSettings`, `saveSettingsStrict` and `flushSaves` are each gated on `writeRefusal`.
  - In `dataFileAccess.read`, `refuseSaves` and `liftUnreadableRefusal` set and clear the sticky Notice.
  - `writeDataFile` raises `WriteRefusedError` and reads the file back after each write.
- `PodNotesSettingsTab.ts`: the import (954/975/1012) and API-key (1113/1118) strict callers.

**Retry never runs when nothing is pending.** The read is reached only when `failedWrite` or unsaved edits exist, or when `base` is null. The catch schedules only under the same condition, so with `base` null it never schedules. The test `schedules no retry of a failed read before anything was loaded` guards this. Probe P3 (`tests/probes-head.txt`): 5 user edits during the refusal caused 0 reads, because `saveSettings` is gated.

**Can't loop faster than the checkpoint.**
- The delay is `lastSaveAt + 10 s - now`, and `lastSaveAt` is set before the read.
- `saveLogged(true)` re-checks and re-arms for the rest of the interval. In P3, a timer fired at +3 s re-armed for 7,000 ms, and exactly 1 read ran at +10 s.
- Live retry gaps on the head were 10,000, 10,004, 10,004 and 10,000 ms.

**Stops on unload.** `scheduleCheckpoint`'s `unloaded` guard covers the read path too. In P4, an unload during a read outage made one flush attempt and left no timer (`armed 1 -> pending 0`).

**A reconciled write still writes repaired raw entries.** Reintroducing the skip makes the damaged-entry test fail with `expected { Before: null, Remote: {...} } to deeply equal {...}`. The raw `null` entry stays on disk, because `read()` restores it from `base`, so the read-back matches `failedWrite.written`. Without the skip, the write repairs it.

**Known limit 1: reverting all pending edits.** It is a data-loss path for later edits, and its state misleads the user. Reproduced in unit probes P1 and P1b and live (`live/phaseRevert-head.log`, `live/revert-head/stranded.png`):

1. Reads fail. The user sets Skip forward to 25, and the read fails, which sets the refusal and arms the retry.
2. The user sets Skip forward back to 15 (gated). The retry fires, finds nothing pending, returns early and arms nothing.
3. The disk recovers. The user sets Skip backward to 33.
4. After 25 s and `flushSaves()`, disk is unchanged (`6274a232`) and memory holds skipB 33. Refusal is `unreadable-data-file`, the timer is null, and the sticky Notice is still visible.

Only an external data.json change or a plugin reload saves the edits. Quit sets `obsidian-closing` before the unload flush, so the edits are lost. The sticky Notice says PodNotes "saves nothing until the file can be read again". A user who sees the disk recover would reasonably keep editing and quit.

P1b shows a second route that needs no deliberate revert. A settings import's strict save fails on the read, and its rollback, rejected at once by the refusal gate, restores the previous settings. Nothing is pending, so the same state follows.

On master's `writer.ts` (`tests/probes-master.txt`), P1, P1b and the control P1c all strand edits. The head saves P1c (the same outage without the revert): disk 25/33, the Notice is hidden, no timer is left. So this is not a regression; the PR fixes all but this path. Classified as **follow-up F1**.

**Known limit 2: older-schema read-only retries.** No data loss, and nothing the user would act on wrongly. P2: with an older-schema file and pending edits, there were 6 reads a minute, 0 writes and one Notice, which correctly says to restart. The cost is one data.json read and one `console.error` every 10 s for the rest of the session, since playback keeps work pending. P6 shows the same class for a newer-schema file synced in at runtime: 6 reads a minute, 0 writes and **no Notice**. That silence is pre-existing on master, which also never saves and shows nothing. Only the retries are new. Classified as **follow-up F2**.

**Observation, not a defect.** P5: a strict save whose *read* fails shows the sticky unreadable Notice, plus the caller's Notice in the real UI. That is pre-existing and a different message: a state Notice, not the generic write Notice N2 removed.

## 2. Tests

**New tests fail on master's `writer.ts`.** Command and results are in `tests/new-tests-on-master-src.log`; the source was restored and verified by hash. 6 of 95 tests in the three changed files fail:

- `saves an edit whose save could not read the file once it reads again` (disk lacks skipForwardLength 25)
- `warns once per streak while writes and then reads fail, and saves every edit once both work`
- `rolls back a SecretComponent reference when strict persistence fails` (two Notices)
- `rolls a failed import back to the podcasts it replaced, keeping the synced one` (two Notices)
- `warns once when neither the import nor its rollback saves...`, both the `intact` and `unreadable` variants

**Mutations.** Each ran through the full unit suite of 1,736 tests (`tests/mutations/summary.txt`; `writer.ts` was restored byte for byte, checked with `git hash-object` and an empty `git status`).

| Mutation | Result |
| --- | --- |
| drop-pending-guard (retry a failed read with nothing pending) | killed: `schedules no retry of a failed read before anything was loaded` |
| drop-read-retry (master's behavior) | killed by both read-outage tests |
| drop-refusal-rethrow | killed: `starts no write once the window unloads...`, by the `clock.pending()` assertion at :377 |
| strict-shows-generic-notice (`save()` passes `true`) | killed by 4 tests: the API-key test and 3 import tests |
| reintroduce-reconciled-skip | killed by both damaged-entry variants: the raw `Before: null` stays on disk, and 2 writes instead of 4 |
| notice-decided-after-settle (dd7ce70's check) | killed by both "neither saves" variants |
| read-retry-immediately (0 ms read retry) | killed by both read-outage tests (`clock.delays`) |
| refusal-notifies-background (a refused background write shows the generic Notice, no retry) | **survives**, see nit N1 |

The tests are not tautological. They drive `saveSettings`, `saveSettingsStrict` and the settings tab's `applyImportedSettings` the way the app does. They assert exact Notice text and arguments, disk contents, write counts and timer delays.

## 3. Gates

Node `v22.23.2`, npm 10.9.8, via `fnm exec --using 22.23.2 --`, with `TMPDIR=/data/home/tmp/v426` (`gates/summary.txt` and per-gate logs):

| Gate | Result |
| --- | --- |
| npm ci | pass |
| npm run lint | pass: oxlint clean; Svelte policies 0 errors, 0 warnings |
| npm run format:check | pass, 336 files |
| npm run typecheck | pass |
| npm run build | pass |
| npm run test | pass: svelte-check 0 errors and 0 warnings on both configs; 99 files, 1,736 tests |
| npm run test:tooling | pass, 10 tests |
| npm run size | head 285,447 raw / 90,856 gzip against master (48566ab source) 285,338 / 90,816, so **+109 raw / +40 gzip**, as claimed |
| npm run docs:build | pass: 18 pages; all 112 legacy fragments resolve |
| npm run test:e2e (head, once) | **77 passed in 12 files**, rc 0, 244 s |

## 4. Runtime: real Obsidian 1.14.4

**Setup.**
- Isolated instance from `npm run start:e2e-obsidian`, with `XDG_RUNTIME_DIR` unset (vault `podnotes-verify-426`).
- Doctor: vault `podnotes-verify-426`, api and uri true, `dev:errors` clean, `version` `1.14.4 (installer 1.14.4)`.
- Fixture feed `http://podnotes.test:22599/feed.xml` (PodNotes Fixture), added through Settings.
- Builds: master bundle `f4a2158a` (built from 48566ab's `writer.ts`) and head bundle `99113210` (`live/bundles.txt`). The head bundle alone contains `persist(!1)`.
- Each run started from the same baseline data.json (`6274a232`, `scripts/prep.sh`) and a plugin reload.

**Fault injection.** `scripts/probe/install.js` wraps `adapter.write` and `adapter.read` for the exact data.json path only. It records every attempt with epoch-ms times and logs `.notice` nodes once each. Every phase restored the adapter and the original data.json in an EXIT trap, and I checked each restore by hash; all show `6274a232`.

**User paths**, driven with `ui.mjs`:
- Settings > PodNotes > Skip backward and Skip forward
- Podcast grid > PodNotes Fixture > Fixture Episode 1 or 2, then Toggle playback
- Settings > Import settings > Import, then the confirm modal's Import

Only the OS file dialog is stubbed: `scripts/probe/filepick.js` delivers the file through a real `change` event.

### (e) sequence, recorded

No user action happens after step 5. Driver: `scripts/driverE.sh`; logs: `live/phaseE-{master,head}.log`.

1. Writes fail.
2. The user sets Skip backward to 35.
3. Reads fail too.
4. The user plays Fixture Episode 1 for 8 s, pauses, and sets Skip forward to 25.
5. Reads recover while writes still fail.
6. Writes recover.

| | Master | Head |
| --- | --- | --- |
| After writes recover | disk `6274a232` unchanged for 34 s (skipF 15, skipB 15, no progress), against memory 25/35/8.02 s | retry wrote at 1791851731653, 4.1 s after restore; disk `77235fb7`: skipF 25, skipB 35, Episode 1 at 7.942482 |
| Notices | generic + sticky; the sticky stays visible to the end (`live/master/frame-70s.png`) | 1 generic + 1 sticky over the write, read, write streak; no second generic Notice when reads recovered and the write failed again |
| Sticky Notice | never cleared; refusal stuck | cleared when the retry read succeeded (`live/head/frame-25s.png` shows it, `frame-40s.png` doesn't); refusal null |
| Retries | none | at +10,000, +10,004, +10,004, +10,000 ms |
| Idle after save | n/a | 20 s with 0 writes, timer null |

Recordings: `live/{master,head}/sequence.mp4`, with contact sheets and extracted frames.

Master's first run (`live/master-run1/`) is superseded: my driver's Settings close click dismissed the sticky Notice. The rerun closes Settings with blur and Escape.

### Failed settings import

The strict path. OPML import doesn't use `saveSettingsStrict`; it takes the generic path.

- **Plain import** (`live/phaseImport-*.log`): writes fail, then import `{skipForwardLength: 44, skipBackwardLength: 44}`.
  - Master shows **2** Notices: "PodNotes could not save your changes." and "Could not import PodNotes settings. The failed import was rolled back...".
  - The head shows **1**, the import's own.
  - The rollback counts as saved because the file already holds the rolled-back state (`writeDataFile`'s read-back).
- **Import with retries** (`live/phaseImport2-*.log`, screenshots `live/import2-{master,head}/after-import.png`): the import runs while Episode 2 plays with unsaved progress, so the rollback differs from disk and fails too.
  - The head shows exactly **1** Notice, "Could not import PodNotes settings or persist its rollback...", through 4 failed writes (import, rollback, pause, 10 s retry).
  - After writes recovered, the retry saved the rollback: skipF 15, not 44, with Episode 2 at 19.937562. 0 writes followed in 12 s idle.
  - Master showed the generic Notice plus the import Notice.

### count:writes parity

Normal session (`live/count-*.txt`): play Fixture Episode 2 for 24 s, pause, then 11 s idle, from `6274a232`.

| Build | Attempted | Completed | Failed | Bytes |
| --- | --- | --- | --- | --- |
| master | 4 | 4 | 0 | 11,921 |
| head | 4 | 4 | 0 | 11,921 |

### Other checks

- Every screenshot and frame was opened and checked. There is no UI defect from this PR, and no flicker: consecutive frames are identical apart from the cursor.
- The sticky refusal Notice covers the PodNotes view's top tabs while it shows. That is Obsidian's placement and pre-existing.

## 5. GitHub checks on cb8d43e

All 15 check runs completed successfully, except the informational Dependabot automation, which was neutral (`gates/github-checks.txt`):

- Test, Unit tests, Svelte checks and tooling tests
- Lint and format, Build and sizes, Docs
- Validate PR title (two runs), Dependency Review
- CodeQL, Analyze (javascript-typescript), Analyze (actions)
- Macroscope - Correctness Check, Cloudflare Pages

The commit status (Devin Review) is success. There are no review comments. The PR body has a "Docs at release" section and no em-dash; the diff and commits have none either.

## Findings

None blocking.

## Follow-ups (non-blocking; file as issues)

**F1. Reverting every pending edit during a read outage stops the retry, and later edits are never saved** (`src/main.ts:544-546` with `src/persistence/writer.ts:185,194`).

- **Impact:** after a read outage in which the pending edits were reverted, or a failed import was rolled back, edits made after the disk recovers stay in memory only. They are lost on quit or app kill. The sticky Notice keeps saying saves resume "until the file can be read again".
- **Scope:** pre-existing. Master loses every such edit. The head narrows it to this path.
- **Cause:** `saveSettings` and `flushSaves` drop every request while `writeRefusal` is set. The writer's retry is the only thing that reads, and it stops once nothing is pending.
- **Fix:** while only the runtime `unreadable-data-file` refusal holds, hand the request to the writer instead of dropping it. For example, add `DataFileWriter.retryPending()`, which calls `scheduleCheckpoint()` when `base` is set and work is pending, and call it from `saveSettings` in that state. The retry's read then lifts the refusal and writes, at most every 10 s.
- **Keep the startup refusal gated:** with `base` null, `persist` would write memory over the unread file (`toWrite = local`).
- **Test:** extend `saves an edit whose save could not read the file once it reads again` with a revert before the first retry and a new edit after recovery. Assert that the edit lands and that `clock.delays` stay at 10,000.
- **PR description:** the author's description of this limit ("can leave the unreadable Notice up") understates it. Record the real impact in the issue.

**F2. Read-only retries while a retry can't succeed** (`src/persistence/writer.ts:194`).

- **Cases:** an older-schema file (the `older-data-file` refusal lasts until restart) and a newer-schema or invalid file synced in at runtime (`PodNotesDataError`). The writer reads data.json and logs an error every 10 s for the rest of the session, while work is pending. No data is at risk.
- **Optional fix:** don't schedule after a read failure that a retry can't cure. For example, `dataFileAccess.read` could throw `WriteRefusedError` for `older-data-file`, and the catch could skip scheduling for it and for `PodNotesDataError`.
- **Separate, pre-existing issue:** the newer-schema runtime case never tells the user that saves stopped (P6, master too). It deserves its own unit with a Notice like the older-schema one.

## Nits

- **N1. The unload test's `notice` assertion can't fail** (`src/main.persistence.test.ts:376`). The test drives only strict saves, and `persist(false)` never calls `onWriteFailed`.
  - The rethrow's removal is still caught, by the `clock.pending()` assertion.
  - The rethrow's other job is still untested: keeping the generic Notice off a refused *background* write, such as the unload flush after `obsidian-closing`. Mutation `refusal-notifies-background` passes all 1,736 tests.
  - Fix: add a background case, for example a pending edit, `writeRefusal = "obsidian-closing"`, then `plugin.writer.unload()`, and assert no Notice and no timer.
- **N2. The read-retry catch lost its why-comment in cb8d43e** (`src/persistence/writer.ts:193`). Why a failed read retries isn't visible from writer.ts: only a read lifts the unreadable refusal, and `saveSettings` drops requests while it holds. dd7ce70's one-line comment said this; it is the crux of F1. Optional.

## Cleanup

- Deleted the outputs I created: `node_modules`, `docs/node_modules`, `docs/dist`, `docs/.astro`, `build/`, `main.js` and `.obsidian-e2e-vaults`. `git status --short --ignored` is empty at `cb8d43e`.
- Deleted the scratch directory `/data/home/tmp/v426`. Its drivers are archived in `evidence/V-426/scripts/`, and the temporary `src/v426-scratch.test.ts` is copied to `evidence/V-426/tests/`.
- Stopped my fixture server and my Obsidian instance: `stop:e2e-obsidian` terminated 9 processes, and `--dry-run` reports `pids: none`. No process with a `verify-426` HOME or command line is left. Other units' instances were not touched.