diff --git a/AGENTS.md b/AGENTS.md index 24a0831..b635a62 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -129,7 +129,12 @@ what a generic reviewer misses: re-derive either row from `is_frozen()` re-litigate this. (c) The deletion raise **does not branch on the error variant** — `SyncFailed` and `Io` both raise. §8 ※1's silent power-loss window is the *Committed* window; the Tombstone window is zero by §6, so a failed flush is - a real breach. (d) The cover is tied to a **durable checkpoint save**, not + a real breach. The *marker* that carries the raise across a restart (#312) + does distinguish them, and that is not a contradiction: within a session + neither half is durable, so both are reported the same; across a restart + `SyncFailed`'s frame is replayable, so reporting it after replay applied the + deletion would be a latching alarm about data that is gone. Which is why the + marker records a seq for that half and nothing for `Io`. (d) The cover is tied to a **durable checkpoint save**, not to the WAL truncation that follows: truncation is the physical scrub of superseded frames, and gating on it leaves a permanent warning whenever frames land mid-run (`FollowUp`) or the truncate fails on an otherwise @@ -148,8 +153,17 @@ what a generic reviewer misses: "are there rows", not on the engine being degraded — because the main #295 scenario is a clean launch followed by a later failure. Findings proposing to collapse the list, treat `SyncFailed` as benign, gate the cover on - truncation, or merge the issues into `initFailures` re-litigate these — do - not raise them. The "no commit-side ledger" clause that used to sit in this + truncation, or merge these two issues into `initFailures` re-litigate these + — do not raise them. The dividing line is **retraction, not provenance**: a + durability fact the *disk* can retract belongs on this list, and one no + amount of recovery can retract belongs with the latching startup failures. + `EngineInitFailure.historyDeletionLost` (#312) is the second kind — the + deletion is already lost and only the user deleting again resolves it (two + user actions retire the *row*: acknowledging it, and a wipe that makes the + claim false; neither is the disk healing) + — so it is not a breach of this entry, and proposals to move it onto the + runtime list (or to fold the runtime rows into it) contradict the same rule + from the other side. The "no commit-side ledger" clause that used to sit in this list is **withdrawn**; see (b). Deliberately **out of scope**, for two different reasons — do not merge them. (i) a persistent `spawn_compact` thread-spawn failure leaves `scrub_pending` @@ -157,13 +171,197 @@ what a generic reviewer misses: scrub compaction's `save()` fails raises nothing (#311), so deleted strings sit in the old checkpoint and past Committed frames for the session. Both are deferred *physical scrubs*: the deletion itself is durable and startup heals - it. (iii) is **not** like them — the ledger is process-local, so on the `Io` + it. (iii) was **not** like them — the ledger is process-local, so on the `Io` half, where no frame reached the WAL and no checkpoint landed, the deletion is - not durable, startup does not heal it, and the report is gone on the very - restart where the entry resurrects (#312). **#295 is therefore only - half-closed**, with #312 as the remainder; it wants an on-disk marker, which - is new persisted state and so its own PR. Do not cite this entry to suppress - that. Separately, #313 records a + not durable, startup does not heal it, and the report was gone on the very + restart where the entry resurrects. **#312 closed it** (and with it #295) via + the `.deletion-pending` sidecar. Its settled shape, all of it reached by + review rather than by first draft: + the checkpoint header's reserved bytes are unusable, because the raise + condition *is* a failed checkpoint write (and an in-place header rewrite + would recompute a CRC outside tmp+rename, risking the whole history to report + one deletion); only `NotFound` is clean, so every malformed or unreadable + marker reports and no CRC is needed — and that is enforced by decoding as a + **round trip against the writer** (only what `encode` emits is accepted) + rather than by validating fields, which missed four shapes in a row; the + reader also refuses to *open* anything that is not a regular file, since a + FIFO at that path would block the IME's startup thread forever, and `rename` + replaces a symlink, a FIFO or an unwritable file at the path rather than + writing through it, which also keeps a failed promotion from leaving a + witness that re-based seq numbers could satisfy — but `rename` guards only + the destination, so both the tmp `write_atomic` writes through and the + marker the reader opens go through one resolution + (`O_NOFOLLOW | O_NONBLOCK` plus an `fstat` on the obtained descriptor), + closing by construction the symlink that would truncate a file this crate + does not own and the readerless FIFO that would hang startup or the + key-processing thread; a check-then-open would leave the two resolutions a + substitution can race, which is why the reader stopped doing that; + `rename` promoting a *name* rather than the flushed descriptor stays open + and is stated rather than papered over — POSIX has no fd-based rename, a + uniquely-named tmp would break the marker-plus-orphan pair below, and + anything able to write in that directory can overwrite the destination + outright with no window to hit; + promotion of an `Unflushed{seq}` witness to `Lost` is **mandatory, not an + optimization** — a seq is a position within one WAL file, not an epoch, and + the witness test `seq > applied_seq` is an *inequality over a high water + mark*, so once that file is replaced any later frame above the witness + answers it "applied" whether or not the tombstone ever existed in the new + lineage; a **seq floor** was tried here and removed, because skipping the one + number the claim names changes nothing about an inequality (gaps are legal), + and the entry that claimed it gave "epoch discipline" was wrong: a floor on a + position creates no identity. While a report is owed and the disk does not + yet say `Lost`, appends **freeze** — the state that would answer the witness + must not advance until the lineage-independent form has landed; + a claim counts as landed when the **logical** marker holds it — the canonical + file *or* its flushed orphan tmp — because `write_atomic` syncs before it + renames, so a rename that fails has still made the bytes durable where the + next `read` merges them; treating that as failure froze appends on every + commit forever against a record that already said what was wanted (which + stage failed comes from `write_atomic_staged`, never from comparing the + bytes back — a `write_all` that completed before `sync_all` failed leaves an + image that compares equal without being durable, and calling that landed is + the power-loss hole the sidecar exists to close); + the **success** side says which stage it reached too (`Durable` / + `NameUnconfirmed`), because collapsing it into `Ok` reopened that same hole + from the other end: a rename whose parent-dir fsync failed is durable bytes + under a name that can roll back, and for this store the previous name is a + *weaker claim* — a just-promoted `Lost` falling back to a suppressible + `Unflushed{seq}`, or a first write vanishing entirely — while the process + had already recorded `Holds(Lost)` and let appends past the freeze. The + belief the writer hands back is `Unknown`, and everything else follows from + the machinery already there rather than from a new rule: no desired state is + satisfied, so every commit re-projects, and `flushed != Holds(Lost)` keeps + appends frozen until the promotion is durably named. Best-effort stays + correct for the checkpoint and the user dict, whose previous content is a + complete older state, and for the marker's *removal*, whose rollback + over-reports — the one direction this format may fail in; + a stronger record on disk **satisfies** a weaker desired one (the merge + lattice, not equality): `merge_write` absorbs a requested `Unflushed` back + into a surviving `Lost`, so exact equality made the desired state + unreachable and every commit paid another key-thread full sync forever; + `merge_write` returns the value it **persisted**, not the one requested, and + the belief records that — the write merges, so asking for `Unflushed` over a + surviving `Lost` leaves `Lost`, and a caller repeating its own request would + hold a belief the disk never had and skip the next reconcile as already + satisfied; + what the disk holds is a **three**-valued belief (`MarkerState`: + absent / holds-these-bytes / unknown), because a file nobody could read is + neither of the first two — collapsing it into absence let a failed unlink of + an unreadable marker look settled, and `Unknown` never satisfies a desired + state so the retry cannot be skipped; + an owed report whose disk does not yet say `Lost` **freezes appends** — the + one case where a sidecar may stop learning — whether the marker was + unreadable or holds a decoded `Unflushed` the promotion could not replace, + because either may name a sequence a later frame would answer and nothing + about numbering can prevent that; `frozen` already means "appending to this + file is not safe" (exempt: a disk that already says `Lost`, unsatisfiable by + any sequence, and a claim this session raised about the file it is still + appending to); + a write reports **which stage failed**, and the caller never infers it: bytes + read back cannot distinguish a flushed image from one that only reached the + page cache before `sync_all` failed, so `write_atomic_staged` says whether + the failure was pre-durability or the rename alone; + a failed write or removal sets the belief to `Unknown` rather than leaving + the old value — a short write can leave a truncated orphan the previous value + does not describe, and a stale `Absent` would satisfy a desired `None` and + skip the cleanup; + sequence exhaustion is refused **where the number is issued**, never carried + by `frozen`: a witness at the representable ceiling would otherwise make `+1` + panic across the UniFFI constructor in debug and hand out seq 0 in release, + and representing it as a freeze is wrong in kind — a compaction clears a + freeze by rewriting the file, and rewriting a file creates no numbers, so the + heal put the wrap straight back (adoption saturates instead, and `u64::MAX` + is refused rather than spent since assigning it leaves no successor); + a marker's *claim* and the *observation* of it are separate — anything + unreadable claims `Lost` by the fail-safe rule but confirms nothing, so only + a `confirmed` read reaches `marker_on_disk`, and `deletion_pending_checkpoint` + seeds the `session` claim as well as the ledger so the two cannot disagree + about what is outstanding; + **every commit reconciles the marker** — in `apply_records`, under the wal + mutex, before the appends — and that, not any per-event retry, is what makes + the disk agree with the projection; a record reconciled only at chosen events + is a cache with invalidation, which is why six review rounds each found a + different event with no retry behind it (ack, wipe, the compaction's cover, + recovery's removal, recovery's promotion, that compaction itself), and it is + affordable on the key path only because a matching `flushed` makes it a + memory comparison; before the appends because the harm is a WAL advancing + past an un-promoted witness; + **no compaction runs while a v1 checkpoint awaits its migration commit** + (`OpenReport::v1_checkpoint_retained`), and the refusal lives in + `run_compact_impl` — the single site that performs the write — not at any + scheduler: a compaction is not the migration (it writes a v2 checkpoint over + the v1 file with none of the commit's steps), and there are five schedulers + with nothing stopping a sixth, so gating them one at a time took three review + rounds and still left the scrub branch reaching it. The startup hint and both + schedulers do suppress, but only to avoid creating a worker that would refuse + — while the veto holds the threshold never clears, so a degraded session would + otherwise spawn an OS thread per commit from the key path. The veto is a + runtime cell seeded from the report and **released by `clear`'s commit point**, + because it is a fact about the bytes on that path and not about what startup + saw: a wipe writes an empty v2 checkpoint over the same file, superseding the + un-migrated copy on purpose, and a veto keyed on the startup observation would + outlive the bytes it protects and refuse the heal `clear` itself posts for a + failed physical step — a frozen WAL and a half-finished wipe with no retry, on + a flow that deliberately does not restart. The + legacy-WAL variant is exempt because there the compaction *is* the heal; + a startup retraction whose unlink fails hands the debt to + `compaction_recommended` for promptness alone + instead of a thousand frames later; the runtime seeds `MarkerClaims::flushed` + from `OpenReport::marker_on_disk` rather than assuming a clear disk, since + `None` is a positive claim that a failed promotion contradicts by leaving a + *suppressible* `Unflushed` under a memory claim of `Lost`, and `flushed` is + an observation rather than a claim, so a wipe folds `session` and leaves it + standing; + an empty loaded history and an empty durable set together settle a `Lost` + claim — either half alone was wrong once, and requiring both also removes + any need to tell an encoded `Lost` from the fallback a malformed marker + decodes to; the startup + counterpart of `clear` covering the ledger from its empty checkpoint, scoped + to `Lost` because `Unflushed` is about durability rather than presence and + replay can empty memory while the checkpoint still holds the entry; writes **merge** rather than replace, + `Io` absorbing, and go through the shared `write_atomic`; the tmp/rename gap + that once argued for a hand-rolled in-place write is closed by `read` merging + the orphan tmp, which can only strengthen a claim — writing by hand instead + cost three rounds of re-deriving durability details the shared primitive + already encodes (symlink replacement, unlink-failure checking, the new dir + entry's fsync); a claim + whose write failed is held in memory so the next raise re-asserts it; startup + **never retracts**, because a witness satisfied by replay was only satisfied + out of the page cache — it hands the claim to the runtime ledger for a durable + checkpoint to settle — and a witness that is *not* satisfied is promoted to + unconditional, because a WAL quarantine re-bases seq numbering and an + unrelated later frame would otherwise settle it; the compaction retraction + shares the wal guard with the ledger cover, because the window between a CAS + and a separate unlink cannot be pinned by a deterministic test — but it does + **not** retract an inherited report that nothing has delivered yet, since a + checkpoint written this session persists the *resurrected* entry and so + settles nothing about a previous session's claim; the logical marker is the canonical + file *and* the atomic write's orphan tmp — `read` merges the second, so + removal must clear both or a `Lost` orphan rebuilds the unclearable latch — + removal reports whether it is actually clear and an acknowledgement settles + only when it is (an empty + directory left there is a placeholder and gets cleared; a non-empty one is + someone else's and does not, which the return value makes visible rather than + silent); `clear` + removes the marker **unconditionally and separately**, since a previous + session's marker moves no counter in this one and the cover would early-return + past it when the ledger is untouched; and the acknowledgement happens on a + **click of the row**, neither at load nor at menu-build time — `bootstrap()` + runs on IMKit probe launches that never show a menu, and IMKit also builds + the menu without displaying it (measured: the record was consumed four + seconds after an untouched relaunch), so only a person clicking is evidence + the report was delivered. + One class stays open by construction and is documented rather than fixed: + the marker lives in the checkpoint's directory, so a failure of that whole + directory (read-only volume, EACCES, parent removed) takes the marker with + it. Findings proposing a header flag, a CRC, a plain overwrite, a tmp+rename + write, a cover outside the wal mutex, a *replay-evidence* retraction at + startup, an ack at load or at menu-build time, or folding `clear`'s wipe into + the cover re-litigate these — do not raise them. (Retraction against a + durable checkpoint at startup **is implemented**, not rejected, and so is the + single owed-predicate that decides whether the status row stays. Findings + about either are in scope — this list must never suppress review of the + mechanism it describes.) Separately, #313 records a pre-existing privacy race: `apply_records` appends to the commit log outside the wal mutex, so a commit in flight can re-create `commit-log.jsonl` after `clear` unlinked it. Findings re-raising any of these should point at the @@ -186,12 +384,17 @@ what a generic reviewer misses: for writing inside the wal critical section on every commit and a compaction holds for `cover_durable_residue`, so a merged ledger would put a main-thread menu poll behind history I/O, where the ledger's single atomic load blocks - on nothing. Reason (2) is the harder blocker. (2) **lex-core cannot see the `SyncFailed` raise.** - `apply_batch`'s witness is `(WalRecord, Option)` and a `SyncFailed` - tombstone carries `Some(seq)`, indistinguishable from a healthy one — by - design, since the residue deliberately excludes `SyncFailed` (its frame is - replayable). Merging would mean widening that witness to a three-state - durability value across a settled PR1 surface to serve a reporting concern. + on nothing. Reason (2) is the harder blocker. (2) **`apply_batch`'s witness must not widen.** + It is `(WalRecord, Option)`, and a `SyncFailed` tombstone carries + `Some(seq)`, indistinguishable from a healthy one — by design, since the + residue deliberately excludes `SyncFailed` (its frame is replayable). + Merging would mean widening that witness to a three-state durability value + across a settled PR1 surface to serve a reporting concern. (#312 put the + ledger's *on-disk projection* in lex-core, `user_history/deletion_marker.rs`, + so "lex-core never sees this distinction" is no longer the phrasing — lex-core + owns the file family and the format. What it still does not see is the raise + event: the engine classifies the failure and calls in. The blocker is the + witness, not the crate.) The two ledgers answer different questions: the residue asks "may the durable set still hold this key" (gating the no-op skip), the ledger asks "did this deletion reach disk at all" (reporting). Findings proposing to diff --git a/Resources/en.lproj/Localizable.strings b/Resources/en.lproj/Localizable.strings index d85077b..ac86e35 100644 --- a/Resources/en.lproj/Localizable.strings +++ b/Resources/en.lproj/Localizable.strings @@ -15,6 +15,7 @@ "⚠️ 学習履歴の一部を復旧できませんでした(学習は継続中)" = "⚠️ Some learning history could not be recovered (learning continues)"; "⚠️ 削除した学習内容を保存できませんでした(削除が取り消される可能性があります)" = "⚠️ A deleted entry could not be saved (the deletion may be undone)"; "⚠️ 新しい学習内容を保存できていません(再起動すると失われます)" = "⚠️ Recent learning is not being saved (it will be lost on restart)"; +"⚠️ 前回のセッションの削除が保存されていません(削除した内容が復元されている可能性があります。確認して再度削除してください)" = "⚠️ A deletion from a previous session was not saved (the deleted entry may be back; please check and delete it again)"; /* Settings Window */ "Lexime 設定" = "Lexime Settings"; diff --git a/Resources/ja.lproj/Localizable.strings b/Resources/ja.lproj/Localizable.strings index d961d93..5a350a3 100644 --- a/Resources/ja.lproj/Localizable.strings +++ b/Resources/ja.lproj/Localizable.strings @@ -15,6 +15,7 @@ "⚠️ 学習履歴の一部を復旧できませんでした(学習は継続中)" = "⚠️ 学習履歴の一部を復旧できませんでした(学習は継続中)"; "⚠️ 削除した学習内容を保存できませんでした(削除が取り消される可能性があります)" = "⚠️ 削除した学習内容を保存できませんでした(削除が取り消される可能性があります)"; "⚠️ 新しい学習内容を保存できていません(再起動すると失われます)" = "⚠️ 新しい学習内容を保存できていません(再起動すると失われます)"; +"⚠️ 前回のセッションの削除が保存されていません(削除した内容が復元されている可能性があります。確認して再度削除してください)" = "⚠️ 前回のセッションの削除が保存されていません(削除した内容が復元されている可能性があります。確認して再度削除してください)"; /* Settings Window */ "Lexime 設定" = "Lexime 設定"; diff --git a/SPEC.md b/SPEC.md index 09b315f..9e7f29b 100644 --- a/SPEC.md +++ b/SPEC.md @@ -457,7 +457,7 @@ decay = 1.0 / (1.0 + hours_elapsed / 168.0) 学習されていない候補の削除(no-op)には Tombstone を書かず `F_FULLFSYNC` を払わない。**ただし「メモリに無い」は「ディスクに無い」を意味しない** — 退避(§退避)で落ちたエントリや、耐久化が確立しなかった削除の対象は、メモリから消えていても直近の checkpoint には残っている。これらのキーは `DurableResidue` として記録され、no-op 判定から除外して必ず Tombstone を書く(そうしないと削除が黙って無効になり、次回起動で復活する)。キー単位で追跡するため、退避が起きても**それ以外の未学習候補の削除は fast path のまま**。この状態は次の compaction 成功で解ける(追跡キーが上限に達した場合のみ、キー単位の追跡を諦めて「常に Tombstone を書く」保守動作に退避する。上限は 1 compaction 区間では到達しないよう設定してあるので、これは checkpoint が失敗し続けている状態の症状)。 -Tombstone の WAL 耐久化が失敗した場合(`Io` / `SyncFailed`)は、フォールバックとして checkpoint を**同期的に**書き出して削除を永続化する(`durability_failed` → `run_gated_compact`、design §5.4)。**その checkpoint も失敗した場合**(削除が WAL にも checkpoint にも届かない二重障害)は握り潰さず、`durability_issues()` の `DeletionNotPersisted` として報告する(§保存の「実行中の耐久性報告」)。 +Tombstone の WAL 耐久化が失敗した場合(`Io` / `SyncFailed`)は、フォールバックとして checkpoint を**同期的に**書き出して削除を永続化する(`durability_failed` → `run_gated_compact`、design §5.4)。**その checkpoint も失敗した場合**(削除が WAL にも checkpoint にも届かない二重障害)は握り潰さず、`durability_issues()` の `DeletionNotPersisted` として報告する(§保存の「実行中の耐久性報告」)。この報告はプロセスローカルなので、同じ二重障害を sidecar marker にも記録して次回起動に引き継ぐ(§保存の「未永続の削除の引き継ぎ」)。 ### 保存(WAL + Checkpoint、LXUD v2) @@ -465,9 +465,28 @@ Tombstone の WAL 耐久化が失敗した場合(`Io` / `SyncFailed`)は、 - **WAL**: `user_history.lxud.wal`(8 バイトファイルヘッダ `LXWL` + version 2。フレーム形式: payload_len + CRC32(seq+payload) + seq + bincode(`WalRecord::Committed|Tombstone`)) - **seq**: WAL フレームは単調増加の連番を持ち、checkpoint ヘッダの `applied_seq` が「効果を含む最後の seq」を記録。replay は `seq > applied_seq` のフレームのみ適用するため、checkpoint 書き込みと WAL truncate の間でクラッシュしても二重適用が構造的に起きない - **書き込み**: 確定時に WAL append(Committed は 50 frame ごとに write barrier = `fcntl(F_BARRIERFSYNC)`)、閾値到達で background compaction(checkpoint を tmp + `sync_all` + rename + 親 dir fsync(best-effort・log-only)で書き出し + 条件付き WAL truncate)。compaction の排他は `compact_gate` (Mutex)、削除・障害後の即時要求は `scrub_pending` で直列化。削除は `Tombstone` frame(削除の WAL 表現、書き込み時に毎回 `F_FULLFSYNC`)を append し、直後に非同期スクラブ compaction をスケジュールして物理消去する。全消去(`clear`)は空 checkpoint(`applied_seq` = 現 WAL 最大 seq)を先行書き込みしてコミットポイントとし、以後どのクラッシュ点でも空履歴に収束する(旧 WAL frame は全て skip される) -- **場所**: `~/Library/Application Support/Lexime/user_history.lxud` +- **場所**: `~/Library/Application Support/Lexime/user_history.lxud`(family: `.wal` / `.tmp` / `.v1.bak` / `.corrupt-` / `.deletion-pending`) - **起動時(エンジン経路)**: `recovery::open_recovering` — checkpoint ロード → WAL replay(evict なし + 事後 1 回)→ in-memory 復元。破損は `.corrupt-` へ隔離(直近 3 個保持)して空で継続、WAL 末尾破損は last-good オフセットで物理修復。どのファイル状態でも起動は成功し学習は継続する(`OpenReport` に結果を記録。Err は EACCES 等の環境障害のみ)。`OpenReport` は v1→v2 migration の commit 失敗(`migration_failed`。v1 ファイルは温存する。再試行のタイミングは経路による — legacy WAL を消費していた場合は WAL が frozen になるため `appends_frozen` 由来の起動時 compaction が副作用として v2 checkpoint を書き変換を完了させ、そうでなければ次回起動が再試行する。**compaction は migration ではない**(`.v1.bak` 退避も `Migrated` 状態設定も行わない)ため、失敗した migration の再試行に compaction を使うことはしない — commit が失敗している経路でそれを走らせると v1 ファイルを潰す。`.v1.bak` は design 決定 #13 どおり best-effort のままで、正しさの前提条件ではない)と append 凍結(`appends_frozen`。このセッションの学習は compaction が heal するまでメモリのみ)も持つ。`migration_failed` は `checkpoint_state` / `wal_state` が健全値のまま真になりうるので独立フィールドが要る。`appends_frozen` は逆に `RepairFailed` / `Quarantined` と同時に立つ場合もあり(凍結の 5 経路のうち健全値のままなのは legacy WAL つき migration 失敗のみ)、どちらの向きにも畳めない — どちらも `checkpoint_state` / `wal_state` は健全な値のままなので、それらだけでは正常起動と区別できない -- **実行中の耐久性報告**: `durability_issues()` が「いま成立している」耐久性の問題を severity 順のリストで返す(`OpenReport` の実行中版)。①`DeletionNotPersisted` — 削除の WAL 耐久化(append の `Io` / `SyncFailed`)と §5.4 の同期 checkpoint fallback が両方失敗した状態。**半分ずつ意味が違う**: frame が WAL に届かなかった (`Io`) 側は削除がメモリのみで起動時 heal が無い(旧 checkpoint が勝つ)が、frame は届いて flush だけ失敗した (`SyncFailed`) 側は replay が削除を再適用するので、失うのは電源断のみ。ユーザー向け文言が再起動に言及しないのはこのため(後者では再起動が救済側)。世代カウンタを 1 つの atomic に 3 本(`raised_memory_only` / `raised_deletion` / `covered`、各 21bit)詰めて管理し(1 load で両方の事実が同時点の値になる — 2 つの atomic に分けると間に raise が入ったとき同時には成立していないペアを返し、重い方の行を落とす)、raise は wal ロック下・メモリ適用の後、被覆は「durable set がその削除を含まなくなる」2 点= compaction の `save` 成功と `clear` の空 checkpoint 成功。compaction は snapshot **前**に世代を読むため、snapshot 後に立った raise を誤って被覆しない。②`LearningMemoryOnly` — WAL frame が付かないまま適用された確定があり、それをどの durable checkpoint も含んでいない状態。**当初は `is_frozen()` から導出していたが、根拠だった「確定が memory-only」⟺「WAL frozen」は偽**: frozen guard で弾かれた確定は seq 採番に到達せず `last_appended_seq` が動かないため、その確定より前の snapshot を持つ進行中 compaction が `truncate_covered` を通って freeze を解除しうる — 確定は checkpoint にも WAL にも無いまま報告は clean になる。よって導出をやめ、①と同じ台帳に `raised_memory_only` として載せた(freeze は「ファイルが追記可能か」という本来の意味に戻り、`OpenReport` だけが読む)。壊れたディスクでは両方立つのが定常なので単一 enum には畳まない。読み取りは atomics のみで wal ロックを取らない(キー処理スレッドが append 中ずっと保持しているため)。Swift は `EngineControlService` 経由でポールし、`DegradedStatus` がステータスメニューの行に落とす(`menu()` は開くたびに再導出するので latch しない。ただし**回復の検知は受動的**で、行が消えるのはディスク回復後の次の確定操作が compaction を走らせた時点。`Io` 側は frozen が全 append を失敗させるので次の確定で再試行されるが、`SyncFailed` 側は frozen にならないため閾値 (1000 frame / 1 MiB) まで待つ。定期 compaction も終了時 flush も無いので、入力を止めたユーザーには行が残り続ける。逆に、append は失敗するが checkpoint は書ける状態(`clear` の truncate 失敗など)では、確定ごとに raise → その heal compaction が被覆、となるため行は確定の合間に消える — その瞬間は実際に durable checkpoint がメモリを覆っているので正しい。latch する `EngineInitFailure` とは寿命が違うため統合しない。runtime 行は init failure の有無に関わらず出す(`menu()` は「行があるか」で判定し、エンジンが degraded かでは判定しない)— 起動 clean・実行中に故障が主シナリオだから)。**このリストは意図的に 2 項目で、耐久性の問題を網羅しない**: 削除自体は durable だが物理スクラブが遅延しているだけの状態(scrub compaction の `save` 失敗 #311、`spawn_compact` の恒久失敗)は報告しない。またプロセスローカルなので、`Io` 側の削除が実際に復活する再起動時には報告が消えている(#312 — #295 の残存ギャップ) +- **実行中の耐久性報告**: `durability_issues()` が「いま成立している」耐久性の問題を severity 順のリストで返す(`OpenReport` の実行中版)。①`DeletionNotPersisted` — 削除の WAL 耐久化(append の `Io` / `SyncFailed`)と §5.4 の同期 checkpoint fallback が両方失敗した状態。**半分ずつ意味が違う**: frame が WAL に届かなかった (`Io`) 側は削除がメモリのみで起動時 heal が無い(旧 checkpoint が勝つ)が、frame は届いて flush だけ失敗した (`SyncFailed`) 側は replay が削除を再適用するので、失うのは電源断のみ。ユーザー向け文言が再起動に言及しないのはこのため(後者では再起動が救済側)。世代カウンタを 1 つの atomic に 3 本(`raised_memory_only` / `raised_deletion` / `covered`、各 21bit)詰めて管理し(1 load で両方の事実が同時点の値になる — 2 つの atomic に分けると間に raise が入ったとき同時には成立していないペアを返し、重い方の行を落とす)、raise は wal ロック下・メモリ適用の後、被覆は「durable set がその削除を含まなくなる」2 点= compaction の `save` 成功と `clear` の空 checkpoint 成功。compaction は snapshot **前**に世代を読むため、snapshot 後に立った raise を誤って被覆しない。②`LearningMemoryOnly` — WAL frame が付かないまま適用された確定があり、それをどの durable checkpoint も含んでいない状態。**当初は `is_frozen()` から導出していたが、根拠だった「確定が memory-only」⟺「WAL frozen」は偽**: frozen guard で弾かれた確定は seq 採番に到達せず `last_appended_seq` が動かないため、その確定より前の snapshot を持つ進行中 compaction が `truncate_covered` を通って freeze を解除しうる — 確定は checkpoint にも WAL にも無いまま報告は clean になる。よって導出をやめ、①と同じ台帳に `raised_memory_only` として載せた(freeze は「ファイルが追記可能か」という本来の意味に戻り、`OpenReport` だけが読む)。壊れたディスクでは両方立つのが定常なので単一 enum には畳まない。読み取りは atomics のみで wal ロックを取らない(キー処理スレッドが append 中ずっと保持しているため)。Swift は `EngineControlService` 経由でポールし、`DegradedStatus` がステータスメニューの行に落とす(`menu()` は開くたびに再導出するので latch しない。ただし**回復の検知は受動的**で、行が消えるのはディスク回復後の次の確定操作が compaction を走らせた時点。`Io` 側は frozen が全 append を失敗させるので次の確定で再試行されるが、`SyncFailed` 側は frozen にならないため閾値 (1000 frame / 1 MiB) まで待つ。定期 compaction も終了時 flush も無いので、入力を止めたユーザーには行が残り続ける。逆に、append は失敗するが checkpoint は書ける状態(`clear` の truncate 失敗など)では、確定ごとに raise → その heal compaction が被覆、となるため行は確定の合間に消える — その瞬間は実際に durable checkpoint がメモリを覆っているので正しい。latch する `EngineInitFailure` とは寿命が違うため統合しない。runtime 行は init failure の有無に関わらず出す(`menu()` は「行があるか」で判定し、エンジンが degraded かでは判定しない)— 起動 clean・実行中に故障が主シナリオだから)。**このリストは意図的に 2 項目で、耐久性の問題を網羅しない**: 削除自体は durable だが物理スクラブが遅延しているだけの状態(scrub compaction の `save` 失敗 #311、`spawn_compact` の恒久失敗)は報告しない。また**このリストはプロセスローカル**で、`Io` 側の削除が実際に復活する再起動時には残らない — その一線だけは sidecar marker が引き継ぐ(下記「未永続の削除の引き継ぎ」)。**①には第 3 の発生源がある**: 前セッションの `Unflushed` marker を replay が適用し、かつ durable checkpoint がまだ覆っていない起動では、起動時に台帳へ直接 seed される(wal ロック外・起動時 1 回)。replay は page cache から読めたことしか証明しないので、durable checkpoint が覆うまでは実際に「いま成立している」耐久性の問題であり、`replayed_deletion` 由来の起動時 compaction が撤回する +- **未永続の削除の引き継ぎ**(#312): 二重障害で失われた削除を sidecar `user_history.lxud.deletion-pending`(16 バイト固定: magic `LXDM` + version + flags + witness_seq)に記録し、次回起動の `OpenReport.deletion_lost` として報告する。checkpoint ヘッダの reserved バイトは使わない — raise 条件そのものが「checkpoint 書き込みの失敗」なので必要な瞬間に書けないチャネルに乗ることになり、in-place のヘッダ書き換えは CRC(先頭 28 バイトを保護)の再計算を tmp+rename の外で行うことになる。 + - **記録内容は 2 種**: frame が WAL に届かなかった `Io` 側は `Lost`(無条件に報告)、frame は届いた `SyncFailed` 側は tombstone の seq を witness に持つ。 + - **書き込みは read-modify-write で merge**(`Io` が吸収、witness は max)。`SyncFailed` は WAL を凍結しないので、凍結を解いた compaction のあとに `Lost` の上へ `Unflushed` が来る経路が実在し、全置換だと抑止可能な witness に格下げされる。書き込みに失敗した主張はセッション内にも保持し、次の raise が再表明する(memory-only な raise でも — 射影されるのは claim であって、その raise が持ってきた breach ではない)。 + - **`NotFound` だけが clean**。デコードは**フィールド検査ではなく writer との往復**で行い、`encode` が生成しうるバイト列だけを受理する(長さ・magic・version・flags・reserved・witness の 0/非 0 整合がすべて一度に閉じる — フィールド検査は 4 回続けて取りこぼした)。読み取り失敗も同様に報告側。**regular file 以外は開かない** — marker の位置に FIFO があると read-only の `open` が writer を待って無限ブロックし、起動時同期経路なので IME が起動不能になる (検査の仕方は下記の書き込み側と同一)(破損は報告方向にしか倒れないので CRC 不要。読み取り失敗を `Err` にすると sidecar 1 個で履歴 open 全体が落ち学習が止まる)。**書き込みは共有の `write_atomic`(tmp → fsync → rename → 親 dir fsync)**。`rename` は宛先の symlink / FIFO を**追従せず置換**するので、「リンク先を上書きする」「unlink 失敗を見ずに create する」が構造的に消える。親 dir fsync は checkpoint / user dict では **best-effort・log-only** (§6) だが、**marker ではそうしない** — その根拠 (「最悪でも 1 つ前のファイルに巻き戻るだけ」) が marker にだけ成立しないため。巻き戻り先が「1 つ前の完全な状態」である store では recency を失うだけで次の書き込みが取り戻すが、marker の 1 つ前は**より弱い claim** (`Lost` に昇格した直後なら抑制可能な `Unflushed{seq}`、初回書き込みならファイル自体が無い) であり、これは形式が唯一倒れてはいけない向きである。よって `write_atomic_staged` は成功側も **`Durable` / `NameUnconfirmed` の 2 値で返し**、marker は後者を `MarkerState::Unknown` として記録する — どの desired state も満たさないので毎コミットの照合が再アサートし続け、`flushed != Holds(Lost)` の freeze 述語により **`Lost` が durable に名付けられるまで append は凍結されたまま**になる。`AtomicWriteFailure` の doc が既に述べていた「どの段階まで行ったかは caller には推測できないので writer が言う」という原則を、成功パスにも適用しただけで新しい機構ではない。なお**除去**側は best-effort のままでよい: unlink の巻き戻りは古い marker を復活させる = 過剰報告であり、報告方向にしか倒れない (空の履歴に対する `Lost` は「両方空」規則が反証する)。**tmp 側も同じ保護が要る**: rename が守るのは宛先だけで、tmp は名前で open されるため素の `File::create` は symlink を追ってリンク先を truncate し、reader のいない FIFO では無限に待つ(marker の writer はキー処理スレッド上)。`O_NOFOLLOW | O_NONBLOCK` で open し、得た fd に対して `fstat` で regular file を確認する — 事前の `symlink_metadata` 検査では検査と open が同じ名前を 2 回引くので、その間に差し替えられると元の罠がそのまま残る。**読み取り側も同じ 1 回解決**にする (marker の位置に FIFO を置かれる経路は起動時同期)。なお `rename` が昇格させるのは fd ではなく**名前**なので、open と rename の間に tmp を差し替えられる窓は閉じない — POSIX に fd ベースの rename が無く、tmp を一意名にすると「論理的な marker は canonical + orphan tmp の対」が壊れ、rename 直前の stat は窓を狭めるだけで閉じたように読める。そもそもこのディレクトリに書ける主体は宛先を直接上書きできるので、窓を突く必要が無い。塞がずに明記する。一時期これを手書きの in-place 書き込みに置き換えたが、その唯一の理由(tmp+rename のクラッシュ窓が、より強い claim を読まれない sibling に残す)は **`read` が marker と orphan tmp を merge する**ことで根から閉じた — orphan は claim を強めることしかできない。**論理的な marker は `.deletion-pending` と `.deletion-pending.tmp` の対**である: `read` が orphan を merge する以上、除去も両方を対象にしなければ claim が生き残る(`Lost` の orphan が毎起動で報告を復活させ、ack でも消せない — 隠蔽を直した結果として latch を作り直すことになる)。どちらのパスも、**空の**ディレクトリなら placeholder として除去するが、**中身があるものは消さない**(外部の restore が置いたものであり、main thread で無制限のツリーを歩くことにもなる)。除去は成否を返し、消せなかった場合はレポートを負ったままにする — 「消せない行が残り続ける」問題は、黙って消したことにするのではなく**消せなかったと言う**ことで解く。 + - **`Lost` の主張は、durable set と読み込んだ状態の両方が空なら空虚**として撤回する(`clear` が空 checkpoint から台帳を被覆するのと同じ根拠 — 生き残ったと言っているエントリが 1 つも存在しない)。これが無いと、marker を unlink できなかった全消去が、次回起動で空の履歴に対して警告を出す。**両方を要求する**のは、片方だけの規則がそれぞれ一度ずつ誤ったから: durable set だけだと、隔離された空の checkpoint に WAL replay がエントリを戻す経路で黙る。読み込んだ状態だけだと、replay された tombstone がメモリを空にする一方で checkpoint にはエントリが残り、電源断でそれが戻る — しかも**デコードは malformed も `Lost` に倒す**ので、その主張が本当は耐久性についての witness だった可能性を排除できない。両方を要求すると「ディスクにも無く replay でも戻らない」という主張が実際に必要とする条件そのものになり、**明示的に書かれた `Lost` と malformed からのフォールバックを区別する必要も消える**(checkpoint が何かを保持している限りどちらも反証不能)。`Lost` に限る点は変わらない: `Unflushed` は存在ではなく耐久性の主張なので、空であることは何も settle しない。 + - **それ以外の起動時撤回は durable checkpoint に対してのみ**。判定は「`open_recovering` が返る時点でディスク上にある checkpoint」= migration 経路ではこの関数自身が書いたもの、に対して 1 回だけ行う。witness が *replay* で満たされた場合はこれに当たらない — page cache から読めたことしか証明していない(失敗したのは flush)ので、実行中の台帳に「未被覆の削除」として引き継ぐ(したがって migration が削除を含む checkpoint を書いた起動では、この引き継ぎ経路には落ちず撤回される)。どちらでも満たされなければ報告し、**その場で `Lost` に昇格**させる。**これは最適化ではなく必須**である: seq は 1 つの WAL ファイル内の*位置*であって epoch ではなく、witness の判定 `seq > applied_seq` は**高水位標に対する不等式**なので、ファイルが差し替わったあとは witness より上の任意の frame がそれを「適用済み」と答えてしまう — その tombstone が新しい系統に存在したかどうかと無関係に。一時期ここに**採番の下限** (`set_seq_floor`) を入れたが撤去した: 不等式に対して「クレームが名指す 1 つの番号を飛ばす」ことは何も変えない (gap は合法)。「これで世代整合を epoch で守る規律になる」と書いたのは誤りで、*位置*に下限を置いても同一性は生まれない。**報告が負われている間、ディスクがまだ `Lost` を言っていなければ append を凍結する** — witness に答えてしまう状態を、系統非依存の形が着地するまで進めない。 + - **撤回は 5 経路で、それぞれ根拠が違う**: ①起動時、witness が**その時点で durable な checkpoint**(`durable_applied_seq`)に覆われていた場合 — save 成功と unlink の間のクラッシュの残骸か、migration commit 自身が覆う checkpoint を書いた場合。①' 起動時、上記の「両方空」で `Lost` が空虚と判定された場合(根拠が最も違うのはこれ — 覆っている checkpoint は無く、*主張の対象が存在しない*)②compaction の `save` 成功(ledger の被覆と同一の wal guard 下 — CAS 成功と unlink の間に新しい raise が入る窓は決定的テストで守れないため、guard witness を型で要求して表現不能にした)③`clear` の無条件削除(前セッション由来の marker は当セッションの台帳を動かさない)④`ack_open_report()`。**②は継承クレームには効かない** — 当セッションが書く checkpoint は*復活したエントリ*を永続化するので、前セッションの未配信レポートについては何も settle しない。よって未 ack の継承レポートがある間、②は unlink しない。 + - **起動時の撤回が unlink に失敗したら `compaction_recommended` に載せる**。判定自体は恒久的(主張の対象は消えており、以後の学習はそれではない)だが、記録できる場所が今まさに消せなかったそのファイルしかないので、負債を runtime に渡す。runtime 側の射影は stale な marker を retry するが、それが走るのは compaction のときだけで既定では 1000 frame 先 — その前に再起動すると、replay で非空になった履歴に対して marker を読み直し、この起動が撤回済みと判定した損失を報告してしまう。**`deletion_lost` も同じ経路に載せる**(ディスクがまだ `Lost` を言っていない場合に限る)— 「compaction が復活したエントリを祝福してしまうから載せない」という旧規則は誤りだった: `inherited_owed` はセッション台帳の外にあるので cover は報告を settle できず、compaction がするのは無条件 marker の再射影だけである。昇格が失敗したときそれを促すものが他に無いため、これは必須。 + + ただし **marker 由来かどうかに関わらず、v1 checkpoint が commit の再試行を待って残っている間は `compaction_recommended` 自体を veto する**。compaction は migration ではない(`.v1.bak` も `Migrated` も踏まずに v1 の上へ v2 を書く)ので、v1 のバイトが唯一の複製である経路でそれを走らせると破壊する。これは *compaction を走らせること自体*の性質なので、**拒否は `run_compact_impl`(書き込みを行う唯一の場所)に置く**。スケジューラ側で gate するのは原理的に取りこぼす: 現状 5 つ(起動時ヒント / scrub 分岐 / 閾値 / `clear` の heal / `FollowUp`)あり 6 つ目を止めるものが無く、実際 1 つずつ塞いで 3 ラウンド費やしたうえ scrub 分岐が残っていた。起動時ヒントとスケジューラ 2 箇所(`spawn_compact` / `spawn_threshold_compact`)も落とすが、それは拒否する worker を spawn しないための最適化にすぎない(veto 中は閾値が永久に解消しないので、放置すると劣化セッションが確定ごとにキー処理スレッドから OS スレッドを作り続ける)。**veto は起動時の観測ではなく「いまそのパスに v1 のバイトがあるか」なので、`clear` の空 checkpoint 書き込み成功(コミットポイント)で解除する** — 全消去は未 migration の複製ごと履歴を supersede するのが意図であり、起動時 report のまま握り続けると `clear` 自身が投げる heal(物理ステップ失敗時)まで拒否して、凍結した WAL と中途半端な wipe が再起動まで直らない(設定画面からの全消去は再起動しない)。legacy WAL を消費した場合だけは例外で、そこでは compaction こそが意図された heal(WAL が凍結され、それを解除する compaction が副作用で v2 checkpoint を書いて変換を完了させる)。 + - **射影の照合は「コミットごと」が正準で、それ以外はすべて順序か promptness**。ディスクが射影と一致することを保証するのは `apply_records` が wal ロック下・append の**前**に行う照合であって、個別のイベントに足した retry ではない。*一部のイベントでしか照合しない記録は射影ではなく無効化つきキャッシュ*であり、実際レビューは 6 ラウンドかけて「そのイベントでは retry されない」を 1 つずつ (ack / 全消去 / compaction の cover / 起動時の削除 / 起動時の promotion / その compaction 自身) 出し続けた。append の**前**なのは、閉じたい害が「未昇格の witness を追い越して WAL が進む」ことだから。健全時は `flushed` との比較 1 回で syscall ゼロなので、キー処理経路に置ける — これが成立するのは次項で `flushed` を観測から始めているため。起動時 compaction のスケジュールと全消去の heal は promptness であって正しさの担い手ではない。 + - **ディスク上のより強い記録は、より弱い desired を満たす**(等値ではなく merge 束で判定)。`merge_write` は要求された `Unflushed` を生き残った `Lost` に吸収するので、等値判定だと desired に到達できず、毎コミットがキー処理スレッドで full sync を払い続ける livelock になっていた。逆向き — witness が無条件クレームを覆う — は成立しない。 + - **書き込みが「実際に何を残したか」を返し、信念にはそれを記録する**。`merge_write` は merge するので、生き残った `Lost` の上に `Unflushed` を要求しても残るのは `Lost` — 要求の方を記録すると、ディスクが一度も持たなかった信念を抱え、次の照合が「既に一致」と見て skip する。何が landed したかを知っているのは writer だけなので、writer が返す。 + - **「ディスクに何があるか」は 3 値**(`MarkerState`: 不在 / このバイト列 / **不明**)。ファイルはあるが誰も読めていない状態は前 2 者のどちらでもなく、これを「不在」に潰すと、読めない marker の unlink 失敗が「解決済み」に見えてしまう(射影が一致と判断して skip し、生き残ったファイルが次回起動で誤報を出す)。`Unknown` はどの desired とも一致しないので retry を skip できない。 + - **報告が負われていて、ディスクがまだ `Lost` を言っていない間、append を凍結する** — sidecar が学習を止めてよい唯一のケース。読めない marker であれ、昇格に失敗して残ったデコード済み `Unflushed{seq}` であれ、いずれも witness に答えてしまう状態を進めてはならない (上記のとおり floor では守れない — 撤去済み)。凍結が解けるのは昇格が着地したときだけで、`frozen` は元々「このファイルに追記するのは安全でない」という意味なので新機構ではない。**ただし「着地」は canonical file だけを見て判定しない**: `write_atomic` は rename の前に tmp を flush するので、rename だけが失敗したケースでは orphan tmp が正確なバイト列を耐久化しており、論理的な marker (canonical + orphan の対) は既に主張を持っている。ここを失敗扱いにすると、毎コミット凍結 → compaction が解除 → 次のキー入力でまた凍結、が永久に続く。どの段で失敗したかは **writer が返す** (`write_atomic_staged` の `NotDurable` / `FlushedNotRenamed`) のであって、バイト列を読み戻して比較するのではない — `write_all` が完了したあとに `sync_all` が失敗した場合、page cache 上のイメージは一致するのに耐久化されておらず、それを「着地した」と扱うのがまさにこの sidecar が塞ぐべき電源断の穴になる。表現可能な上限の witness については、`+1` が debug で UniFFI コンストラクタ越しに panic し release で seq 0 を配るので、**採番する場所で拒否する** — 凍結では*種類が違う*。`frozen` は「このファイルは追記可能な v2 形ではない」であって compaction がファイルを書き直せば正当に解除されるが、ファイルを書き直しても番号は増えないので、heal が wrap をそのまま戻してしまう。採用時は saturating にし、`u64::MAX` は「使う」のではなく「拒否する」(それを割り当てると後continuation が表現できないため。2^64 のうち 1 つを捨てる方が、もう 1 つ状態を持つより安い)。 + - **runtime は「ディスクに何があるか」を推定せず観測から始める**。ただし *claim* と *observation* は別物で、混同すると逆向きに壊れる: 読めない marker は fail-safe 規則で `Lost` を*主張*するが、それは「バイトを読んで確かめた」ではない。`read` は両方を返し (`MarkerObservation { breach, confirmed }`)、`marker_on_disk` に載るのは `confirmed` な観測だけ — でないと runtime が `flushed = Some(Lost)` を信じ、以後の照合がすべて skip し、生きた `Unflushed` witness を昇格させる者がいなくなる。また `deletion_pending_checkpoint` は台帳だけでなく `session` クレームにも seed する: 台帳と claim が「何が未解決か」で食い違うと、照合が desired=None を計算してまだ誰も永続化していない witness を unlink してしまう。`MarkerClaims::flushed` は `OpenReport::marker_on_disk` で初期化する — `None` は「ディスクは空」という積極的な主張であり、撤回の unlink に失敗した起動や promotion の書き込みに失敗した起動はそれを裏切るバイトを残す。特に promotion 失敗は、メモリが `Lost` を持つ一方でディスクに*抑止可能な* `Unflushed{seq}` が残るので、後続の checkpoint がその witness を満たして「まだ負っている報告」を黙って撤回しうる。観測から始めることで、射影は「ディスクが既に一致している」ときだけ skip でき(健全時は毎回 skip = syscall ゼロ)、一致していないときは再表明が自然に retry になる。なお `flushed` は主張ではなく観測なので、全消去は `session` だけを畳んで `flushed` は畳まない — 消去はファイルを消滅させないため。 + - **消費は行が描画された時点ではなくクリック時点**。`menu()` は行を組み立てるだけで ack しない。`bootstrap()` は IMKit の probe 起動でも走りメニューを出さないので起動時 ack は論外だが、**メニューの構築自体も配信の証拠にならない** — IMKit は表示せずに `menu()` を呼ぶ(実測: 誰も触っていない再起動の 4 秒後に記録が消費された)。人がその行をクリックしたことだけが配信の証拠になる。ack は台帳を見て(当セッションの raise があれば消さない)、wal mutex を `try_lock` で取る(main thread を塞がない。取れなければ次回のクリックに持ち越す=安全側)。**行を出し続けるかは ack の成否ではなく「エンジンがまだレポートを負っているか」という単一述語**が決める — ack が完了できなかった場合も、commit point 前に失敗した全消去も、どちらも「負ったまま」であり、同じ問いが両方に答える。 + - Swift は latch する `EngineInitFailure.historyDeletionLost` に落とす — **ディスクの回復では撤回されない**過去の事実なので runtime 行ではない(runtime 行はディスク自身が撤回する)。行を退役させるのは 2 つのユーザー操作だけで、どちらもディスクの回復ではない: **確認**(クリック → `ack_open_report()`。主張は真のまま「配信済み」になる)と**全消去**(主張が*偽*になる — marker は消えており、行はもう存在しない項目の再削除を促してしまう)。両方とも `EngineControlService.retractRowIfSettled()` を通り、行を出し続けるかは「エンジンがまだ負っているか」という単一述語が決める。 + - **閉じない障害クラス**: marker は checkpoint と同じディレクトリに書くため、ディレクトリ全体が書けない障害(読み取り専用ボリューム / EACCES / 親削除)では marker も書けず報告不能。ENOSPC・特定ブロックの EIO のように checkpoint 固有の失敗が対象 - **オフラインツール経路**: `UserHistory::open` / `open_with_wal` は無副作用・厳格エラーのまま(監査ツールが稼働中 IME のファイルを rename しない) ### 退避 diff --git a/Sources/Controller/DegradedStatus.swift b/Sources/Controller/DegradedStatus.swift index ee79f02..8e449e1 100644 --- a/Sources/Controller/DegradedStatus.swift +++ b/Sources/Controller/DegradedStatus.swift @@ -6,12 +6,29 @@ import Foundation /// the point: /// /// - `EngineInitFailure` latches. It records what went wrong while the engine -/// was starting, and nothing during the session retracts it. +/// was starting, and nothing the *disk* does during the session retracts it. +/// One row has an exception, and it is a user action rather than a recovery: +/// `.historyDeletionLost` is retired by acknowledging it or by a wipe (see +/// below). /// - `LexHistoryDurabilityIssue` is polled and clearable. A frozen WAL thaws /// when a compaction restores appendable form; an unpersisted deletion is /// covered by the next durable checkpoint. Folding these into `initFailures` /// would make a recovered disk keep warning forever. /// +/// The dividing line is retraction, not where the fact came from: +/// `.historyDeletionLost` is a durability failure too, but it is a *past* one, +/// so no amount of the disk recovering retracts it and it latches like the rest +/// of startup. Two things retire it, and neither is the disk healing: +/// +/// - **a full wipe**, which makes the claim false rather than stale — there is +/// no longer an entry the deletion could have failed against; +/// - **the user acknowledging it**, which leaves the claim true but delivered. +/// +/// Both reach `EngineControlService.retractRowIfSettled()`, which asks the +/// engine whether the report is still owed rather than inferring it from which +/// action ran — so a wipe that fails before its commit point, or an +/// acknowledgement whose unlink fails, keeps the row. +/// /// The other half of that separation is that a runtime issue must show even /// when startup was clean — the main #295 scenario is a healthy launch /// followed by a disk that fails hours later. So `menu()` gates on "are there @@ -19,13 +36,35 @@ import Foundation /// display nothing in exactly the case this exists for. enum DegradedStatus { - /// Titles for the disabled status rows, init failures first. - /// Empty when there is nothing to report. + /// One status row. + struct Row: Equatable { + let title: String + /// Whether clicking the row acknowledges a durable record behind it. + /// + /// True for exactly one row. Every other row is derived — it either + /// re-derives from live state on each menu open, or latches in memory + /// and dies with the process — so there is nothing for a click to + /// settle. The lost-deletion row is backed by a file that outlives the + /// process, and only a person can say they have seen it: IMKit calls + /// `menu()` on its own, without displaying anything, so "the menu was + /// built" is not evidence of delivery. Verified on-device — the record + /// was consumed four seconds after a relaunch nobody touched. + let acknowledgeable: Bool + } + + /// Status rows, init failures first. Empty when there is nothing to report. static func rows( initFailures: [EngineInitFailure], runtimeIssues: [LexHistoryDurabilityIssue] - ) -> [String] { - initFailures.map(title(for:)) + runtimeIssues.map(title(for:)) + ) -> [Row] { + initFailures.map { failure in + Row(title: title(for: failure), acknowledgeable: isAcknowledgeable(failure)) + } + runtimeIssues.map { Row(title: title(for: $0), acknowledgeable: false) } + } + + static func isAcknowledgeable(_ failure: EngineInitFailure) -> Bool { + if case .historyDeletionLost = failure { return true } + return false } static func title(for failure: EngineInitFailure) -> String { @@ -54,6 +93,16 @@ enum DegradedStatus { return NSLocalizedString( "⚠️ 学習履歴の一部を復旧できませんでした(学習は継続中)", comment: "Degraded status: user history partially lost, learning continues") + case .historyDeletionLost: + // Says 「前回」 and tells the user what to do, where the runtime row + // below says only that a save is failing right now. Without that + // split the two read as near-duplicates, and on a disk that is + // still failing they appear together — the steady state, not a + // corner. This is the one that has already happened: the entry is + // back, and only the user can finish the job. + return NSLocalizedString( + "⚠️ 前回のセッションの削除が保存されていません(削除した内容が復元されている可能性があります。確認して再度削除してください)", + comment: "Degraded status: a deletion from a previous session never reached disk") case .customSettings: return NSLocalizedString( "⚠️ 設定ファイルの読み込みに失敗(デフォルト設定で動作中)", diff --git a/Sources/EngineContainer.swift b/Sources/EngineContainer.swift index 7300da9..c6a236e 100644 --- a/Sources/EngineContainer.swift +++ b/Sources/EngineContainer.swift @@ -22,6 +22,49 @@ final class EngineContainer { initFailures.append(failure) } + /// The history failures a recovery report produces, as a pure function of + /// the two independent facts it can carry. + /// + /// Extracted so the one property that matters here is testable: a lost + /// deletion and a quarantine are independent facts about the same startup, + /// so the lost-deletion case must not live inside the mutually exclusive + /// branch chain that reports the quarantine — routing it through there + /// would let `dataLossSuspected` mask it. Asserting that over + /// `DegradedStatus.rows` cannot work: that function maps whatever list it + /// is handed, so it would only be testing `Array.map`. + static func historyFailures( + deletionLost: Bool, + dataLossSuspected: Bool, + detail: String, + deletionDetail: String + ) -> [EngineInitFailure] { + var failures: [EngineInitFailure] = [] + if deletionLost { + failures.append(.historyDeletionLost(detail: deletionDetail)) + } + if dataLossSuspected { + failures.append(.historyDataLoss(detail: detail)) + } + return failures + } + + /// Drop the lost-deletion row, whether because a wipe made the claim false + /// or because the user acknowledged it. The only latched row with a + /// retraction, and both of its retractions are user actions. + /// + /// Both callers reach it through `EngineControlService.retractRowIfSettled`, + /// which gates on the engine's own `deletionReportOwed()` rather than on + /// control flow — so this stays a pure render-cache edit with no policy in + /// it. (A `historyWasCleared()` wrapper used to sit in front of the wipe + /// path; it never acquired a caller once the service took over, and the + /// test that named it was pinning the wrapper instead of the gate.) + func retractDeletionLostRow() { + initFailures.removeAll { + if case .historyDeletionLost = $0 { return true } + return false + } + } + init( engine: LexEngine?, dictionary: LexDictionary?, @@ -118,6 +161,13 @@ final class EngineContainer { // return first. (migrationFailed cannot co-occur with // migratedFromV1, being its negation; it is appended uniformly // rather than special-cased.) + // Built lazily: interpolating a UniFFI enum goes through Swift's + // reflection runtime, and on a clean launch this would be the first + // such call in the process — paying its one-time warmup in front of + // the IME becoming responsive, for a string nothing then uses. + func stateDetail() -> String { + "checkpoint: \(report.checkpointState), wal: \(report.walState)" + } var degraded = "" if report.migrationFailed { degraded += " migration=failed(v1 kept)" @@ -125,24 +175,56 @@ final class EngineContainer { if report.appendsFrozen { degraded += " appends=frozen(memory-only until compaction)" } + // Appended outside the branch chain below, not inside it: the + // chain is mutually exclusive, and a lost deletion co-occurs + // freely with a quarantine (independent facts about the same + // startup). Routing it through the chain would hide it behind + // dataLossSuspected — which is also why it is a case of its own + // and not folded into .historyDataLoss: that one means "past + // learning was lost", this one means the opposite, data that + // survived a deletion the user asked for (#312). + var deletionDetail = "" + if report.deletionLost { + degraded += " deletion=lost(prior session, entry may be back)" + deletionDetail = stateDetail() + degraded + NSLog( + "Lexime: A deletion from a previous session was not persisted (%@)", + deletionDetail) + } + var quarantineDetail = "" if report.dataLossSuspected { - var detail = - "checkpoint: \(report.checkpointState), wal: \(report.walState)" + quarantineDetail = stateDetail() if !report.quarantinedPaths.isEmpty { - detail += ", quarantined: \(report.quarantinedPaths.joined(separator: ", "))" + quarantineDetail += + ", quarantined: \(report.quarantinedPaths.joined(separator: ", "))" } - detail += degraded - NSLog("Lexime: User history recovered with data loss (%@)", detail) - failures.append(.historyDataLoss(detail: detail)) + quarantineDetail += degraded + NSLog("Lexime: User history recovered with data loss (%@)", quarantineDetail) } else if report.migratedFromV1 { NSLog( "Lexime: User history migrated from v1 (\(report.framesReplayed) frames)\(degraded)" ) - } else if !report.clean { + } else if !report.clean && !report.deletionLost { + // deletionLost logged its own line above, carrying the same + // `degraded` fragment — without this clause a lost deletion on + // an otherwise healthy start says it twice. NSLog( "Lexime: User history recovery events: checkpoint=\(report.checkpointState) wal=\(report.walState)\(degraded)" ) } + failures.append( + contentsOf: EngineContainer.historyFailures( + deletionLost: report.deletionLost, + dataLossSuspected: report.dataLossSuspected, + detail: quarantineDetail, + deletionDetail: deletionDetail)) + // No ack here. `bootstrap()` runs on every process launch, + // including the short-lived IMKit probe launches this controller + // already designs around — none of which ever render a menu. Acking + // at load would consume the report on the user's behalf and put + // #295's gap back one layer up. Nor does `menu()` ack: IMKit + // builds the menu without displaying it, so construction is not + // delivery either. The row's click handler is what acknowledges. history = h } catch { NSLog("Lexime: Failed to open user history at %@: %@", historyPath, "\(error)") diff --git a/Sources/EngineInitFailure.swift b/Sources/EngineInitFailure.swift index 88d3cc3..fbe71ec 100644 --- a/Sources/EngineInitFailure.swift +++ b/Sources/EngineInitFailure.swift @@ -22,6 +22,20 @@ enum EngineInitFailure { /// User history recovery quarantined corrupt data: learning is running, /// but some past learning was lost (bytes preserved in `.corrupt-*`). case historyDataLoss(detail: String) + /// A deletion requested in an earlier session never reached disk, so the + /// history loaded at startup may still hold the entry it was meant to + /// remove (#312). The inverse of `.historyDataLoss`: here data survived + /// that should not have. + /// + /// An init failure rather than a `LexHistoryDurabilityIssue` on lifetime. + /// Runtime issues are polled because the *disk* retracts them — a frozen + /// WAL thaws, an unpersisted deletion is covered by the next checkpoint. + /// No amount of the disk recovering retracts this one: the deletion is + /// already lost, and only the user deleting again resolves it. Two user + /// actions do retire the row — acknowledging it, and wiping the history, + /// which makes the claim false rather than stale — and both go through + /// `EngineControlService.retractRowIfSettled()`. + case historyDeletionLost(detail: String) /// Custom settings.toml exists but failed to parse (defaults in effect). case customSettings(detail: String) } diff --git a/Sources/LeximeInputController.swift b/Sources/LeximeInputController.swift index 6bce55a..d4239c6 100644 --- a/Sources/LeximeInputController.swift +++ b/Sources/LeximeInputController.swift @@ -188,15 +188,23 @@ class LeximeInputController: IMKInputController { // Re-derived on every open, so a runtime issue that has since healed // stops being shown. See DegradedStatus for why the polled issues are // not merged into the container's latched initFailures. + let control = AppContext.shared.makeEngineControlService() let rows = DegradedStatus.rows( initFailures: AppContext.shared.engineContainer.initFailures, - runtimeIssues: AppContext.shared.makeEngineControlService() - .historyDurabilityIssues()) + runtimeIssues: control.historyDurabilityIssues()) if !rows.isEmpty { for row in rows { - // No action/target: IMKit renders these as disabled status rows. - let item = NSMenuItem(title: row, action: nil, keyEquivalent: "") - item.isEnabled = false + // Status rows are disabled — except the one whose durable + // record a click retires. Building the menu is not delivery: + // IMKit calls this method on its own, so acknowledging here + // would consume the #312 report on a launch that displayed + // nothing (measured: four seconds after an untouched relaunch). + let item = NSMenuItem( + title: row.title, + action: row.acknowledgeable ? #selector(acknowledgeDeletionReport) : nil, + keyEquivalent: "") + item.target = row.acknowledgeable ? self : nil + item.isEnabled = row.acknowledgeable menu.addItem(item) } menu.addItem(.separator()) @@ -212,6 +220,17 @@ class LeximeInputController: IMKInputController { return menu } + /// The user clicked the lost-deletion row, which is the only evidence + /// this process can have that the report reached a person. + /// + /// The row's fate is the engine's to decide — it declines to retire the + /// record while a compaction holds the wal mutex, or while this session + /// has an unpersisted deletion of its own, and in both cases the marker + /// stays and the row must too. + @objc private func acknowledgeDeletionReport() { + AppContext.shared.makeEngineControlService().acknowledgeHistoryReport() + } + @objc private func showSettings() { SettingsWindowController.shared.showWindow() } diff --git a/Sources/Services/EngineControlService.swift b/Sources/Services/EngineControlService.swift index d044f4b..e45488b 100644 --- a/Sources/Services/EngineControlService.swift +++ b/Sources/Services/EngineControlService.swift @@ -10,6 +10,24 @@ protocol EngineControlService { /// `LexSessionEvents` is the async candidate channel), and the sink is the /// status menu, which re-derives its rows on every open anyway. func historyDurabilityIssues() -> [LexHistoryDurabilityIssue] + + /// Retire the on-disk record behind a startup `deletionLost` report, now + /// that its row has been shown to a person. Called from the menu row's + /// action rather than from bootstrap or from building the menu: IMKit does + /// both on its own, without displaying anything, so neither is evidence + /// that the report reached anyone. + /// + /// Whether the row survives is `deletionReportOwed()`, not this call — + /// see there. Idempotent, and never blocks on the engine's locks. + func acknowledgeHistoryReport() + + /// Whether a lost-deletion report from a previous session is still owed. + /// + /// The single authority for whether the row belongs on screen. Both an + /// acknowledgement the engine could not complete and a wipe that failed + /// before its commit point leave it owed, so one question answers for both + /// call sites. + func deletionReportOwed() -> Bool } enum EngineControlServiceError: Error, LocalizedError { @@ -34,6 +52,12 @@ final class DefaultEngineControlService: EngineControlService { guard let engine = container.engine else { throw EngineControlServiceError.engineUnavailable } + // `defer`, because the wipe's commit point comes before the physical + // steps whose failures it throws: a throw does not tell us whether the + // report was retired. `retractRowIfSettled` asks the engine instead of + // inferring it from control flow — the same rule the acknowledgement + // uses, so the row has one authority rather than two. + defer { retractRowIfSettled() } try engine.clearHistory() } @@ -42,4 +66,42 @@ final class DefaultEngineControlService: EngineControlService { // container already latched as `.history`, not a durability problem. container.history?.durabilityIssues() ?? [] } + + func acknowledgeHistoryReport() { + // Off the main thread, because acking is what finally unlinks the + // marker: two `remove_file`s and possibly a `remove_dir`, on the one + // path that only ever runs when the volume is already misbehaving. + // `try_lock` inside `ack_open_report` keeps it off the wal mutex but + // says nothing about the syscalls themselves, and this is reached from + // a menu click on the main thread. + // + // Nothing is lost by deferring it: the row is re-derived every time + // `menu()` is built, so it disappears on the next open regardless of + // whether the ack finished before this call returned. Retracting back + // on main keeps `initFailures` single-threaded. + // Captured strongly, and that is load-bearing: the click handler calls + // this on a *temporary* — `makeEngineControlService().acknowledgeHistoryReport()` + // — so the service is released the moment this method returns. A weak + // capture would let the ack unlink the marker and clear the engine's + // owed predicate while the main-queue half saw `nil`, leaving the + // acknowledged row on screen for the rest of the process's life. The + // closure owning what it needs is the whole guarantee; there is no + // cycle, since nothing retains the closure past its run. + guard let history = container.history else { return } + DispatchQueue.global(qos: .utility).async { [self] in + history.ackOpenReport() + DispatchQueue.main.async { retractRowIfSettled() } + } + } + + func deletionReportOwed() -> Bool { + // No history means nothing was reported, so nothing is owed. + container.history?.deletionReportOwed() ?? false + } + + /// Drop the status row exactly when the engine no longer owes the report. + private func retractRowIfSettled() { + guard !deletionReportOwed() else { return } + container.retractDeletionLostRow() + } } diff --git a/Tests/TestDegradedStatus.swift b/Tests/TestDegradedStatus.swift index 9dd3f0a..68863ed 100644 --- a/Tests/TestDegradedStatus.swift +++ b/Tests/TestDegradedStatus.swift @@ -9,8 +9,11 @@ func testDegradedStatus() { runtimeIssues: [.deletionNotPersisted]) assertEqual(runtimeOnly.count, 1, "a runtime issue shows without any init failure") assertTrue( - runtimeOnly.first?.contains("削除") ?? false, + runtimeOnly.first?.title.contains("削除") ?? false, "the row must name the deletion, not just 'degraded'") + assertTrue( + !(runtimeOnly.first?.acknowledgeable ?? true), + "a polled row has no durable record to acknowledge") // S2. Both sources, both rendered, init failures first. let both = DegradedStatus.rows( @@ -20,7 +23,7 @@ func testDegradedStatus() { // trap on subscript and take the whole runner down with it, hiding every // later test behind a crash instead of a named failure. assertEqual( - both, + both.map { $0.title }, [ DegradedStatus.title(for: EngineInitFailure.historyDataLoss(detail: "x")), DegradedStatus.title(for: LexHistoryDurabilityIssue.deletionNotPersisted), @@ -39,6 +42,90 @@ func testDegradedStatus() { DegradedStatus.title(for: LexHistoryDurabilityIssue.deletionNotPersisted) != DegradedStatus.title(for: LexHistoryDurabilityIssue.learningMemoryOnly), "the two runtime issues need distinguishable rows") + + // S3 (#312). A disk that is still failing shows the past loss and the live + // one together — the steady state, not a corner. They must not read as the + // same sentence twice: one says a save is failing now, the other that one + // already failed and the entry may be back. + let acrossRestart = DegradedStatus.rows( + initFailures: [.historyDeletionLost(detail: "x")], + runtimeIssues: [.deletionNotPersisted]) + assertEqual(acrossRestart.count, 2, "a past loss and a live one are two rows") + assertTrue( + DegradedStatus.title(for: EngineInitFailure.historyDeletionLost(detail: "x")) + != DegradedStatus.title(for: LexHistoryDurabilityIssue.deletionNotPersisted), + "the latching row must not duplicate the polled one") + assertTrue( + DegradedStatus.title(for: EngineInitFailure.historyDeletionLost(detail: "x")) + .contains("前回"), + "the latching row must place the loss in a previous session") + + // It also co-occurs with a quarantine: independent facts about one startup. + // `rows` cannot establish that — it maps whatever list it is handed, so + // asserting over it would only be testing Array.map. The claim that matters + // is EngineContainer's, that the lost-deletion case is appended outside its + // mutually exclusive branch chain, which is what S4 below tests directly. + + // S4 (#312). A lost deletion must survive alongside a quarantine, which is + // the branch chain's masking case: routing it through the chain would let + // dataLossSuspected swallow it, and no assertion over `rows` could see it. + let coexisting = EngineContainer.historyFailures( + deletionLost: true, dataLossSuspected: true, detail: "d", deletionDetail: "x") + assertEqual(coexisting.count, 2, "a quarantine must not mask the lost deletion") + assertTrue( + coexisting.contains { + if case .historyDeletionLost = $0 { return true } else { return false } + }, + "the lost deletion is one of them") + assertEqual( + EngineContainer.historyFailures( + deletionLost: false, dataLossSuspected: true, detail: "d", deletionDetail: "x" + ).count, + 1, + "a quarantine alone is one row") + assertTrue( + EngineContainer.historyFailures( + deletionLost: false, dataLossSuspected: false, detail: "d", deletionDetail: "x" + ).isEmpty, + "a clean start reports nothing") + + // S4b (#312). Exactly one row is clickable, and it is the one backed by a + // file. IMKit builds this menu on its own without displaying it — measured + // on-device, the record was consumed four seconds after an untouched + // relaunch — so a click is the only evidence delivery actually happened. + let acknowledgeable = acrossRestart.filter { $0.acknowledgeable } + assertEqual(acknowledgeable.count, 1, "only the lost-deletion row is acknowledgeable") + assertEqual( + acknowledgeable.first?.title, + DegradedStatus.title(for: EngineInitFailure.historyDeletionLost(detail: "x")), + "and it is that row, not another") + assertTrue( + DegradedStatus.rows(initFailures: [.historyDataLoss(detail: "x")], runtimeIssues: []) + .allSatisfy { !$0.acknowledgeable }, + "a quarantine latches but has no durable record a click could retire") + + // S5 (#312). A wipe retracts the latched row — the row asks the user to + // delete an entry that no longer exists — and nothing else. + // + // Against `retractDeletionLostRow` directly, which is what production + // reaches. It used to go through a `historyWasCleared()` wrapper that no + // production path ever called: the wipe runs + // `EngineControlService.clearHistory`'s `defer { retractRowIfSettled() }`, + // and the interesting half is that gate — it asks the engine whether the + // report is still owed rather than inferring it from which action ran, so + // a wipe that throws before its commit point, or one whose unlink failed, + // keeps the row. Pinning the wrapper vouched for the unconditional shape + // that gate replaced. + let container = EngineContainer( + engine: nil, dictionary: nil, history: nil, userDict: nil, + initFailures: [.historyDeletionLost(detail: "x"), .historyDataLoss(detail: "y")]) + container.retractDeletionLostRow() + assertEqual(container.initFailures.count, 1, "only the lost-deletion row is retracted") + assertTrue( + container.initFailures.contains { + if case .historyDataLoss = $0 { return true } else { return false } + }, + "an unrelated latched failure survives a history wipe") } func testHistoryDurabilityFFI() { diff --git a/engine/crates/lex-core/Cargo.toml b/engine/crates/lex-core/Cargo.toml index 72ca7f3..7bddae7 100644 --- a/engine/crates/lex-core/Cargo.toml +++ b/engine/crates/lex-core/Cargo.toml @@ -28,9 +28,17 @@ anyhow = { version = "1", optional = true } candle-core = { version = "0.10", optional = true } candle-nn = { version = "0.10", optional = true } -[target.'cfg(target_vendor = "apple")'.dependencies] -# Direct fcntl(F_BARRIERFSYNC) for the WAL write barrier: std's sync_data -# maps to F_FULLFSYNC on Apple targets (~4ms vs ~0.3ms on M-series). +[target.'cfg(unix)'.dependencies] +# Two uses, one Apple-only and one every-unix, which is why this is `cfg(unix)` +# and not `cfg(target_vendor = "apple")`: +# +# - `fcntl(F_BARRIERFSYNC)` for the WAL write barrier, Apple-gated at its call +# site: std's `sync_data` maps to F_FULLFSYNC there (~4ms vs ~0.3ms on +# M-series). +# - `O_NOFOLLOW | O_NONBLOCK` in `persist::regular_only`, which every unix +# target needs. Leaving this table Apple-only made that code build on the +# macOS host and fail to resolve `libc` on Linux CI — a break a macOS-only +# local gate cannot see. libc = "0.2" [build-dependencies] diff --git a/engine/crates/lex-core/src/persist.rs b/engine/crates/lex-core/src/persist.rs index 9c571f9..76df217 100644 --- a/engine/crates/lex-core/src/persist.rs +++ b/engine/crates/lex-core/src/persist.rs @@ -69,22 +69,213 @@ pub(crate) fn ensure_parent_dir(path: &Path) -> io::Result<()> { /// rename can become durable before the file contents, manufacturing a corrupt /// file on power loss. The parent-dir fsync only makes the rename itself /// durable; per the LXUD design (§6) it is deliberately best-effort and -/// log-only — APFS journals renames, and the worst case of an unsynced rename -/// is rolling back to the previous file, never corruption. The tmp name +/// log-only **for this entry point** — APFS journals renames, and the worst +/// case of an unsynced rename is rolling back to the previous file, never +/// corruption. That is the right trade for a store whose file holds a complete +/// older state (the checkpoint, the user dictionary): a rollback costs recency, +/// which the next write re-establishes. It is the wrong trade for a store whose +/// previous content is a *weaker claim* — the deletion marker, where rolling +/// back turns `Lost` into a suppressible `Unflushed`, or into no file at all. +/// Those callers take [`write_atomic_staged`], which reports whether the name +/// became durable instead of collapsing it into `Ok`. The tmp name /// appends `.tmp` to the full file name ([`suffixed`]); `with_extension` would /// strip the store's extension and leave a stray sibling `.tmp`. +/// +/// `rename` protects the *destination* — it replaces a symlink or a FIFO +/// rather than writing through it — but that says nothing about the tmp, which +/// is opened by name like any other file. [`create_regular`] closes that end, +/// so neither half of the write can be diverted by whatever a restore or sync +/// tool left lying at either name. +/// +/// What this still cannot promise: `rename` promotes a *name*, not the +/// descriptor whose bytes were flushed. A writer that replaced the tmp between +/// the open and the rename would have its file promoted instead. POSIX offers +/// no fd-based rename, so the window cannot be closed by construction, and the +/// two ways to narrow it are both worse than stating it: a uniquely-named tmp +/// breaks the LXUD rule that the logical marker is the canonical file *and* +/// its orphan tmp (a crash must leave the stronger claim findable at a known +/// name), and a stat-before-rename only shrinks the window while reading as +/// though it closed it. It also buys nothing: anything that can write into +/// this directory can overwrite the destination outright at any moment, with +/// no window to hit. pub(crate) fn write_atomic(path: &Path, bytes: &[u8]) -> io::Result<()> { - let tmp = suffixed(path, ".tmp"); - ensure_parent_dir(path)?; - let mut f = File::create(&tmp)?; - f.write_all(bytes)?; - f.sync_all()?; + // Both outcomes are `Ok` here on purpose — see the trade above. Written as + // an explicit discard rather than `.map(|_| ())` so that adding a third + // outcome has to be answered for this caller too. + match write_atomic_staged(path, bytes) { + Ok(AtomicWrite::Durable | AtomicWrite::NameUnconfirmed) => Ok(()), + Err(e) => Err(e.into_io()), + } +} + +/// How far a *successful* [`write_atomic_staged`] got — the success-side twin +/// of [`AtomicWriteFailure`], and it exists for the same reason. +/// +/// That type's doc already states the rule: the stage a write reached "is not +/// something a caller can infer … so the writer says which it was". The +/// success path used to break it, collapsing both of these into `Ok(())` with +/// a log line. The two are not interchangeable for every store: the bytes are +/// durable in both, but in the second the *name* pointing at them is not, so a +/// power loss can restore whatever the path held before. +pub(crate) enum AtomicWrite { + /// Bytes flushed, and the directory entry naming them is flushed too. + Durable, + /// Bytes flushed and renamed, but the parent directory entry was not + /// synced. A power loss can roll the *name* back to its previous target — + /// including to no entry at all, when this write created it. + /// + /// Harmless where the previous target is a complete older state. Not + /// harmless where it is a weaker claim: a caller that treats this as landed + /// may stop guarding the very thing the write was recording. + NameUnconfirmed, +} + +/// How far [`write_atomic`] got before failing. +/// +/// The stage is not something a caller can infer, and one tried: reading the +/// tmp back and comparing bytes cannot tell a flushed image from one that +/// merely reached the page cache before `sync_all` failed. Both compare equal, +/// and treating the second as durable is precisely the power-loss window this +/// crate exists to close. So the writer says which it was. +pub(crate) enum AtomicWriteFailure { + /// The bytes never became durable — create, write or sync failed. + NotDurable(io::Error), + /// The bytes are flushed at the tmp path; only the rename failed. For a + /// store whose logical record includes its orphan tmp, the claim has + /// landed even though the name did not move. + FlushedNotRenamed(io::Error), +} + +impl AtomicWriteFailure { + pub(crate) fn into_io(self) -> io::Error { + match self { + Self::NotDurable(e) | Self::FlushedNotRenamed(e) => e, + } + } +} + +pub(crate) fn write_atomic_staged( + path: &Path, + bytes: &[u8], +) -> Result { + use AtomicWriteFailure::{FlushedNotRenamed, NotDurable}; + let tmp = tmp_path(path); + ensure_parent_dir(path).map_err(NotDurable)?; + let mut f = create_regular(&tmp).map_err(NotDurable)?; + f.write_all(bytes).map_err(NotDurable)?; + f.sync_all().map_err(NotDurable)?; drop(f); - fs::rename(&tmp, path)?; + if let Err(e) = fs::rename(&tmp, path) { + // The tmp's contents are durable — `sync_all` above — but its *name* + // may not be: `create_regular` added a directory entry that nothing has + // flushed yet. A caller that treats the orphan as a landed claim is + // relying on the next `read` finding it, so the entry has to survive a + // power loss too. Here the parent-dir fsync decides the outcome outright + // rather than grading it as the success path below does: with no rename + // to fall back on, a directory that was not synced leaves nothing about + // this write dependable, and the caller must not build on it. + // + // Confirmed undetectable by measurement: no deterministic test can + // tell a synced directory entry from an unsynced one without + // simulating power loss, so removing this survives the suite. It is + // carried by the argument, not by a test. + return Err(if sync_parent_dir(path) { + FlushedNotRenamed(e) + } else { + NotDurable(e) + }); + } if !sync_parent_dir(path) { - warn!("parent dir sync failed; rename durability unconfirmed (best-effort, by design)"); + warn!("parent dir sync failed; rename durability unconfirmed"); + return Ok(AtomicWrite::NameUnconfirmed); } - Ok(()) + Ok(AtomicWrite::Durable) +} + +/// The temporary path [`write_atomic`] writes through. +/// +/// Called by `write_atomic` itself, so a site that needs to name, sweep, or +/// block that file derives it from the same definition rather than re-spelling +/// the convention. The one that made this matter is a fault injector: a test +/// planting an obstacle at a separately-spelled path stops obstructing +/// anything the moment the writer moves, and passes vacuously instead of +/// failing. (Fixture code that asserts on a literal name is left alone — it +/// fails loudly rather than vacuously if the convention moves.) +pub(crate) fn tmp_path(path: &Path) -> PathBuf { + suffixed(path, ".tmp") +} + +/// Create-or-truncate a path, refusing anything that is not a regular file. +/// +/// `File::create` resolves a symlink at the final component and truncates +/// whatever it points at, and blocks indefinitely opening a FIFO that has no +/// reader. Both are reachable without an adversary — a restore or a sync tool +/// leaving an entry at a `.tmp` name is enough — and both are worse here than +/// a failed write: the first destroys a file this crate does not own, the +/// second hangs the thread that called it, which for the deletion marker is +/// the key-processing thread. +/// +/// Closed by construction rather than by a preceding `symlink_metadata` check, +/// which would leave the check and the open racing over the same name: +/// `O_NOFOLLOW` makes the symlink case fail in the open itself, `O_NONBLOCK` +/// turns the readerless-FIFO case into `ENXIO` instead of a wait, and the +/// `fstat` afterwards runs on the descriptor already obtained — so what it +/// reports is what was opened, not what the name resolves to now. That last +/// one is what catches a FIFO whose reader happens to be attached, plus +/// sockets and device nodes. +/// +/// Unix-only on purpose. A port would have to answer these for its own +/// namespace, and a portable fallback would answer "no protection" silently. +#[cfg(unix)] +fn create_regular(path: &Path) -> io::Result { + let mut opts = fs::OpenOptions::new(); + opts.write(true).create(true).truncate(true); + regular_only(&mut opts, path) +} + +/// Open an existing path for reading, refusing anything that is not a regular +/// file — the read counterpart of [`create_regular`], and the same single +/// resolution. +/// +/// A `symlink_metadata` check followed by an open is two resolutions of the +/// same name, so substituting a FIFO between them leaves the blocking open +/// intact; that is the shape this replaced. Callers distinguish `NotFound` +/// from every other error themselves — for the deletion marker only the former +/// is clean. +#[cfg(unix)] +pub(crate) fn open_regular(path: &Path) -> io::Result { + let mut opts = fs::OpenOptions::new(); + opts.read(true); + regular_only(&mut opts, path) +} + +/// Open through `opts` with the final component resolved exactly once, and +/// reject anything that is not a regular file. +/// +/// `O_NOFOLLOW` fails a symlink in the open itself rather than acting on its +/// target. `O_NONBLOCK` keeps a FIFO from turning the call into a wait — for +/// writing it becomes `ENXIO` with no reader, for reading it returns at once — +/// and has no effect on a regular file. The `fstat` then runs on the +/// descriptor already obtained, so what it reports is what was opened rather +/// than what the name resolves to now; that is what catches a FIFO whose other +/// end happens to be attached, plus sockets and device nodes. +/// +/// Unix-only on purpose. A port would have to answer these for its own +/// namespace, and a portable fallback would answer "no protection" silently. +#[cfg(unix)] +fn regular_only(opts: &mut fs::OpenOptions, path: &Path) -> io::Result { + use std::os::unix::fs::OpenOptionsExt as _; + + let f = opts + .custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK) + .open(path)?; + if !f.metadata()?.is_file() { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("{} is not a regular file", path.display()), + )); + } + Ok(f) } /// Best-effort fsync of the parent directory so the rename itself is durable. @@ -224,3 +415,46 @@ pub(crate) fn rotate_quarantined(path: &Path, keep: usize) { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_failed_rename_is_distinguished_from_a_failed_flush() { + // The stage has to come from the writer. A caller that read the tmp + // back and compared bytes could not tell these apart: a flushed image + // and one that only reached the page cache before `sync_all` failed + // compare equal, and calling the second durable is exactly the + // power-loss window the atomic write exists to close. + let dir = tempfile::tempdir().unwrap(); + + // Rename blocked by a non-empty directory at the destination; create, + // write and sync all succeed, so the bytes are durable at the tmp. + let dest = dir.path().join("store.bin"); + fs::create_dir(&dest).unwrap(); + fs::write(dest.join("occupant"), b"x").unwrap(); + match write_atomic_staged(&dest, b"payload") { + Err(AtomicWriteFailure::FlushedNotRenamed(_)) => {} + other => panic!("expected a rename-stage failure, got {:?}", other.is_ok()), + } + assert_eq!( + fs::read(tmp_path(&dest)).unwrap(), + b"payload", + "and the flushed bytes really are where the caller is told to look" + ); + + // Nothing durable: the tmp path itself cannot be created. + let other = dir.path().join("other.bin"); + fs::create_dir(tmp_path(&other)).unwrap(); + match write_atomic_staged(&other, b"payload") { + Err(AtomicWriteFailure::NotDurable(_)) => {} + v => panic!("expected a pre-durability failure, got {:?}", v.is_ok()), + } + + // The `sync_all` half is not reachable without fault injection in this + // module, which it deliberately does not have — it takes only paths + // and bytes. What is pinned here is that the two *reachable* stages + // are reported distinctly, which is what the caller branches on. + } +} diff --git a/engine/crates/lex-core/src/user_history/deletion_marker.rs b/engine/crates/lex-core/src/user_history/deletion_marker.rs new file mode 100644 index 0000000..5534a39 --- /dev/null +++ b/engine/crates/lex-core/src/user_history/deletion_marker.rs @@ -0,0 +1,516 @@ +//! Sidecar marker: a deletion the user asked for did not reach disk (#312). +//! +//! The runtime durability ledger lives in an atomic on `LexUserHistory`, so it +//! dies with the process — and the `Io` half of `DeletionNotPersisted` has no +//! startup heal: the old checkpoint still holds the entry and wins on the next +//! start. The report was therefore gone on the very restart where the deletion +//! resurrects. This file is the ledger's on-disk projection, so the fact +//! survives to the launch that materialises it. +//! +//! Layout — 16 fixed bytes, no CRC (see "fail-safe" below): +//! +//! | offset | size | field | content | +//! |--------|------|-------------|------------------------------------------| +//! | 0 | 4 | magic | `LXDM` | +//! | 4 | 1 | version | `1` | +//! | 5 | 1 | flags | bit0 = a witness seq follows | +//! | 6 | 2 | reserved | **must be 0** in version 1 | +//! | 8 | 8 | witness_seq | u64 LE, set with bit0; 0 is invalid | +//! +//! "Ignored on read" would be the usual convention for a reserved field, and +//! it is wrong here: `decode` accepts only what `encode` emits, so a non-zero +//! reserved byte resolves to `Lost` like any other unrecognised shape. A later +//! writer that read the field as ignorable and used it would turn every +//! witnessed `Unflushed` into an unconditional lost-deletion warning. Spending +//! it needs a version bump, which is what the version byte is for. +//! +//! **Fail-safe by construction: only `NotFound` means clean.** A read error, a +//! bad magic, an unknown version, any length other than [`LEN`], a witness of +//! 0 — every outcome other than "there is no file" resolves to the strongest +//! claim ([`DeletionBreach::Lost`], reported unconditionally). The last two are +//! the load-bearing ones, because they are the shapes that would otherwise +//! resolve toward *suppression*: a longer file read as a well-formed prefix, +//! and a zero seq that no applied_seq can fail to cover. Suppressing a report is the only direction that +//! demands a well-formed witness, which is why no CRC is needed: corruption can +//! only push the marker toward reporting. It is also why reading never returns +//! an error to the caller — surfacing one would let a sidecar nobody can read +//! fail the whole history open, i.e. stop learning outright. +//! +//! Deliberately holds **no** input strings: which entry was deleted is exactly +//! the text the deletion was meant to erase. The witness is a WAL seq. + +use std::fs; +use std::io; +use std::path::{Path, PathBuf}; + +use tracing::warn; + +use crate::persist; + +const MAGIC: &[u8; 4] = b"LXDM"; +const VERSION: u8 = 1; +const LEN: usize = 16; +const FLAG_WITNESS: u8 = 0b0000_0001; + +/// A deletion whose durability failed, in the form the next startup needs. +/// +/// The two halves differ in whether a restart heals them, which is the whole +/// reason the witness exists: reporting the healed half would be a latching +/// privacy alarm about data that is, in fact, gone. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum DeletionBreach { + /// No durable representation at all: the WAL append failed, so no frame + /// exists, and the synchronous checkpoint fallback failed too. The old + /// checkpoint still holds the entry and no restart heals it. + Lost, + /// The frame reached the WAL at `seq` but its flush was not confirmed. A + /// plain restart replays it (the deletion takes); only power loss can + /// still undo it — which is what makes the seq worth recording. + Unflushed { seq: u64 }, +} + +impl DeletionBreach { + /// Combine two breaches into the claim that covers both. + /// + /// `Lost` absorbs: one deletion with no durable representation is not made + /// healable by another that has a frame. Two `Unflushed` keep the **max** + /// seq — the suppression test asks whether a given state has reached this + /// seq, and the lower of two seqs can be covered while the higher is still + /// missing. + /// + /// Both rules are one-directional: no merge can weaken an outstanding + /// claim. That is what makes a read-modify-write safe against a concurrent + /// reader — and, with the write being tmp+rename through + /// [`crate::persist::write_atomic`], what makes the orphan tmp a crash can + /// leave harmless, since it can only carry a claim at least as strong. + pub fn merge(self, other: Self) -> Self { + match (self, other) { + (Self::Lost, _) | (_, Self::Lost) => Self::Lost, + (Self::Unflushed { seq: a }, Self::Unflushed { seq: b }) => { + Self::Unflushed { seq: a.max(b) } + } + } + } + + /// Whether this breach still stands against a state that has replayed up + /// to `applied_seq`. + /// + /// `Lost` always stands. `Unflushed` is settled once the state in question + /// includes its frame. *Which* state is the caller's choice, and recovery + /// deliberately asks twice with different ones rather than folding them + /// into a single comparison: against the **durable checkpoint's** + /// `applied_seq` to decide retraction, because only a checkpoint on disk + /// can settle a durability claim, and against the **loaded** `applied_seq` + /// only to choose between promoting the claim to `Lost` and handing it to + /// the runtime ledger. A witness that replay satisfied is explicitly not + /// retracted — replay proves the frame was readable, not that the flush + /// happened. A frame beyond a repaired tail leaves `applied_seq` short of + /// the witness, which is the power-loss case and correctly still stands. + pub fn outstanding(self, applied_seq: u64) -> bool { + match self { + Self::Lost => true, + Self::Unflushed { seq } => seq > applied_seq, + } + } + + fn encode(self) -> [u8; LEN] { + let mut buf = [0u8; LEN]; + buf[0..4].copy_from_slice(MAGIC); + buf[4] = VERSION; + if let Self::Unflushed { seq } = self { + buf[5] = FLAG_WITNESS; + buf[8..16].copy_from_slice(&seq.to_le_bytes()); + } + buf + } + + /// Total decode: **only what [`Self::encode`] can produce is accepted**; + /// everything else is `Lost`, and nothing panics. + /// + /// Written as a round-trip against the writer rather than as a series of + /// field checks. Field checks are what this was, and they were wrong four + /// times in a row — a length other than [`LEN`], an unknown version, a + /// witness of 0, an unrecognized flags byte — each found separately, each + /// the same bug: a byte the writer never emits, read as if it meant + /// something. Comparing against the encoding leaves no unchecked byte, so + /// there is no fifth one to forget. It also rejects a non-zero reserved + /// field, a witness flag with no seq, and a seq with no witness flag, + /// none of which anyone had thought to name. + /// + /// Reached from `#[uniffi::constructor]`, where a slice panic would cross + /// the FFI boundary. + fn decode(bytes: &[u8]) -> Self { + let Ok(exact) = <[u8; LEN]>::try_from(bytes) else { + return Self::Lost; + }; + if exact == Self::Lost.encode() { + return Self::Lost; + } + // Seq 0 cannot round-trip: `encode` writes it only for `Lost`, which + // the comparison above already claimed. + let seq = u64::from_le_bytes(exact[8..16].try_into().expect("8-byte field")); + let witnessed = Self::Unflushed { seq }; + if seq != 0 && exact == witnessed.encode() { + return witnessed; + } + Self::Lost + } +} + +/// Path of the marker for a history family (`.deletion-pending`). +/// +/// Suffixed, not `with_extension`: the family shares the checkpoint's full +/// file name so quarantine rotation and the clear sweep keep matching it. +/// It does **not** contain `.corrupt-`, so [`persist::quarantined_files`] +/// never picks it up (pinned by a test that calls the real predicate). +/// +/// `pub` for fault injection: a test in a dependent crate needs to name the +/// path to plant an obstacle at it. Nothing in production derives it outside +/// this module. +pub fn marker_path(checkpoint_path: &Path) -> PathBuf { + persist::suffixed(checkpoint_path, ".deletion-pending") +} + +/// Read the marker. `None` means — and only means — there is no file. +/// +/// See the module docs: every other outcome, including an unreadable file, is +/// [`DeletionBreach::Lost`]. +/// +/// Reads at most [`LEN`] bytes rather than `fs::read`, which pre-sizes its +/// buffer from the file's length: whatever sits at this path is attacker-free +/// but not size-checked, and this crate already holds the line that a length +/// taken from disk must not size an allocation (see `persist`'s bincode +/// readers). A longer file is malformed anyway — `decode` needs the first +/// [`LEN`] bytes, and a file that has more of them is not this format. +/// What the marker path holds, and whether the bytes behind it were actually +/// read. +/// +/// The two are different questions and conflating them cost a review round. +/// `breach` answers *what is claimed*, under the fail-safe rule that anything +/// unreadable claims `Lost`. `confirmed` answers *do we know that from the +/// file*, and only a successful read and decode sets it. A caller recording +/// what the disk holds — `OpenReport::marker_on_disk`, which seeds the +/// runtime's `flushed` — must use the second: believing a synthesized `Lost` +/// makes every later reconcile skip, leaving a live `Unflushed` witness on +/// disk that nothing will ever promote. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct MarkerObservation { + pub breach: DeletionBreach, + pub confirmed: bool, +} + +impl MarkerObservation { + /// What the disk holds, as a belief: only a confirmed read can name bytes. + pub fn state(self) -> MarkerState { + if self.confirmed { + MarkerState::Holds(self.breach) + } else { + MarkerState::Unknown + } + } +} + +/// What a process believes the marker path holds. +/// +/// Three states, because reality has three and an `Option` has two. The +/// missing one is `Unknown` — a file is there and nobody has managed to read +/// it — and collapsing that into "absent" is what let a failed unlink of an +/// unreadable marker look settled: the projection found the disk already in +/// the desired state, skipped, and the surviving file reported a lost deletion +/// on the next start. A `confirmed` bit bolted onto the *read* was the +/// half-measure; the belief itself has to carry it. +#[derive(Clone, Copy, PartialEq, Eq, Debug, Default)] +pub enum MarkerState { + /// Nothing there — an observed absence, or a removal that succeeded. + #[default] + Absent, + /// These exact bytes are there. + Holds(DeletionBreach), + /// A file is there and nobody has read it. Never equal to any desired + /// state, so a projection can never conclude it has nothing to do — which + /// is the entire point of the variant. + Unknown, +} + +impl MarkerState { + /// Whether the disk already says at least what is wanted. `Unknown` never + /// does. + /// + /// "At least", not "exactly", and the merge lattice decides it: claims are + /// one-directional, so a `Lost` on disk covers a desired `Unflushed` — it + /// reports more, never less, which is the only direction this format is + /// allowed to fail in. Exact equality deadlocked instead: with a stale + /// `Lost` surviving and a later `SyncFailed` wanting `Unflushed`, + /// `merge_write` absorbs the request back into `Lost` every time, so the + /// desired state was unreachable and every commit paid another key-thread + /// full sync without converging. + pub fn satisfies(self, desired: Option) -> bool { + match (self, desired) { + (Self::Absent, None) => true, + (Self::Holds(held), Some(want)) => held.merge(want) == held, + _ => false, + } + } +} + +pub fn read(checkpoint_path: &Path) -> Option { + let path = marker_path(checkpoint_path); + // The marker *and* any orphan tmp beside it. A crash between the tmp's + // flush and the rename leaves the stronger claim in the sibling, and + // reading only the marker would hand the next startup a witness it can + // suppress. Merging makes the orphan able to strengthen the claim and + // never to weaken it, which is what lets this go back through the shared + // atomic write instead of a hand-rolled in-place one. + let claims = [read_at(&path), read_at(&persist::tmp_path(&path))]; + claims + .into_iter() + .flatten() + .map(|(bytes, readable)| MarkerObservation { + breach: DeletionBreach::decode(&bytes), + confirmed: readable, + }) + .reduce(|a, b| MarkerObservation { + breach: a.breach.merge(b.breach), + // Both halves, since either one being a guess makes the pair a + // guess about what the path as a whole holds. + confirmed: a.confirmed && b.confirmed, + }) +} + +/// One file's bytes, under the fail-safe rule: `None` means — and only means — +/// there is no file, and anything unreadable comes back as a buffer that +/// [`DeletionBreach::decode`] resolves to `Lost`. +fn read_at(path: &Path) -> Option<(Vec, bool)> { + // Through one descriptor, not a `symlink_metadata` check followed by an + // open: those are two pathname resolutions, and a restore or a sync tool + // replacing the checked regular file with a FIFO in between leaves the + // blocking open exactly where it was. This runs synchronously inside + // `LexUserHistory::open`, on the thread the IME starts up on, so that open + // never returning means the input method never becomes available. + // `open_regular` resolves the name once and validates what it got. + let mut file = match persist::open_regular(path) { + Ok(f) => f, + Err(e) if e.kind() == io::ErrorKind::NotFound => return None, + Err(e) => { + warn!("unpersisted-deletion marker unreadable ({e}); reporting conservatively"); + return Some((Vec::new(), false)); + } + }; + // LEN + 1, so a longer file is *seen* to be longer rather than read as a + // well-formed prefix — the round-trip in `decode` then rejects it. + let mut buf = Vec::with_capacity(LEN + 1); + match io::Read::read_to_end(&mut io::Read::take(&mut file, LEN as u64 + 1), &mut buf) { + Ok(_) => Some((buf, true)), + Err(e) => { + warn!("unpersisted-deletion marker unreadable ({e}); reporting conservatively"); + Some((Vec::new(), false)) + } + } +} + +/// Merge `breach` into whatever the marker already claims and write it back. +/// +/// Read-modify-write, not a plain overwrite. A full replacement is +/// last-write-wins, and an `Unflushed` landing on top of an outstanding `Lost` +/// would downgrade the claim to one the next startup can suppress — losing +/// exactly the report this file exists for. +/// +/// The reachable route to that ordering is not the obvious one, and stating it +/// wrongly is how the first version of this file grew a test that proved +/// nothing. An `Io` append *freezes* the WAL, and the frozen guard turns every +/// later append in the session into `Io` too — so `Lost` cannot be followed by +/// an `Unflushed` while the freeze holds. What lifts it is a compaction whose +/// `snapshot_to_cover` generation predates the `Io` raise: it early-returns +/// from the cover (leaving the marker outstanding) yet still reaches +/// `truncate_covered`, which thaws the file. The next tombstone can then append +/// and fail its flush, arriving as `Unflushed` on top of a standing `Lost`. +/// +/// **Through [`write_atomic`], the shared primitive.** A revision of this file +/// wrote in place instead, on the argument that a torn marker decodes to +/// `Lost` so atomicity buys nothing, and that tmp+rename's crash window hides +/// a stronger claim in a sibling. The first half was true and the second was +/// the wrong fix: hiding is what [`read`] now prevents by merging the orphan, +/// which can only strengthen a claim. What writing by hand cost was everything +/// `write_atomic` already encodes — three review rounds re-derived it one +/// durability detail at a time (a symlink at the path being followed and its +/// target truncated, an unlink failure left unchecked before `File::create`, a +/// newly created directory entry never fsynced so power loss could drop the +/// filename while its contents were durable). `rename` replaces a symlink or a +/// FIFO at the destination rather than writing through it, and syncs the +/// parent. `persist`'s own module doc calls these the single-source +/// implementations "so the stores cannot drift apart on the durability +/// details"; this file drifted, and is back. +/// +/// Callers hold the wal mutex, which serializes the read against a concurrent +/// write. The one exception is recovery's promotion of an unsatisfied witness, +/// which runs before the `HistoryWal` enters its mutex and is exclusive by +/// ownership rather than by locking. +/// +/// Measured on an M4 (release, APFS) at **p50 10.1ms**, against **12.3ms p50** +/// for the synchronous fallback checkpoint the same call runs immediately +/// afterwards. The caller skips a projection whose value it has already +/// flushed, so the cost is per *change* of claim, not per raise. +/// +/// A barrier flush instead of `sync_all` would cost ~0.3ms and would still +/// cover the scenario #312 is named for (a process restart keeps the page +/// cache), but it would reopen a power-loss window in the *report* about a +/// deletion whose own power-loss window §6 sets to zero. +/// What a completed write lets this process believe the path now holds. +/// +/// The mirror of [`MarkerObservation::state`] on the write side, and it exists +/// for the same reason: a claim and the knowledge of it are different facts. +/// `Durable` names bytes. `NameUnconfirmed` does not — the bytes are flushed +/// but the directory entry pointing at them is not, so a power loss can restore +/// whatever the path held before the write. For this file that is a *weaker* +/// claim (a promoted `Lost` falling back to a suppressible `Unflushed`, or to +/// no marker at all), which is the one direction the format may never fail in. +/// +/// `Unknown` rather than a new variant, because it is the same belief already +/// spelled there: this process cannot name what the path holds. Everything the +/// caller needs follows from that and is not re-derived here — `satisfies` is +/// false, so every commit re-projects until a write confirms, and the freeze +/// predicate in `apply_records` (`flushed != Holds(Lost)`) keeps appends off +/// the WAL until the promotion is durably named. +/// +/// Confirmed undetectable by measurement, like its sibling in `persist`: no +/// deterministic test can fail a directory fsync without simulating power loss. +/// The mapping below is testable and is what `test_an_unconfirmed_name_is_not_a +/// _belief` pins; the syscall failing is carried by the argument. +fn belief(outcome: persist::AtomicWrite, merged: DeletionBreach) -> MarkerState { + match outcome { + persist::AtomicWrite::Durable => MarkerState::Holds(merged), + persist::AtomicWrite::NameUnconfirmed => MarkerState::Unknown, + } +} + +pub fn merge_write(checkpoint_path: &Path, breach: DeletionBreach) -> io::Result { + // The claim only — whether the existing bytes were readable does not + // change what has to be written, and merging is one-directional so an + // unreadable existing marker (conservatively `Lost`) can only strengthen. + let merged = read(checkpoint_path).map_or(breach, |existing| existing.breach.merge(breach)); + let image = merged.encode(); + match persist::write_atomic_staged(&marker_path(checkpoint_path), &image) { + Ok(outcome) => Ok(belief(outcome, merged)), + // The logical marker is the canonical file **and** its orphan tmp — + // `read` merges them and `remove` clears both — and that rule holds + // here too: `write_atomic` flushes the tmp before renaming, so a + // rename that fails (a non-empty directory sitting at the canonical + // name, say) has still made the claim durable where the next `read` + // will find it. Reporting failure there made the claim look unlanded + // forever — appends froze on every commit, each compaction thawed + // them, and the next keystroke froze again. + // + // Which stage failed is taken from the **writer**, never inferred. An + // earlier version compared the bytes back and accepted a match: that + // cannot tell a flushed image from one that only reached the page + // cache before `sync_all` failed, since both compare equal — and + // calling the second durable is exactly the window this file exists to + // close. + // + // `Holds`, not `Unknown`, and that is not the success path's rule being + // contradicted: this branch is only reached when `sync_parent_dir` + // *succeeded*, so the orphan's own directory entry is durable and the + // next `read` will find it and merge it. The claim is named; only the + // canonical file's name is not, and `read` does not need it. + Err(persist::AtomicWriteFailure::FlushedNotRenamed(e)) => { + warn!("marker rename failed ({e}); the flushed orphan carries the claim"); + Ok(MarkerState::Holds(merged)) + } + Err(persist::AtomicWriteFailure::NotDurable(e)) => Err(e), + } +} + +/// Remove the marker, reporting whether the record is now gone. +/// +/// **Both files.** [`read`] merges the atomic write's orphan tmp so that a +/// crash between its flush and the rename can only strengthen a claim; the +/// logical marker is therefore the pair, and removing one of them would leave +/// the claim standing. A `Lost` orphan would then re-report on every launch +/// with no acknowledgement able to clear it — the unclearable latch, rebuilt +/// out of the fix for hiding. +/// +/// `true` means the record is gone — removed, or never there. `false` means it +/// stands, and the caller must keep telling the user so: an acknowledgement +/// that says it succeeded while the marker survives drops the row, takes away +/// the retry, and lets the warning come back on the next launch anyway. +/// +/// Best-effort in the sense that it does not retry — the disk this runs +/// against is the one that just failed — but never in the sense of hiding the +/// outcome. +/// +/// A **directory** at either path is removed only when empty. It is not ours: +/// something external put it there, and `remove_dir_all` would both walk an +/// unbounded tree on the thread the menu runs on and delete whatever a restore +/// had placed inside. An empty one is a placeholder and safe to clear; a full +/// one stays, and the `false` return makes that visible instead of silent — +/// which is the honest form of the "unclearable latch" this fallback was added +/// to prevent, since the user is now told the acknowledgement did not take. +pub fn remove(checkpoint_path: &Path) -> bool { + let path = marker_path(checkpoint_path); + // Both, and `&` not `&&`: the orphan must be attempted even when the + // canonical marker refuses, or a `false` return would leave a claim the + // caller was never told about. + remove_one(&path) & remove_one(&persist::tmp_path(&path)) +} + +fn remove_one(path: &Path) -> bool { + let Err(e) = fs::remove_file(path) else { + return true; + }; + if e.kind() == io::ErrorKind::NotFound { + return true; + } + if fs::remove_dir(path).is_ok() { + warn!( + "removed an empty directory left at the marker path {}", + path.display() + ); + return true; + } + warn!( + "failed to remove unpersisted-deletion marker {}: {e}", + path.display() + ); + false +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::persist::AtomicWrite; + + #[test] + fn test_an_unconfirmed_name_is_not_a_belief() { + // The whole of the fix, in the one place it is measurable. The syscall + // that produces `NameUnconfirmed` cannot be failed deterministically — + // a directory fsync only fails under conditions a test cannot stage — + // so what a test can pin is that the *outcome* is not laundered into a + // positive belief on its way to `flushed`. + // + // Direction matters, not just inequality: `Holds` would make + // `satisfies` true, the projection skip, and the freeze lift, so a + // power loss rolling the directory entry back to the weaker claim the + // promotion replaced would go unnoticed and unretried. + assert_eq!( + belief(AtomicWrite::Durable, DeletionBreach::Lost), + MarkerState::Holds(DeletionBreach::Lost), + "a durably named write is the one thing that may name bytes" + ); + assert_eq!( + belief(AtomicWrite::NameUnconfirmed, DeletionBreach::Lost), + MarkerState::Unknown, + "flushed bytes under an unflushed name are not a claim this \ + process can say the disk holds" + ); + // The witness half too: `Unflushed` is the claim a rollback would + // *restore*, so writing one must not be the case that gets the + // exemption for being weaker. + assert_eq!( + belief( + AtomicWrite::NameUnconfirmed, + DeletionBreach::Unflushed { seq: 7 } + ), + MarkerState::Unknown, + ); + } +} diff --git a/engine/crates/lex-core/src/user_history/mod.rs b/engine/crates/lex-core/src/user_history/mod.rs index fa8b40a..888e765 100644 --- a/engine/crates/lex-core/src/user_history/mod.rs +++ b/engine/crates/lex-core/src/user_history/mod.rs @@ -3,6 +3,7 @@ //! Records confirmed conversions and uses frequency × recency scoring to //! promote learned candidates in subsequent sessions. +pub mod deletion_marker; mod persistence; pub mod recovery; #[cfg(test)] @@ -570,6 +571,16 @@ impl UserHistory { results } + /// Whether this history holds nothing at all. + /// + /// Used by recovery to settle an unpersisted-deletion marker: a claim that + /// some entry survived a deletion is false, not merely stale, when there + /// is no entry. Mirrors the reasoning `clear` uses when it covers the + /// ledger from its empty checkpoint. + pub fn is_empty(&self) -> bool { + self.unigrams.is_empty() && self.bigrams.is_empty() + } + /// Iterate all unigram records as (reading, surface, entry). /// Used by offline tooling (`lextool history-audit`) to mine the history. pub fn unigrams(&self) -> impl Iterator { @@ -701,3 +712,21 @@ impl UserHistory { self.durable_residue.cover(&snapshot.durable_residue); } } + +/// Test support: the temporary path a durable checkpoint write goes through. +/// +/// Exported so a fault-injection test in a dependent crate can put a directory +/// here and fail the checkpoint write — and only that write — leaving the rest +/// of the family (notably the unpersisted-deletion marker) writable and the +/// existing checkpoint readable, which is what a restart-crossing test needs. +/// +/// It forwards to the definition `write_atomic` itself calls. Re-spelling the +/// convention here would have made the obstacle independent of the path the +/// writer actually uses, so a change inside `write_atomic` would leave the +/// injector planting a directory nobody visits and the tests passing +/// vacuously. No production caller; `persist` is crate-private, so this is the +/// seam. A proper injectable writer for checkpoints — the shape `WalIo` gives +/// the WAL — would retire both this and the older parent-as-a-file trick. +pub fn checkpoint_tmp_path(checkpoint_path: &std::path::Path) -> std::path::PathBuf { + crate::persist::tmp_path(checkpoint_path) +} diff --git a/engine/crates/lex-core/src/user_history/recovery.rs b/engine/crates/lex-core/src/user_history/recovery.rs index 9491987..b870342 100644 --- a/engine/crates/lex-core/src/user_history/recovery.rs +++ b/engine/crates/lex-core/src/user_history/recovery.rs @@ -12,6 +12,19 @@ //! migration writes). Offline tooling must keep using the strict, side- //! effect-free [`UserHistory::open`] / [`super::wal::open_with_wal`]: an //! audit tool renaming a live IME's files would be an incident. +//! +//! It also runs **before the history is shared**: the `HistoryWal` it returns +//! has not entered its mutex yet and no session can reach it. That is the +//! standing exemption for the one write it makes to the unpersisted-deletion +//! marker, which everywhere else in the engine happens only under the wal mutex +//! (the mutex is what makes a raise and a cover unable to interleave). A future +//! path that re-opens a *live* history would break the exemption, not merely +//! bend it. This function removes the marker on two grounds and no others: a +//! *durable checkpoint* on disk already covers the witness — including the one +//! the migration path writes itself — or the `Lost` claim is vacuous, both the +//! durable set and the loaded state being empty, so no entry exists for the +//! deletion to have failed against. Replay reaching the witness is neither: it +//! proves the frame was readable, not flushed. use std::fs; use std::io; @@ -21,6 +34,7 @@ use tracing::{info, warn}; use crate::persist; +use super::deletion_marker; use super::persistence::{load_checkpoint, CheckpointLoaded}; use super::wal::{ classify_wal, legacy_valid_prefix, scan_legacy, scan_v2, wal_path_for, HistoryWal, WalFormat, @@ -116,6 +130,91 @@ pub struct OpenReport { /// Startup-compaction hint (§5.1-6): recovery results should be /// checkpointed early so the next startup is clean. Consumed in PR2. pub compaction_recommended: bool, + /// A **v1 checkpoint is still on disk** waiting for its migration commit to + /// be retried, and no legacy WAL was consumed. + /// + /// While this holds, no compaction may run at all: a compaction is not the + /// migration — it writes a v2 checkpoint over the v1 file with none of the + /// commit's steps (no `.v1.bak`, no `Migrated`) — so it would destroy the + /// v1 bytes on the one path where they are the only copy. + /// + /// A *field* rather than a guard at each scheduler, because guarding + /// schedulers is inherently incomplete: there are five of them (the + /// startup hint, the scrub branch, the threshold, `clear`'s heal, and + /// `FollowUp`), nothing stops a sixth, and three review rounds went into + /// gating them one at a time. `run_compact_impl` refuses instead, which is + /// the single place that performs the write. + /// + /// What this field states is what recovery *observed*, so the runtime seeds + /// a cell from it rather than reading it as the live answer: a `clear` + /// writes an empty v2 checkpoint over the same path and supersedes the v1 + /// bytes mid-session. See `HistoryResource::v1_checkpoint_retained`. + /// + /// The legacy-WAL variant is excluded because there the compaction **is** + /// the intended heal: that path freezes the WAL, and the compaction which + /// thaws it writes the v2 checkpoint as a side effect, completing the + /// conversion. + pub v1_checkpoint_retained: bool, + /// A previous session could not persist a deletion the user asked for, + /// and nothing since has covered it (#312) — so the state just loaded may + /// still hold the entry that deletion was meant to remove. + /// + /// Its own field for the same reason `migration_failed` is: on this path + /// `checkpoint_state` is *truthfully* `Loaded` — the checkpoint read + /// perfectly, it just contains something that should be gone — so no + /// existing enum can carry the fact without a state per combination. It + /// deliberately stays out of `data_loss_suspected()`, whose user-facing + /// wording is "some past learning was lost": this is the opposite loss, + /// data that survived when it should not have. + /// + /// Feeds `compaction_recommended` **when the disk does not already say + /// `Lost`** — a reversal of the note that used to sit here, kept as a + /// record of why. + /// + /// That note said a compaction "would checkpoint the resurrected entry and + /// cover the ledger — i.e. tell the user everything is fine". All three + /// premises are false now, and the last was false when it was written: + /// + /// - it cannot cover the ledger — nothing raised it this session, so the + /// cover early-returns, which the note's own parenthetical conceded; + /// - it cannot tell the user anything is fine — the row is driven by + /// `inherited_owed` and the latched `EngineInitFailure`, and a + /// compaction touches neither; + /// - re-checkpointing the resurrected entry changes nothing. The entry is + /// already in the durable checkpoint. That is *why* it resurrected. + /// + /// What a compaction does do is project, and with the inherited claim + /// standing the projection writes `Lost`. That is the point: recovery's + /// promotion is best-effort, and when it fails the disk keeps the + /// *suppressible* `Unflushed{seq}` under a memory claim of `Lost`. A + /// healthy session raises nothing, so nothing re-projects until a + /// threshold compaction a thousand frames away — and ordinary commits + /// advance the WAL past the witness in the meantime, so a restart before + /// then reads the witness as replayed and suppresses a report that is + /// still owed. Scheduling the compaction is what makes the promotion + /// actually happen. + pub deletion_lost: bool, + /// What the marker file holds once this function is done with it, as + /// observed rather than assumed. + /// + /// The runtime seeds `MarkerClaims::flushed` from this. That field means + /// "what the disk holds, as far as we know", and starting it at `None` was + /// a claim recovery is in a position to contradict: a retraction whose + /// unlink failed, or a promotion whose write failed, both leave bytes + /// behind that the process would then believe were gone. The promotion + /// case is the sharp one — the disk keeps the *suppressible* + /// `Unflushed{seq}` while memory holds `Lost`, so a later checkpoint can + /// satisfy the stale witness and silently retract the very report the + /// promotion exists to make unconditional. + /// + /// Internal; not surfaced over UniFFI. + pub marker_on_disk: deletion_marker::MarkerState, + /// A previous session's deletion *was* applied by this startup's replay, + /// but out of the page cache — the flush that failed never happened, so + /// power loss still undoes it. Not a report: a live durability problem the + /// engine seeds its runtime ledger from, so the first durable checkpoint + /// retracts it. Internal; not surfaced over UniFFI. + pub deletion_pending_checkpoint: bool, } impl OpenReport { @@ -137,6 +236,7 @@ impl OpenReport { && !self.migrated_from_v1 && !self.migration_failed && !self.appends_frozen + && !self.deletion_lost } } @@ -156,6 +256,10 @@ pub fn open_recovering( appends_frozen: false, replayed_deletion: false, compaction_recommended: false, + v1_checkpoint_retained: false, + marker_on_disk: deletion_marker::MarkerState::Absent, + deletion_lost: false, + deletion_pending_checkpoint: false, }; // --- 1. checkpoint --- @@ -178,7 +282,29 @@ pub fn open_recovering( let v1_checkpoint_present = migrate; // --- 2. WAL --- + // The checkpoint's own coverage, before replay moves `applied_seq`. The + // marker below needs the two apart: a witness the *checkpoint* covers is + // durably persisted, while one only *replay* reaches is still riding the + // page cache. + let checkpoint_applied_seq = history.applied_seq(); + // …and whether it holds anything, before replay can change the answer. + let checkpoint_empty = history.is_empty(); let mut wal = HistoryWal::new(checkpoint_path); + // Read once, here; §3b reuses the value, and nothing between writes the + // file. + // + // A sequence *floor* used to be installed from an outstanding + // `Unflushed{seq}` here, on the theory that a rebase must not re-issue the + // number the claim names. That was a mis-model and is gone: the witness + // test is an **inequality** (`seq > applied_seq`), so any later frame above + // the witness satisfies it — gaps are legal and `applied_seq` is a high + // water mark. Skipping one number changes nothing. The seq is a *position + // within one WAL file*, not an epoch, and it stops meaning anything the + // moment that file is replaced; only promotion to `Lost` survives a + // lineage change, which is why promotion is mandatory rather than an + // optimization, and why a report that is owed with the promotion unlanded + // freezes appends (see `apply_records`). + let marker = deletion_marker::read(checkpoint_path); let mut legacy_wal_consumed = false; match fs::read(&wal_path) { Err(e) if e.kind() == io::ErrorKind::NotFound => { @@ -347,6 +473,142 @@ pub fn open_recovering( report.migration_failed = !report.migrated_from_v1; } + // --- 3b. unpersisted-deletion marker (#312) --- + // Evaluated here, after the migration commit, because the question it asks + // is "does the checkpoint that is durable **when this function returns** + // cover the witness" — and on the migration path this function writes that + // checkpoint itself. Asking before the commit answered against a v1 file + // that predates the deletion, so a migration whose save had just persisted + // the deletion still handed the engine a live durability warning and left + // the marker for an asynchronous compaction to clean up. One evaluation + // point against one durable state, rather than a settle-again special case + // in the migration branch. + // + // `durable_applied_seq` is what is on disk now: the checkpoint this startup + // wrote if the migration committed (it was serialized from `history`, so it + // covers everything memory holds), otherwise the one that was loaded. + let (durable_applied_seq, durable_empty) = if report.migrated_from_v1 { + (history.applied_seq(), history.is_empty()) + } else { + (checkpoint_applied_seq, checkpoint_empty) + }; + // A retraction this startup decided but could not carry out. The verdict + // is permanent — the entries the claim spoke of are gone, and later + // learning is not them — but the only place to record it is the file we + // just failed to unlink, so the debt has to be handed to the runtime + // instead. Recovery is the last marker site outside the single projection + // writer (`apply_marker`), and this is the hole that left: the runtime's + // projection does retry a stale file, but only when a compaction runs, and + // the next one is a thousand frames away. A restart before then reloads + // the marker against a history that replay has made non-empty, and reports + // a loss this startup already refuted. Scheduling the compaction *now* is + // what makes the retry prompt, through the channel other recovery results + // already use rather than a mechanism of its own. + let mut marker_retraction_stuck = false; + if let Some(observed) = marker { + let breach = observed.breach; + if breach == deletion_marker::DeletionBreach::Lost && durable_empty && history.is_empty() { + // `Lost` says an entry survived the deletion. Refuting that takes + // **both** halves, and each alone was wrong once: + // + // - the durable set alone — a checkpoint emptied by `clear` — says + // nothing when replay brings entries back from the WAL; + // - the loaded state alone lets a replayed tombstone empty memory + // while the checkpoint still holds the entry, and `decode` maps + // *malformed* input to `Lost` too, so a garbled `Unflushed` + // would be retracted as a refuted presence claim and a power + // loss would restore the entry with nothing reported. + // + // Together they say what the claim actually needs: no entry on + // disk, and none replayed back. That also removes any need to tell + // a decoded `Lost` from a fallback one — neither is refutable + // while the checkpoint still holds something. + info!("an unpersisted-deletion marker outlived the entries it referred to"); + marker_retraction_stuck = !deletion_marker::remove(checkpoint_path); + if marker_retraction_stuck { + // `state()`, not the claim: an unlink that failed on a file + // nobody could read leaves the disk *unknown*, and recording + // that as absence let the projection skip the retry. + report.marker_on_disk = observed.state(); + } + } else if !breach.outstanding(durable_applied_seq) { + // A durable checkpoint contains the deletion's effect, so it is + // persisted. Either a crash landed between a successful `save()` + // and the unlink that follows it, or the migration above just wrote + // the covering checkpoint. Retracting is sound because the evidence + // is a checkpoint on disk. + info!("an unpersisted-deletion marker is covered by a durable checkpoint"); + marker_retraction_stuck = !deletion_marker::remove(checkpoint_path); + if marker_retraction_stuck { + report.marker_on_disk = observed.state(); + } + } else if breach.outstanding(history.applied_seq()) { + // The frame is provably not in the state we just loaded, so the + // deletion did not take and nothing will make it take. Promote the + // claim to unconditional: seq numbering is *re-based* whenever a + // WAL is quarantined or reinitialized (`adopt_empty` restarts at + // the checkpoint's applied_seq + 1), so an unrelated later frame + // could otherwise satisfy this witness and settle a report that is + // still owed. Having answered the question once, the answer stops + // depending on a comparison a reset can invalidate. + warn!("a deletion from a previous session was never persisted ({breach:?})"); + report.deletion_lost = true; + // Best-effort, and the runtime is told which way it went. A + // promotion that fails leaves the *suppressible* witness on disk + // under a memory claim of `Lost`, so the next checkpoint to reach + // that seq would retract a report that is still owed. Handing the + // observed value out means the runtime's first projection sees + // disk != desired and re-asserts, rather than believing a write + // that never landed. + // `confirmed` gates the whole thing: a marker that could not be + // read comes back as `Lost` by the fail-safe rule, and recording + // that fallback as an observation would have the runtime believe + // the disk holds `Lost` when it may hold a live `Unflushed` + // witness. `flushed` would then match, every reconcile would skip, + // and the witness would sit there until something satisfied it. + report.marker_on_disk = if !observed.confirmed { + deletion_marker::MarkerState::Unknown + } else if breach == deletion_marker::DeletionBreach::Lost { + deletion_marker::MarkerState::Holds(breach) + } else { + match deletion_marker::merge_write( + checkpoint_path, + deletion_marker::DeletionBreach::Lost, + ) { + // The writer's belief verbatim. It is `Unknown` when the + // bytes are flushed but the directory entry naming them is + // not — the same "suppressible witness under a memory claim + // of `Lost`" hazard the `Err` arm below is about, reached + // through a power loss rolling the name back instead of + // through the write failing outright. Wrapping the value in + // `Holds` here is what hid it: the caller decided what the + // disk held from the fact that the call returned. + Ok(belief) => belief, + Err(e) => { + warn!("failed to promote the unpersisted-deletion claim: {e}"); + deletion_marker::MarkerState::Holds(breach) + } + } + }; + } else { + // Replay applied the deletion and no durable checkpoint covers it, + // so nothing is owed to the user — but replay read that frame out + // of the page cache, which is not the flush that failed: until a + // checkpoint covers it, power loss still undoes the deletion. + // Retracting here would be retract-then-persist, the inverse of the + // discipline every other write on this path follows. Hand the claim + // to the runtime ledger instead — it is a live durability problem + // now, and the first durable checkpoint both settles it and unlinks + // the file. + info!("an unflushed deletion replayed; it stands until a checkpoint covers it"); + report.deletion_pending_checkpoint = true; + report.marker_on_disk = deletion_marker::MarkerState::Holds(breach); + // Reachable only with a decoded `Unflushed` — an unreadable marker + // resolves to `Lost`, which is always outstanding and never lands + // here — so this observation is confirmed by construction. + } + } + // Whatever branch froze the WAL — or left it frozen — this session's // appends are memory-only until a compaction heals the file. Derived // once, here, so a future freeze site cannot report a clean startup by @@ -387,7 +649,27 @@ pub fn open_recovering( // re-attempts properly. The legacy-WAL variant still heals, via // `appends_frozen` below: there the WAL is frozen, which is a real // degradation the compaction genuinely fixes. - report.compaction_recommended = report.migrated_from_v1 + // The two marker feeders below are **promptness, not correctness**. What + // guarantees the disk eventually agrees with the projection is that every + // commit reconciles it (`apply_records`, under the wal mutex, before the + // appends) — a free memory comparison when they already agree. These only + // spare a user whose startup was degraded from waiting until their next + // keystroke, and losing one costs nothing but latency. Keeping that + // division explicit matters: six review rounds were spent adding retry + // triggers one event at a time, and the answer was never another trigger. + let marker_prompt = marker_retraction_stuck + // A report is owed but the disk does not yet say so + // unconditionally: the promotion above failed. Scheduled so the + // runtime's projection re-asserts `Lost` promptly, because nothing + // else will — see `deletion_lost`'s doc for why the old "never + // schedule here" rule was wrong. Skipped when the disk already + // holds `Lost`, since then the projection and the file agree and a + // compaction buys nothing. + || (report.deletion_lost + && report.marker_on_disk + != deletion_marker::MarkerState::Holds(deletion_marker::DeletionBreach::Lost)); + report.compaction_recommended = marker_prompt + || report.migrated_from_v1 || report.data_loss_suspected() || (report.checkpoint_state == CheckpointState::Missing && report.frames_replayed > 0) || report.frames_skipped > 0 @@ -395,6 +677,30 @@ pub fn open_recovering( || wal.needs_compact() || report.appends_frozen; + // …and then vetoed outright when a v1 checkpoint is still on disk waiting + // for its commit to be retried. A compaction is **not** the migration — it + // writes a v2 checkpoint over the v1 file with none of the commit's steps + // (no `.v1.bak`, no `Migrated`) — so running one here destroys the v1 bytes + // on exactly the path where they are the only copy. That is the hazard + // `migration_failed`'s own doc states, and it is a property of *running a + // compaction at all*, not of any one feeder: gating the marker feeders + // alone left `replayed_deletion` scheduling the same worker, which a + // replayed `Unflushed` witness necessarily sets. + // + // The legacy-WAL variant is exempt because there the compaction *is* the + // intended heal: that path freezes the WAL, and the compaction which thaws + // it writes the v2 checkpoint as a side effect, completing the conversion. + // Without a legacy WAL there is nothing to heal and everything to lose, so + // the next launch retries the commit properly. + report.v1_checkpoint_retained = report.migration_failed && !legacy_wal_consumed; + if report.v1_checkpoint_retained { + // Not the guard — `run_compact_impl` refuses on the cell this seeds, + // and that is what makes this safe. This only avoids spawning a worker + // that would immediately give up; the schedulers do the same for the + // requests that arise after startup. + report.compaction_recommended = false; + } + // --- 5. quarantine rotation + v1-backup GC --- persist::rotate_quarantined(checkpoint_path, QUARANTINE_KEEP); gc_v1_backup(checkpoint_path); @@ -432,10 +738,19 @@ pub fn v1_backup_path(checkpoint_path: &Path) -> PathBuf { /// Remove recovery artifacts (`.v1.bak`, `.corrupt-*`, stray `.tmp`) for /// this history family. `clear()` calls this: a privacy wipe must not leave /// rescued bytes behind. +/// +/// Deliberately **not** the sweep for the unpersisted-deletion marker, even +/// though that is a family member too: this returns on its first non-NotFound +/// error and only reaches the `.corrupt-*` files afterwards, so letting a +/// stubborn 16-byte sidecar that holds no user text stand in front of files +/// that hold plenty would be the wrong trade. `clear_impl` removes the marker +/// itself. (The same fail-fast shape means a stubborn `.v1.bak` can already +/// block the quarantine sweep, which does hold user text — a pre-existing +/// weakness of this helper, not one the marker introduces.) pub fn remove_recovery_artifacts(checkpoint_path: &Path) -> io::Result<()> { for path in [ v1_backup_path(checkpoint_path), - persist::suffixed(checkpoint_path, ".tmp"), + persist::tmp_path(checkpoint_path), // The v1 writer used `with_extension("tmp")` (`user_history.tmp`): // a pre-upgrade crash before rename can leave a full serialized // history copy under that name. diff --git a/engine/crates/lex-core/src/user_history/tests_recovery.rs b/engine/crates/lex-core/src/user_history/tests_recovery.rs index af981e4..129e5af 100644 --- a/engine/crates/lex-core/src/user_history/tests_recovery.rs +++ b/engine/crates/lex-core/src/user_history/tests_recovery.rs @@ -17,6 +17,7 @@ use std::path::{Path, PathBuf}; use tempfile::TempDir; +use super::deletion_marker::{self, DeletionBreach}; use super::recovery::{ open_recovering, remove_recovery_artifacts, v1_backup_path, CheckpointState, WalState, }; @@ -1533,3 +1534,1029 @@ fn remove_recovery_artifacts_wipes_backup_and_quarantine() { assert!(!f.cp.with_file_name("user_history.lxud.tmp").exists()); assert!(!f.cp.with_file_name("user_history.tmp").exists()); } + +// --------------------------------------------------------------------------- +// T10: the unpersisted-deletion marker (#312). The runtime ledger is +// process-local, so before this the `Io` half went unreported on exactly the +// restart where the entry comes back. +// +// The axis is independent of the checkpoint × WAL matrix above: the marker +// says something about a *previous* session, not about the files just read. +// --------------------------------------------------------------------------- + +/// The tmp `write_atomic` writes the marker through, derived from the same +/// definition the writer calls — a separately-spelled name would stop +/// obstructing anything the moment the convention moved, and the tests below +/// would pass vacuously (`persist::tmp_path`'s own note). +/// The claim the marker path holds. Most tests are about *what is claimed*; +/// the separate `confirmed` half of the observation only matters to the two +/// that assert on it directly. +fn marker_claim(cp: &Path) -> Option { + deletion_marker::read(cp).map(|o| o.breach) +} + +fn marker_tmp(f: &Fx) -> PathBuf { + crate::persist::tmp_path(&deletion_marker::marker_path(&f.cp)) +} + +fn write_marker(f: &Fx, breach: DeletionBreach) { + deletion_marker::merge_write(&f.cp, breach).unwrap(); +} + +/// The on-disk encoding of a breach, obtained through the writer so the test +/// never re-spells the format. +fn encoded_marker(breach: DeletionBreach) -> Vec { + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("scratch.lxud"); + deletion_marker::merge_write(&cp, breach).unwrap(); + fs::read(deletion_marker::marker_path(&cp)).unwrap() +} + +fn marker_bytes(f: &Fx, bytes: &[u8]) { + fs::write(deletion_marker::marker_path(&f.cp), bytes).unwrap(); +} + +fn open_report_of(f: &Fx) -> super::recovery::OpenReport { + let (_, _, report) = open_recovering(&f.cp).unwrap(); + report +} + +fn applied_seq_of(f: &Fx) -> u64 { + let (h, _, _) = open_recovering(&f.cp).unwrap(); + h.applied_seq() +} + +#[test] +fn t10_no_marker_is_clean() { + let f = fx(); + build_v2_state(&f, false); + let report = open_report_of(&f); + assert!(!report.deletion_lost); + assert!(report.is_clean(), "a plain start must stay clean"); +} + +#[test] +fn t10_lost_marker_reports_and_survives_the_open() { + let f = fx(); + build_v2_state(&f, false); + write_marker(&f, DeletionBreach::Lost); + + let report = open_report_of(&f); + assert!(report.deletion_lost, "a lost deletion must be reported"); + assert!(!report.is_clean()); + // Not folded into the data-loss channel: that one means past learning was + // lost, this means data survived a deletion. + assert!(!report.data_loss_suspected()); + // Nothing to schedule: the disk already holds `Lost`, so the projection and + // the file agree. Not because a compaction would wrongly reassure — it + // cannot. `inherited_owed` sits outside the session ledger, so a cover + // cannot settle it, and the projection re-asserts `Lost` after every one. + // A checkpoint written now persists the resurrected entry and says nothing + // about the previous session's undelivered report. + assert!(!report.compaction_recommended); + // Consumption is `ack_open_report`, not the open: the report has to + // outlive the gap between being built and being delivered. + assert!( + deletion_marker::marker_path(&f.cp).exists(), + "open must not consume the only durable trace" + ); +} + +#[test] +fn t10_unflushed_witness_pins_the_boundary() { + // The suppression rule is `witness <= applied_seq`, and the boundary is the + // whole meaning: seq == applied_seq is the frame that *did* replay. A test + // that only probed values far from the boundary would survive turning `<=` + // into `<`. + let f = fx(); + build_v2_state(&f, false); + let applied = applied_seq_of(&f); + assert!(applied > 0, "fixture must reach a non-zero applied_seq"); + + write_marker(&f, DeletionBreach::Unflushed { seq: applied }); + let report = open_report_of(&f); + assert!( + !report.deletion_lost, + "a frame the loaded state already includes was applied — nothing is owed" + ); + // Applied, but out of the page cache: the flush that failed never happened, + // so this is a live durability problem, not a settled one. Retracting here + // would be retract-then-persist. + assert!(report.deletion_pending_checkpoint); + assert!( + deletion_marker::marker_path(&f.cp).exists(), + "only a durable checkpoint may retract; recovery writes none" + ); + + let f2 = fx(); + build_v2_state(&f2, false); + write_marker(&f2, DeletionBreach::Unflushed { seq: applied + 1 }); + let report = open_report_of(&f2); + assert!( + report.deletion_lost, + "a frame beyond the loaded state never took effect — report it" + ); + assert!(!report.deletion_pending_checkpoint); + // Promoted to unconditional: seq numbering is re-based by a WAL reset, so + // leaving a witness behind would let an unrelated later frame settle a + // report that is still owed. + assert_eq!( + marker_claim(&f2.cp), + Some(DeletionBreach::Lost), + "an answered witness must stop depending on a comparison a reset can invalidate" + ); +} + +#[test] +fn t10_a_witness_the_checkpoint_covers_is_retracted_outright() { + // The residue of a crash between a successful `save()` and the unlink that + // follows it: the deletion IS durable, so the marker is stale. Telling the + // two apart takes the checkpoint's own applied_seq — the post-replay value + // answers "the checkpoint covered it" and "replay reached it" with the same + // number, and only the first is durable. Conflating them showed a live + // durability warning for a deletion that was already persisted. + let f = fx(); + build_v2_state(&f, false); + let covered = { + let (h, _, _) = open_recovering(&f.cp).unwrap(); + let settled = h.clone(); + settled.save(&f.cp).unwrap(); + settled.applied_seq() + }; + assert!(covered > 0); + write_marker(&f, DeletionBreach::Unflushed { seq: covered }); + + let report = open_report_of(&f); + assert!(!report.deletion_lost); + assert!( + !report.deletion_pending_checkpoint, + "a checkpoint-covered deletion is not a live durability problem" + ); + assert_eq!( + marker_claim(&f.cp), + None, + "and its marker is stale, not owed" + ); +} + +#[test] +fn t10_a_reported_witness_cannot_be_settled_by_a_rebased_seq() { + // The concrete shape of the promotion above. A WAL quarantine re-bases + // numbering (`adopt_empty` restarts at the checkpoint's applied_seq + 1), + // so without the promotion a later, unrelated frame reaching the same seq + // would satisfy the old witness and silently drop the report. + let f = fx(); + build_v2_state(&f, false); + let applied = applied_seq_of(&f); + write_marker(&f, DeletionBreach::Unflushed { seq: applied + 5 }); + + assert!(open_report_of(&f).deletion_lost); + // Numbering advances past the old witness on wholly unrelated work. The + // checkpoint must still hold entries: an *empty* durable set settles the + // claim outright (there is nothing left for a deletion to have failed + // against), which is a different rule and would mask this one. + let mut h = UserHistory::new(); + h.record_at(&seg(B), T0); + h.advance_applied_seq(applied + 99); + h.save(&f.cp).unwrap(); + + assert!( + open_report_of(&f).deletion_lost, + "the report must not be settled by seqs the witness never referred to" + ); +} + +#[test] +fn t10_an_empty_history_settles_the_marker() { + // A claim that some entry survived a deletion is *false*, not stale, when + // no entry was loaded at all — the same reasoning `clear` uses when it + // covers the ledger from its empty checkpoint. Without this, a wipe that + // could not unlink the marker warned on the next launch about a history + // that provably holds nothing. + let f = fx(); + let empty = UserHistory::new(); + empty.save(&f.cp).unwrap(); + write_marker(&f, DeletionBreach::Lost); + + let report = open_report_of(&f); + assert!( + !report.deletion_lost, + "there is no entry for the deletion to have failed against" + ); + assert_eq!( + marker_claim(&f.cp), + None, + "and the record goes with the claim" + ); +} + +#[test] +fn t10_a_replayed_tombstone_does_not_settle_a_claim_the_checkpoint_outlives() { + // The refutation of `Lost` takes both halves — an empty durable set AND an + // empty loaded state — and this is the case that needs the first one. A + // tombstone replaying out of the WAL empties memory while the checkpoint + // still holds the entry, so a rule keyed on the loaded state alone would + // retract here; the entry is still on disk, and a power loss before the + // next checkpoint brings it back with nothing ever reported. + // + // Malformed bytes on purpose: `decode` maps every unreadable shape to + // `Lost` (fail-safe), so this claim may well have been a witness about + // durability rather than a presence claim at all. Nothing here can tell, + // which is precisely why emptiness must not be allowed to refute it. + let f = fx(); + let mut h = UserHistory::new(); + let mut wal = HistoryWal::new(&f.cp); + for pair in [A, B] { + let seq = wal.append(&seg(pair), T0).unwrap(); + h.record_at(&seg(pair), T0); + h.advance_applied_seq(seq); + } + h.save(&f.cp).unwrap(); + wal.truncate_wal().unwrap(); + for pair in [A, B] { + wal.append_record(&WalRecord::Tombstone { + segments: seg(pair), + timestamp: T0 + 1, + }) + .unwrap(); + } + + fs::write(deletion_marker::marker_path(&f.cp), b"not a marker").unwrap(); + + let (loaded, _, report) = open_recovering(&f.cp).unwrap(); + // The premise: memory is empty, and the checkpoint on disk is not. + assert_contents(&loaded, &[], &[A, B]); + assert!( + !UserHistory::open(&f.cp).unwrap().is_empty(), + "fixture must keep the entries in the durable checkpoint" + ); + assert!( + report.deletion_lost, + "an entry the checkpoint still holds can resurrect — the claim stands" + ); + assert_eq!( + marker_claim(&f.cp), + Some(DeletionBreach::Lost), + "and the record stands with it, for the next start" + ); +} + +#[test] +fn t10_a_retraction_that_cannot_unlink_schedules_a_compaction() { + // The verdict is permanent — nothing survived the deletion — but the only + // durable place to record it is the file that would not unlink, so the + // debt has to reach the runtime some other way. Without this, the next + // compaction is a thousand frames off; a restart before then reloads the + // same marker against a history replay has made non-empty and reports a + // loss this startup already refuted. + let f = fx(); + let empty = UserHistory::new(); + empty.save(&f.cp).unwrap(); + // A directory with something in it: `remove` clears an *empty* one as a + // placeholder but never walks a populated tree on the startup thread, so + // this is the reachable shape of a retraction that cannot complete. + let path = deletion_marker::marker_path(&f.cp); + fs::create_dir(&path).unwrap(); + fs::write(path.join("left-by-something-else"), b"x").unwrap(); + + let report = open_report_of(&f); + assert!( + !report.deletion_lost, + "the claim is still refuted — the unlink failing does not revive it" + ); + assert!( + report.compaction_recommended, + "and the retry has to be scheduled, since the file still says otherwise" + ); + assert!(path.exists(), "fixture must actually block the removal"); +} + +#[test] +fn t10_malformed_markers_all_report() { + // Fail-safe by construction: only NotFound is clean. Every malformed + // shape resolves to the strongest claim, which is why the format carries + // no CRC — corruption can only push it toward reporting. + // + // The 12-byte case is the one that matters structurally: a parser that + // checked the magic but not the length would slice bytes[8..16] on it and + // panic out through `#[uniffi::constructor]`. + let mut good = Vec::new(); + good.extend_from_slice(b"LXDM"); + good.push(1); + good.push(1); + good.extend_from_slice(&[0, 0]); + good.extend_from_slice(&7u64.to_le_bytes()); + + for (name, bytes) in [ + ("empty", vec![]), + ("three bytes", vec![b'L', b'X', b'D']), + ("twelve bytes", good[..12].to_vec()), + ("bad magic", { + let mut b = good.clone(); + b[0] = b'X'; + b + }), + ("unknown version", { + let mut b = good.clone(); + b[4] = 9; + b + }), + // The one shape that would resolve toward *suppression*: a + // well-formed 16-byte prefix followed by anything else. Reading + // exactly LEN would decode it as a valid witness, and this witness is + // one the fixture's replay satisfies — so the "malformed always + // reports" rule, and the absent CRC that rests on it, would both be + // false. Seq 1 is load-bearing here: a witness beyond `applied_seq` + // reports either way and would prove nothing. + // Bytes the writer cannot emit. Each of these was found separately as + // its own bug before `decode` became a round-trip against `encode`; + // the last two nobody had named at all. + ("unknown flags", { + let mut b = good.clone(); + b[5] = 0x03; + b[8..16].copy_from_slice(&1u64.to_le_bytes()); + b + }), + ("non-zero reserved", { + let mut b = good.clone(); + b[6] = 1; + b[8..16].copy_from_slice(&1u64.to_le_bytes()); + b + }), + ("seq without the witness flag", { + let mut b = good.clone(); + b[5] = 0; + b + }), + // The shape that resolved to *silence* rather than to a suppressible + // witness: seq 0 is below every applied_seq, so recovery read it as + // "the checkpoint already covers this", removed the marker, and + // reported nothing. The writer cannot produce it — WAL numbering + // starts at 1 — so it is malformed like the rest. + ("witness of zero", { + let mut b = good.clone(); + b[8..16].copy_from_slice(&0u64.to_le_bytes()); + b + }), + ("valid prefix, extra bytes", { + let mut b = good.clone(); + b[8..16].copy_from_slice(&1u64.to_le_bytes()); + b.extend_from_slice(b"trailing"); + b + }), + ] { + let f = fx(); + build_v2_state(&f, false); + marker_bytes(&f, &bytes); + assert!( + open_report_of(&f).deletion_lost, + "{name}: an unreadable marker must report, not suppress" + ); + assert!( + deletion_marker::marker_path(&f.cp).exists(), + "{name}: and it must not be silently retracted as checkpoint-covered" + ); + } +} + +#[test] +fn t10_unreadable_marker_reports_without_failing_the_open() { + // A directory at the marker path makes the read fail. It must not + // propagate: `open_recovering`'s only Err is an environmental failure, and + // Swift turns that into "learning disabled" — stopping learning outright + // because a 16-byte sidecar is unreadable is the worst possible default. + let f = fx(); + build_v2_state(&f, false); + fs::create_dir(deletion_marker::marker_path(&f.cp)).unwrap(); + + assert!( + open_report_of(&f).deletion_lost, + "unreadable resolves to reporting" + ); +} + +#[test] +fn t10_removal_reports_whether_the_path_is_actually_clear() { + // Every retraction clears the marker by unlinking, so a removal that + // fails must say so — otherwise the caller drops the status row while the + // record stands, and the warning returns on the next launch with the + // retry gone. + // + // A directory at the path is removed only when empty. A non-empty one is + // not the engine's to delete (a restore put it there, and walking it would + // block the thread the menu runs on), so the honest outcome is `false`. + let f = fx(); + build_v2_state(&f, false); + let path = deletion_marker::marker_path(&f.cp); + + assert!(deletion_marker::remove(&f.cp), "absent counts as clear"); + + fs::create_dir(&path).unwrap(); + assert!( + deletion_marker::remove(&f.cp), + "an empty placeholder is ours to clear" + ); + assert!(!path.exists()); + + fs::create_dir(&path).unwrap(); + fs::write(path.join("restored"), b"someone else's bytes").unwrap(); + assert!( + !deletion_marker::remove(&f.cp), + "a non-empty directory is not ours to delete, and saying so is the point" + ); + assert!( + path.join("restored").exists(), + "and its contents must survive" + ); + // The report is then still owed, which the reader agrees with. + assert_eq!(marker_claim(&f.cp), Some(DeletionBreach::Lost)); +} + +#[test] +fn t10_removal_covers_the_orphan_tmp_too() { + // The logical marker is the pair. `read` merges the atomic write's orphan + // so a crash between its flush and the rename can only strengthen a claim + // — which means removing only the canonical file leaves the claim alive. + // A `Lost` orphan would then re-report on every launch with nothing able + // to clear it: the unclearable latch, rebuilt out of the fix for hiding. + let f = fx(); + build_v2_state(&f, false); + let tmp = + f.cp.with_file_name("user_history.lxud.deletion-pending.tmp"); + write_marker(&f, DeletionBreach::Lost); + fs::write(&tmp, encoded_marker(DeletionBreach::Lost)).unwrap(); + + assert!(deletion_marker::remove(&f.cp)); + assert!(!tmp.exists(), "the orphan is part of what had to go"); + assert_eq!(marker_claim(&f.cp), None); + assert!(!open_report_of(&f).deletion_lost); +} + +#[test] +fn t10_removal_reports_false_when_only_the_orphan_resists() { + // And the outcome is honest about either half: a claim the caller cannot + // clear must not be reported as cleared just because the other file went. + let f = fx(); + build_v2_state(&f, false); + let tmp = + f.cp.with_file_name("user_history.lxud.deletion-pending.tmp"); + write_marker(&f, DeletionBreach::Lost); + fs::create_dir(&tmp).unwrap(); + fs::write(tmp.join("restored"), b"not ours").unwrap(); + + assert!( + !deletion_marker::remove(&f.cp), + "an orphan that resists leaves the record standing" + ); + assert!( + !deletion_marker::marker_path(&f.cp).exists(), + "the half that could go, went" + ); + assert_eq!( + marker_claim(&f.cp), + Some(DeletionBreach::Lost), + "and the surviving orphan still carries the claim" + ); +} + +#[test] +fn t10_a_resisting_marker_does_not_spare_the_orphan() { + // The other order, which is the one that distinguishes `&` from `&&`: if + // the canonical file refuses first, a short-circuit would never attempt + // the orphan and would leave a claim alive that could have been cleared. + let f = fx(); + build_v2_state(&f, false); + let path = deletion_marker::marker_path(&f.cp); + let tmp = + f.cp.with_file_name("user_history.lxud.deletion-pending.tmp"); + fs::create_dir(&path).unwrap(); + fs::write(path.join("restored"), b"not ours").unwrap(); + fs::write(&tmp, encoded_marker(DeletionBreach::Lost)).unwrap(); + + assert!(!deletion_marker::remove(&f.cp), "the marker resisted"); + assert!( + !tmp.exists(), + "but the orphan was still attempted — a refusal must not spare the other half" + ); +} + +#[test] +fn t10_a_fifo_at_the_marker_path_does_not_block_the_open() { + // A read-only `File::open` on a FIFO blocks until a writer appears, and + // this runs synchronously inside the IME's startup. Left unguarded, a + // sidecar nobody can read would stop the input method from ever becoming + // available — so the open itself carries `O_NOFOLLOW | O_NONBLOCK` and the + // descriptor it returns is what gets `fstat`-checked. Deliberately not a + // file-type check *before* the open: that is two resolutions of one name, + // and the paragraph below records why the race between them was the whole + // point of the fix. + let f = fx(); + build_v2_state(&f, false); + let path = deletion_marker::marker_path(&f.cp); + let status = std::process::Command::new("mkfifo") + .arg(&path) + .status() + .expect("mkfifo"); + assert!(status.success()); + + // Note the failure mode: without the guard this does not fail, it HANGS — + // which is exactly the defect (a startup that never completes). A mutation + // check on the guard therefore shows up as a timeout, not a red test. + // + // What this test canNOT see is *how* the guard is built. It passed equally + // against the earlier `symlink_metadata`-then-open form, whose two + // resolutions of the same name let a FIFO substituted in between reach the + // blocking open anyway. Nothing deterministic distinguishes the two — the + // window is a couple of syscalls wide — so the property is carried by + // construction instead: `read_at` goes through `persist::open_regular`, + // which resolves the name once and validates the descriptor it got, and + // there is no longer a path-based check for a substitution to race. + assert_eq!(marker_claim(&f.cp), Some(DeletionBreach::Lost)); + assert!(open_report_of(&f).deletion_lost); +} + +#[test] +fn t10_a_symlink_at_the_marker_tmp_is_not_written_through() { + // The write half of the same hazard the reader guards against. `rename` + // protects the marker's own path — it replaces a symlink rather than + // following it — but the tmp `write_atomic` writes through is opened by + // name, and `File::create` resolves a symlink there and truncates whatever + // it points at. The victim is a file this crate does not own, and the + // marker's writer runs on the key-processing thread. + let f = fx(); + let victim = f.cp.parent().unwrap().join("someone-elses-file"); + fs::write(&victim, b"not ours to truncate").unwrap(); + std::os::unix::fs::symlink(&victim, marker_tmp(&f)).unwrap(); + + assert!( + deletion_marker::merge_write(&f.cp, DeletionBreach::Lost).is_err(), + "a diverted write must fail rather than land somewhere else" + ); + assert_eq!( + fs::read(&victim).unwrap(), + b"not ours to truncate", + "and the symlink's target must be untouched" + ); +} + +#[test] +fn t10_a_fifo_at_the_marker_tmp_does_not_block_the_writer() { + // Same shape as the reader's FIFO guard, on the writer: opening a FIFO + // write-only waits for a reader, and this call sits on the key-processing + // thread under the wal mutex — every keystroke behind it. As with the + // reader's test the unguarded failure is a HANG, not a red assertion, so a + // mutation check on `create_regular` shows up as a timeout. + let f = fx(); + let tmp = marker_tmp(&f); + let status = std::process::Command::new("mkfifo") + .arg(&tmp) + .status() + .expect("mkfifo"); + assert!(status.success()); + + assert!( + deletion_marker::merge_write(&f.cp, DeletionBreach::Lost).is_err(), + "the writer must refuse the FIFO instead of waiting on a reader" + ); +} + +#[test] +fn t10_a_promotion_that_failed_schedules_its_own_retry() { + // The promotion is best-effort, and when it fails the disk keeps the + // *suppressible* `Unflushed{seq}` while memory holds `Lost`. A healthy + // session raises nothing, so nothing re-projects until a threshold + // compaction a thousand frames off — and ordinary commits advance the WAL + // past the witness meanwhile, so a restart before then reads the witness + // as replayed and suppresses a report that is still owed. + // + // Scheduling the compaction is what makes the promotion actually happen. + // This reverses a settled note that said a compaction here would "tell the + // user everything is fine": it cannot cover the ledger (nothing raised it, + // so the cover early-returns) and it cannot touch the row (driven by the + // latched failure and the owed predicate) — all it does is project, which + // with the claim standing writes `Lost`. + let f = fx(); + build_v2_state(&f, false); + let applied = applied_seq_of(&f); + write_marker(&f, DeletionBreach::Unflushed { seq: applied + 1 }); + // Block the *write* without disturbing the read: a read-only parent stops + // `create_regular` from making the tmp (EACCES) while the canonical marker + // still reads fine. Planting an obstacle at the tmp path itself does not + // work — `read` merges the orphan tmp, and an unreadable one resolves to + // `Lost`, which changes the very branch under test. + use std::os::unix::fs::PermissionsExt; + let parent = f.cp.parent().unwrap().to_path_buf(); + let original = fs::metadata(&parent).unwrap().permissions(); + fs::set_permissions(&parent, fs::Permissions::from_mode(0o500)).unwrap(); + + let report = open_report_of(&f); + fs::set_permissions(&parent, original).unwrap(); + assert!(report.deletion_lost, "the report is owed"); + assert_eq!( + marker_claim(&f.cp), + Some(DeletionBreach::Unflushed { seq: applied + 1 }), + "fixture must actually block the promotion, leaving the witness" + ); + assert!( + report.compaction_recommended, + "a promotion that did not land has to be retried, or a later replay settles the witness" + ); + + // And the converse: a promotion that landed owes the disk nothing. + let g = fx(); + build_v2_state(&g, false); + write_marker(&g, DeletionBreach::Unflushed { seq: applied + 1 }); + let report = open_report_of(&g); + assert!(report.deletion_lost); + assert_eq!(marker_claim(&g.cp), Some(DeletionBreach::Lost)); + assert!( + !report.compaction_recommended, + "nothing is owed once the disk holds the unconditional claim" + ); +} + +#[test] +fn t10_an_unreadable_marker_is_never_recorded_as_an_observation() { + // `read` resolves anything unreadable to `Lost` by the fail-safe rule, but + // that is a *claim*, not knowledge of what the bytes say. Recording it as + // an observation seeds the runtime's `flushed` with `Lost`, every later + // reconcile then finds the disk already in agreement and skips, and a live + // `Unflushed` witness sits there with nothing left to promote it. + let f = fx(); + build_v2_state(&f, false); + write_marker(&f, DeletionBreach::Unflushed { seq: 999 }); + // Unreadable, not absent: the file is a regular file whose bytes cannot be + // read, which is what makes the conservative `Lost` a guess. + use std::os::unix::fs::PermissionsExt; + let path = deletion_marker::marker_path(&f.cp); + let original = fs::metadata(&path).unwrap().permissions(); + fs::set_permissions(&path, fs::Permissions::from_mode(0o000)).unwrap(); + + let observed = deletion_marker::read(&f.cp).expect("the file is there"); + assert_eq!( + observed.breach, + DeletionBreach::Lost, + "unreadable still claims the strongest thing" + ); + assert!( + !observed.confirmed, + "but nothing was observed, so it must not be recorded as one" + ); + + let report = open_report_of(&f); + fs::set_permissions(&path, original).unwrap(); + assert!(report.deletion_lost, "and it is reported"); + assert_eq!( + report.marker_on_disk, + deletion_marker::MarkerState::Unknown, + "unknown, not absent: the runtime must neither be told the disk holds \ + Lost when nobody read it, nor that the path is clear when a file is \ + sitting there — the second is what let a failed unlink look settled" + ); +} + +#[test] +fn t10_a_ceiling_witness_refuses_rather_than_wrapping() { + // `applied_seq` comes from a *file*, so a corrupt checkpoint can name + // anything a `u64` can hold. Adding one panics in debug, across the UniFFI + // constructor, and wraps to zero in release: the next tombstone would be + // written with seq 0, recovery would reject it as non-monotonic, and the + // deletion would resurrect. So adoption saturates and the refusal lives at + // the point a number is *issued* — deliberately not a `frozen` flag, which + // a compaction is entitled to clear by rewriting the file, and rewriting a + // file creates no numbers. + let f = fx(); + let mut h = UserHistory::new(); + h.record_at(&seg(A), T0); + // From the *checkpoint*, which is the remaining file-derived input now + // that the sequence floor is gone: a corrupt one can name any value. + h.advance_applied_seq(u64::MAX); + h.save(&f.cp).unwrap(); + + let (_, mut wal, _) = open_recovering(&f.cp).unwrap(); + assert_eq!( + wal.next_seq_for_tests(), + u64::MAX, + "saturated, not wrapped — and this value is the exhausted state, not a \ + spendable last number" + ); + + // The ceiling value is refused rather than spent — assigning it would + // leave no representable successor, and one number out of 2^64 is a + // cheaper price than a second "exhausted but for one" state. + let err = wal + .append_record(&WalRecord::Committed { + segments: seg(B), + timestamp: T0, + }) + .expect_err("the append must refuse rather than wrap"); + assert!(matches!(err, super::wal::AppendError::Io(_))); + + // The refusal is at the point the number is issued, so nothing can clear + // it. Representing exhaustion as a *freeze* failed exactly here: a + // compaction heals a freeze by rewriting the file, and rewriting a file + // creates no sequence numbers, so the next append wrapped to zero. + wal.truncate_wal().ok(); + assert!( + wal.append_record(&WalRecord::Committed { + segments: seg(D), + timestamp: T0, + }) + .is_err(), + "a heal must not resurrect a number that does not exist" + ); +} + +#[test] +fn t10_a_flushed_orphan_counts_as_a_landed_claim() { + // The logical marker is the canonical file *and* its orphan tmp — `read` + // merges them, `remove` clears both — so a rename that fails after the tmp + // was flushed has still landed the claim. Reporting failure there made it + // look unlanded forever: appends froze on every commit, each compaction + // thawed them, and the next keystroke froze again, against a record on + // disk that already said what was wanted. + let f = fx(); + build_v2_state(&f, false); + // A non-empty directory at the canonical name: `create_regular` still + // makes the tmp, the flush succeeds, and only the rename fails. + let path = deletion_marker::marker_path(&f.cp); + fs::create_dir(&path).unwrap(); + fs::write(path.join("left-by-something-else"), b"x").unwrap(); + + let landed = deletion_marker::merge_write(&f.cp, DeletionBreach::Lost) + .expect("a flushed orphan is a landed claim, not a failure"); + assert_eq!( + landed, + deletion_marker::MarkerState::Holds(DeletionBreach::Lost), + "and it is a *named* claim: this branch is only reached with the \ + parent-dir fsync already confirmed, so `read` will find the orphan" + ); + assert_eq!( + marker_claim(&f.cp), + Some(DeletionBreach::Lost), + "and the next read finds it, which is why it counts" + ); + assert!( + open_report_of(&f).deletion_lost, + "so the report survives the restart it exists for" + ); +} + +#[test] +fn t10_a_failed_migration_suppresses_the_marker_compaction() { + // A compaction is not the migration: it writes a v2 checkpoint over the v1 + // file with none of the commit's steps — no `.v1.bak`, no `Migrated`. So + // scheduling one while the commit is failing destroys the v1 bytes on + // exactly the path where they are the only copy. The marker feeders are + // promptness alone (ordinary commits reconcile regardless), which makes + // suppressing them here free. + let f = fx(); + let mut h = UserHistory::new(); + h.record_at(&seg(A), T0); + fs::write(&f.cp, v1_checkpoint_bytes(&h)).unwrap(); + // The witness **with its WAL frame**, not just the marker: replaying that + // tombstone sets `replayed_deletion`, which is a feeder of its own. The + // first version of this test planted the marker alone and so exercised + // only the feeders the fix had gated — it passed while the hazard was + // still reachable through the other one. The veto has to sit on the + // recommendation, and this fixture is what proves it does. + let seq = { + let mut wal = HistoryWal::new(&f.cp); + wal.append_record(&WalRecord::Tombstone { + segments: seg(A), + timestamp: T0 + 1, + }) + .unwrap() + }; + write_marker(&f, DeletionBreach::Unflushed { seq }); + use std::os::unix::fs::PermissionsExt; + let dir = f.cp.parent().unwrap().to_path_buf(); + fs::set_permissions(&dir, fs::Permissions::from_mode(0o555)).unwrap(); + // Root / CAP_DAC_OVERRIDE (containerized CI) ignores directory + // permissions, so the failure cannot be injected — skip rather than + // assert one that will not happen. + if fs::write(dir.join("probe"), b"x").is_ok() { + fs::set_permissions(&dir, fs::Permissions::from_mode(0o755)).unwrap(); + eprintln!("skipping: directory permissions are not enforced here"); + return; + } + let result = open_recovering(&f.cp); + fs::set_permissions(&dir, fs::Permissions::from_mode(0o755)).unwrap(); + let report = result.unwrap().2; + assert!(report.migration_failed, "fixture must block the commit"); + assert!( + report.replayed_deletion, + "the replayed tombstone must arm the feeder the veto has to cover" + ); + assert!( + !report.compaction_recommended, + "no compaction may run over a retained v1 checkpoint" + ); +} + +#[test] +fn t10_a_migration_that_persists_the_deletion_settles_the_marker() { + // The migration commit writes a durable v2 checkpoint serialized from the + // replayed state — which, when replay applied a tombstone, is a checkpoint + // that *contains* the deletion. That is coverage, so the marker is settled + // and nothing is handed to the runtime ledger. + // + // Evaluating the marker before the commit answered against the v1 file, + // which predates the deletion: the engine then seeded a live durability + // warning for an already-persisted deletion and left the marker for an + // asynchronous compaction that might never run. + // + // The fixture is the state the migration path documents as reachable: a + // previous startup's commit failed, so later commits appended v2 frames + // beside the still-v1 checkpoint. + let f = fx(); + let mut h = UserHistory::new(); + h.record_at(&seg(A), T0); + fs::write(&f.cp, v1_checkpoint_bytes(&h)).unwrap(); + let seq = { + let mut wal = HistoryWal::new(&f.cp); + wal.append_record(&WalRecord::Tombstone { + segments: seg(A), + timestamp: T0 + 1, + }) + .unwrap() + }; + write_marker(&f, DeletionBreach::Unflushed { seq }); + + let report = open_report_of(&f); + assert!(report.migrated_from_v1, "fixture must actually migrate"); + assert!( + !report.deletion_lost, + "the deletion took, so nothing is owed" + ); + assert!( + !report.deletion_pending_checkpoint, + "the migration's own checkpoint is the durable coverage" + ); + assert_eq!( + marker_claim(&f.cp), + None, + "and it settles the marker rather than leaving it to a compaction" + ); + + // The deletion really is gone from the checkpoint that was just written. + let (reloaded, _, _) = open_recovering(&f.cp).unwrap(); + assert_contents(&reloaded, &[], &[A]); +} + +#[test] +fn t10_a_migration_does_not_settle_an_unconditional_claim() { + // A `Lost` claim is unconditional, so the checkpoint the migration writes + // — which does contain the resurrected entry, replay having applied no + // tombstone here — settles nothing. It has to come out the other side. + // + // The sibling test above is the complement: when replay *did* apply the + // tombstone, that same commit is genuine coverage. What separates them is + // the claim, not the migration, which is why an earlier version of this + // test asserting "a migration checkpoint is never coverage" stated a rule + // its own neighbour disproves. + let f = fx(); + write_v1_state(&f); + write_marker(&f, DeletionBreach::Lost); + + let report = open_report_of(&f); + assert!(report.migrated_from_v1, "fixture must actually migrate"); + assert!( + report.deletion_lost, + "an unconditional claim is owed no matter what was written" + ); + assert!( + deletion_marker::marker_path(&f.cp).exists(), + "and its record stays until someone is told" + ); +} + +#[test] +fn t10_witness_beyond_a_repaired_tail_still_reports() { + // E5/E16: the tombstone's frame was on disk but is gone now — cut away by + // tail repair, or carried off with a quarantined WAL. `applied_seq` never + // reaches the witness, so the deletion never took effect. This is the + // power-loss half of `Unflushed`, and the only reason the witness is a + // seq rather than a bool. + let f = fx(); + build_v2_state(&f, false); + let full = applied_seq_of(&f); + + let f2 = fx(); + build_v2_state(&f2, false); + cut_tail(&f2.wal, 4); + write_marker(&f2, DeletionBreach::Unflushed { seq: full }); + + let report = open_report_of(&f2); + assert_eq!(report.wal_state, WalState::TailRepaired); + assert!( + report.deletion_lost, + "a witness the repaired file can no longer reach must report" + ); +} + +#[test] +fn t10_marker_coexists_with_a_quarantine() { + // Independent facts about one startup: the checkpoint was corrupt *and* a + // previous deletion never persisted. Neither may mask the other. + let f = fx(); + build_v2_state(&f, false); + corrupt_cp(&f.cp); + write_marker(&f, DeletionBreach::Lost); + + let report = open_report_of(&f); + assert_eq!(report.checkpoint_state, CheckpointState::Quarantined); + assert!(report.data_loss_suspected()); + assert!(report.deletion_lost); +} + +#[test] +fn t10_strict_open_ignores_the_marker() { + // The offline path must stay side-effect-free: an audit tool consuming a + // live IME's marker would delete the report the user never saw. + let f = fx(); + build_v2_state(&f, false); + write_marker(&f, DeletionBreach::Lost); + + UserHistory::open(&f.cp).unwrap(); + assert!(deletion_marker::marker_path(&f.cp).exists()); +} + +#[test] +fn t10_marker_is_not_a_quarantine_file() { + // Rotation matches `*` containing ".corrupt-". The marker shares the + // family prefix, so a widened match would start rotating (and eventually + // deleting) the report. + // + // Calls the production predicate, not a local re-spelling of it. The first + // version of this test re-implemented the `.corrupt-` filter in the test + // helper and therefore could not observe the real one changing at all — a + // mutation widening `quarantined_files` left it green. + let f = fx(); + build_v2_state(&f, false); + write_marker(&f, DeletionBreach::Lost); + // Enough real quarantine files to push rotation past its keep limit, so a + // predicate that matched the marker would actually delete it. + for i in 0..5 { + fs::write( + f.cp.with_file_name(format!("user_history.lxud.corrupt-{i}")), + b"x", + ) + .unwrap(); + } + + let listed = crate::persist::quarantined_files(&f.cp); + let marker = deletion_marker::marker_path(&f.cp); + assert!( + !listed.contains(&marker), + "the marker must not be listed as a quarantine artifact: {listed:?}" + ); + crate::persist::rotate_quarantined(&f.cp, 3); + assert!(marker.exists(), "rotation must not reach the marker"); + assert_eq!(marker_claim(&f.cp), Some(DeletionBreach::Lost)); +} + +#[test] +fn t10_decode_accepts_only_what_encode_emits() { + // The property the round-trip form buys, stated directly: whatever the + // writer can produce reads back as itself, and nothing else is trusted. + // Field-by-field validation cannot be checked this way, which is why it + // was wrong four times. + for breach in [ + DeletionBreach::Lost, + DeletionBreach::Unflushed { seq: 1 }, + DeletionBreach::Unflushed { seq: u64::MAX }, + ] { + let f = fx(); + build_v2_state(&f, false); + write_marker(&f, breach); + assert_eq!( + marker_claim(&f.cp), + Some(breach), + "{breach:?} must survive a round trip" + ); + } +} + +#[test] +fn t10_merge_never_weakens_an_outstanding_claim() { + // The rule the whole file depends on: a plain overwrite would let an + // `Unflushed` land on top of a `Lost` and hand the next startup a witness + // it can suppress — losing precisely the report #312 is about. Both + // orders, because the batch loop can produce either. + let lost = DeletionBreach::Lost; + let a = DeletionBreach::Unflushed { seq: 5 }; + let b = DeletionBreach::Unflushed { seq: 9 }; + assert_eq!(lost.merge(a), lost); + assert_eq!(a.merge(lost), lost); + assert_eq!(a.merge(b), b, "two unflushed frames keep the higher seq"); + assert_eq!(b.merge(a), b); + + // And through the file, which is where the ordering actually matters. + let f = fx(); + build_v2_state(&f, false); + write_marker(&f, DeletionBreach::Unflushed { seq: 5 }); + write_marker(&f, DeletionBreach::Lost); + write_marker(&f, DeletionBreach::Unflushed { seq: 9 }); + assert_eq!(marker_claim(&f.cp), Some(DeletionBreach::Lost)); +} diff --git a/engine/crates/lex-core/src/user_history/wal.rs b/engine/crates/lex-core/src/user_history/wal.rs index 843f769..d3e86ea 100644 --- a/engine/crates/lex-core/src/user_history/wal.rs +++ b/engine/crates/lex-core/src/user_history/wal.rs @@ -482,8 +482,25 @@ impl HistoryWal { let payload = bincode::serialize(record).map_err(io::Error::other)?; let payload_len = u32::try_from(payload.len()) .map_err(|_| io::Error::other("WAL entry too large (>4 GiB)"))?; + // Checked here, where the number is *issued*, rather than carried by a + // flag. `u64::MAX` itself is refused rather than spent: assigning it + // would leave no representable successor, and giving up one number out + // of 2^64 costs nothing next to carrying a second exhausted-but-one + // state. Exhaustion was briefly represented with `frozen`, which is + // wrong in kind: `frozen` means "this file is not in appendable v2 + // form" and a compaction legitimately clears it by rewriting the file + // — but rewriting a file does not create more sequence numbers, so the + // heal would clear the freeze and leave the next append to wrap. The + // check belongs where it cannot be cleared. let seq = self.next_seq; - self.next_seq += 1; + self.next_seq = match self.next_seq.checked_add(1) { + Some(next) => next, + None => { + return Err(AppendError::Io(io::Error::other( + "WAL sequence space exhausted", + ))) + } + }; let mut hasher = crc32fast::Hasher::new(); hasher.update(&seq.to_le_bytes()); @@ -611,7 +628,19 @@ impl HistoryWal { self.entry_count = valid_frames; self.wal_bytes = file_bytes.max(WAL_HEADER_LEN as u64); self.last_appended_seq = max_seq; - self.next_seq = max_seq.max(applied_seq) + 1; + // Saturating, not `+ 1`: both inputs come from *files*, and a corrupt + // checkpoint holding `applied_seq == u64::MAX` would make `+ 1` panic + // in debug — across the UniFFI constructor — and wrap to zero in + // release, handing the next tombstone seq 0 for replay to reject as + // non-monotonic. + // + // Saturating is safe *because* the refusal lives at the assignment + // point: `next_seq == u64::MAX` is the exhausted state, and + // `append_record` refuses it rather than wrapping. Representing + // exhaustion as a freeze instead was wrong in kind — a compaction + // clears a freeze by rewriting the file, and rewriting a file creates + // no numbers, so the heal put the wrap straight back. + self.next_seq = max_seq.max(applied_seq).saturating_add(1); // Existing frames may include an unbarriered tail from before the // restart (their power-loss durability is unknown even though they // were readable). Seed the counter so the first new append issues a @@ -628,6 +657,12 @@ impl HistoryWal { self.adopt_scan(0, WAL_HEADER_LEN as u64, 0, applied_seq); } + /// The number the next append would take. Test seam for the floor. + #[cfg(test)] + pub(super) fn next_seq_for_tests(&self) -> u64 { + self.next_seq + } + /// Mark the on-disk file as not being in appendable v2 form (see the /// `frozen` field docs). Used by recovery when a repair fails, and by /// the engine's clear when its truncation fails: appends must not land diff --git a/engine/src/api/resources.rs b/engine/src/api/resources.rs index 04a6a80..f7099c8 100644 --- a/engine/src/api/resources.rs +++ b/engine/src/api/resources.rs @@ -8,6 +8,7 @@ use tracing::warn; use crate::dict::connection::ConnectionMatrix; use crate::dict::{CompositeDictionary, Dictionary, TrieDictionary}; use crate::session::LearningRecord; +use crate::user_history::deletion_marker::{self, DeletionBreach, MarkerState}; use crate::user_history::recovery::{CheckpointState, OpenReport, WalState}; use crate::user_history::wal::{AppendError, HistoryWal, WalRecord}; use crate::user_history::UserHistory; @@ -134,6 +135,33 @@ pub struct LexUserHistory { /// `open_report()` for the degraded-state menu (data loss) and NSLog /// (benign events). report: OpenReport, + /// Whether the v1 checkpoint a failed migration left behind is *still* the + /// only copy of the user's history. + /// + /// Seeded from [`OpenReport::v1_checkpoint_retained`] and then owned here, + /// because the report answers "what did recovery see at open time" while + /// the compaction veto needs "what is on that path now". `clear` writes an + /// empty v2 checkpoint over the very same file, so its commit point + /// supersedes the v1 bytes — deliberately; a wipe is a wipe — and a veto + /// still keyed on the startup observation would go on refusing for the rest + /// of the process's life, including the heal `clear` posts for its own + /// failed physical steps. That leaves a frozen WAL and a half-finished wipe + /// with no retry short of a restart, on a path that deliberately does not + /// restart. + /// + /// Set once at open, cleared once by the only other writer of the + /// checkpoint path. Two readers, one cell: [`Self::run_compact_impl`] + /// refuses (load-bearing), and the schedulers skip creating a worker that + /// could only reach that refusal (promptness/cost). They cannot disagree, + /// which is the whole reason the fact is not re-derived at each site. + v1_checkpoint_retained: AtomicBool, + /// Compaction workers this resource has created. Test-only, and here + /// because "no OS thread was created" has no other observable: a worker + /// that spawns and then refuses leaves exactly the state one that never + /// spawned leaves, so the cost the veto's scheduler skip exists to avoid is + /// invisible to every assertion about history, WAL or ledger. + #[cfg(test)] + compact_threads: AtomicU64, /// Runtime durability ledger (#295 / #288): what memory holds that no /// durable checkpoint covers. /// @@ -157,6 +185,83 @@ pub struct LexUserHistory { /// 21 bits per generation: a wrap needs ~2M failed appends inside one /// session, and the values never leave the process. durability_ledger: AtomicU64, + /// What the unpersisted-deletion marker should say right now. + /// + /// The marker is documented — in SPEC and in AGENTS — as *the ledger's + /// on-disk projection*, and this is the value it projects. Four call sites + /// used to maintain the file incrementally, each with its own flags and + /// its own decision about when to touch it, and three consecutive review + /// rounds each found one of them forgetting a case: an acknowledgement + /// that ignored a failed unlink, a wipe that did the same, a raise that + /// skipped the write when the breach it carried was `None`. Incremental + /// maintenance is not a projection. Sites now update this value and project it with + /// [`Self::project_marker`]; [`Self::apply_marker`] is the only thing that + /// touches the file. + /// + /// Two claims, kept apart because they retire on different events: + /// - `session` — what this process has failed to persist. Retired by a + /// durable checkpoint covering it. Lives here. + /// - `inherited` — what a previous session left. The ledger's `covered` + /// has no authority over it: a checkpoint written now persists the + /// *resurrected* entry rather than removing it. Retired by delivery + /// (`ack_open_report`) or by a wipe. Lives in + /// [`Self::inherited_owed`], outside this mutex. + /// + /// Every mutation happens under the wal mutex, so the value and the file + /// cannot be updated out of order. This mutex itself is **never held + /// across I/O** — the status menu reads it through + /// [`Self::deletion_report_owed`], and AGENTS' ledger entry makes + /// "the read must take no lock" a hard blocker precisely because a UI poll + /// must not queue behind history I/O. Holders are instruction-length; the + /// wal mutex is what actually serializes writers. + claims: Mutex, + /// Whether a previous session's report is still owed. + /// + /// A bool rather than a claim, and outside the mutex, for one reason each. + /// It is faithful because recovery reports `deletion_lost` only from the + /// branch that promotes the claim to unconditional, so an inherited claim + /// is *always* `Lost` — this is the claim, not a cached derivation of it. + /// And it is lock-free because the status menu reads it on the main + /// thread: AGENTS' ledger entry makes "the read must take no lock" a hard + /// blocker, and a mutex there can be waited on whenever a history worker + /// is preempted mid-update, however briefly it means to hold it. + inherited_owed: AtomicBool, +} + +/// The two outstanding deletion claims, and what they project onto disk. +#[derive(Clone, Copy, Default)] +struct MarkerClaims { + session: Option, + /// What the marker file holds, as far as this process knows. + /// + /// Three states, because reality has three and an `Option` has two. The + /// missing one is [`MarkerState::Unknown`] — a file is there and nobody + /// has managed to read it — and collapsing that into "absent" is what let + /// a failed unlink of an unreadable marker look settled: the projection + /// found `None == None`, skipped, and the surviving file reported a lost + /// deletion on the next start. A `confirmed` bit bolted onto the *read* + /// was the half-measure; the belief itself has to carry it. + /// + /// Set from a confirmed write, or seeded from what recovery observed. + /// Re-reading the file to decide instead would be wrong: matching bytes + /// prove the content reached the page cache, not that `sync_all` + /// returned — so a failed flush would read back as up to date and never be + /// retried, which is the power-loss window the marker exists to close. + flushed: MarkerState, +} + +impl MarkerClaims { + /// What the marker should hold — the stronger of the two claims, or + /// nothing. `inherited` is passed in because it is kept outside this + /// mutex; see [`LexUserHistory::inherited_owed`]. + fn projected(&self, inherited: bool) -> Option { + match (self.session, inherited) { + (Some(s), true) => Some(s.merge(DeletionBreach::Lost)), + (Some(s), false) => Some(s), + (None, true) => Some(DeletionBreach::Lost), + (None, false) => None, + } + } } /// Layout of `LexUserHistory::durability_ledger`, low bits first: @@ -164,6 +269,12 @@ pub struct LexUserHistory { const GEN_BITS: u32 = 21; const GEN_MASK: u64 = (1 << GEN_BITS) - 1; +/// Fold one record's breach into the batch's claim, so the merge rule that +/// makes `Lost` absorbing is stated once rather than at each arm that raises. +fn note_breach(slot: &mut Option, breach: DeletionBreach) { + *slot = Some(slot.map_or(breach, |prev| prev.merge(breach))); +} + fn covered_of(ledger: u64) -> u64 { ledger & GEN_MASK } @@ -259,6 +370,17 @@ pub struct LexHistoryOpenReport { /// Appends were frozen at open: this session's learning stays in memory /// until a compaction restores appendable form. pub appends_frozen: bool, + /// A deletion from a previous session never reached disk, and the state + /// just loaded may still hold the entry it was meant to remove (#312). + /// + /// Surfaced here rather than through `durability_issues()` on lifetime: + /// the runtime list reports what holds *now* and retracts when a + /// checkpoint covers it, whereas no disk recovery retracts this — the + /// deletion is already lost. What does retire it is a user action, and + /// consuming it is an explicit `ack_open_report()`, so the on-disk record + /// outlives the gap between this report being built and something acting + /// on it. + pub deletion_lost: bool, pub frames_replayed: u64, pub frames_skipped: u64, pub quarantined_paths: Vec, @@ -287,6 +409,7 @@ impl From<&OpenReport> for LexHistoryOpenReport { migrated_from_v1: r.migrated_from_v1, migration_failed: r.migration_failed, appends_frozen: r.appends_frozen, + deletion_lost: r.deletion_lost, frames_replayed: r.frames_replayed, frames_skipped: r.frames_skipped, // Lossy on purpose: display-only, and a non-UTF-8 path must not @@ -353,6 +476,32 @@ impl LexUserHistory { path: cp.with_file_name("commit-log.jsonl"), file: None, }; + // A previous session's deletion that this startup's replay applied is + // not settled: replay read the frame from the page cache, and the + // flush that failed never happened. Seeding the ledger makes it what + // it actually is — a live durability problem — so the first durable + // checkpoint both clears the row and unlinks the marker, instead of + // recovery retracting on evidence it does not have. + // The claim a previous session left, if the report says one is owed. + // Always `Lost`, never re-read: recovery sets `deletion_lost` only in + // the branch that promotes the claim to unconditional, and the one + // case where the file still says `Unflushed` there is a promotion + // whose write failed. Re-reading would carry that suppressible witness + // back into memory and project it again — undoing the promotion the + // report is predicated on. It also costs no syscall on the startup + // thread. + let inherited_owed = report.deletion_lost; + let marker_on_disk = report.marker_on_disk; + let pending_claim = match (report.deletion_pending_checkpoint, marker_on_disk) { + (true, MarkerState::Holds(breach)) => Some(breach), + _ => None, + }; + let ledger = if report.deletion_pending_checkpoint { + pack_ledger(0, 1, 0) + } else { + 0 + }; + let v1_checkpoint_retained = report.v1_checkpoint_retained; let this = Arc::new(Self { inner: Arc::new(RwLock::new(history)), wal: Mutex::new(wal), @@ -360,7 +509,32 @@ impl LexUserHistory { scrub_pending: AtomicBool::new(false), commit_log: Mutex::new(commit_log), report, - durability_ledger: AtomicU64::new(0), + v1_checkpoint_retained: AtomicBool::new(v1_checkpoint_retained), + durability_ledger: AtomicU64::new(ledger), + claims: Mutex::new(MarkerClaims { + // The replayed witness, when there is one. + // `deletion_pending_checkpoint` raises the *ledger* for a + // deletion that replay applied but no checkpoint covers; + // leaving `session` empty made the projection compute a + // desired state of `None` and — once every commit reconciles — + // unlink that witness before any checkpoint had persisted the + // deletion, so a power loss would restore the entry with + // nothing left to report it. The ledger and the claims have to + // agree about what is outstanding. + // + // Confirmed by construction: an unreadable marker resolves to + // `Lost`, which is always outstanding and never reaches the + // branch that sets this flag, so this is a decoded `Unflushed`. + session: pending_claim, + // Observed, not assumed. When recovery promoted the claim + // successfully this equals what the projection wants, so the + // first compaction skips the write entirely; when it did not, + // the mismatch is what drives the re-assertion. + flushed: marker_on_disk, + }), + inherited_owed: AtomicBool::new(inherited_owed), + #[cfg(test)] + compact_threads: AtomicU64::new(0), }); // Startup compaction (§5.1-6): checkpoint recovery results early so // the next startup is clean. This is also the heal path for a @@ -375,10 +549,103 @@ impl LexUserHistory { } /// What recovery found and did at open time (§10). + /// + /// Pure — a getter that mutated the disk would be a trap. Clearing the + /// unpersisted-deletion record is [`Self::ack_open_report`], called once + /// the caller has actually taken the report. fn open_report(&self) -> LexHistoryOpenReport { (&self.report).into() } + /// Acknowledge [`Self::open_report`]: the report has been shown to the + /// user, so the on-disk record behind `deletion_lost` can go. + /// + /// Acknowledges the report as a whole, not one field of it: today + /// `deletion_lost` is the only fact with durable state behind it, and a + /// later deliver-once fact joins here rather than growing a second ack. + /// + /// Separate from `open_report` because it is *delivery* that retires the + /// record, not the read. `open_recovering` deliberately leaves the marker + /// alone: it returns long before anything consumes the report, and the + /// launches this exists for are the ones where a failing disk may take the + /// process down in between. For the same reason the caller must not ack at + /// load — a short-lived IMKit probe launch opens the history, never shows a + /// menu, and would consume the report on the user's behalf. Nor at + /// menu-build time: IMKit constructs the menu without displaying it, so + /// construction is not delivery either. Ack when a person **clicks** the + /// row — that is the only evidence anyone saw it. + /// + /// Two guards, both load-bearing: + /// - **the ledger, not the startup flag.** `report.deletion_lost` is frozen + /// at open, so acting on it alone would delete a marker written by a + /// raise that landed since — the session's own breach, silently dropped. + /// Asking the ledger is the question `cover_unpersisted` asks, of the + /// same authority. + /// - **`try_lock`.** This runs on the main thread when the menu opens, and + /// only when the disk is degraded — exactly when a compaction may hold + /// the wal mutex across `cover_durable_residue` and file I/O. A skipped + /// ack costs one more report next launch, the safe direction; a blocked + /// main thread costs the UI. + /// + /// Whether the row should still be shown afterwards is + /// [`Self::deletion_report_owed`], not this call's outcome — the same + /// predicate answers for a wipe, which retires the report without anyone + /// acknowledging it. Both early exits below leave the marker on disk and + /// the report owed, so a caller that drops the row on a failed + /// acknowledgement takes away the only affordance for retrying it while + /// the warning returns on every launch. + /// + /// Idempotent; safe to call when nothing was reported. + fn ack_open_report(&self) { + if !self.report.deletion_lost { + return; + } + // Under the wal mutex like every other marker operation, so an + // acknowledgement cannot land between a raise and its projection. + let wal = match self.wal.try_lock() { + Ok(w) => w, + Err(std::sync::TryLockError::WouldBlock) => return, + Err(std::sync::TryLockError::Poisoned(p)) => p.into_inner(), + }; + let ledger = self.durability_ledger.load(Ordering::SeqCst); + if raised_deletion_of(ledger) > covered_of(ledger) { + // This session raised a breach of its own after the report was + // built. The marker still has to carry that, so there is nothing + // to deliver away yet. + return; + } + let session_only = *lock_recover(&self.claims); + // `session` is provably `None` here — a session claim implies + // `raised_deletion > covered`, which the guard above returned on — so + // the projection without the inherited claim is an unlink. Requiring + // that explicitly keeps a future change to that guard from turning + // this into "wrote the session's claim, then retired the inherited + // one", which is the shape R3 found. + // + // Confirmed by mutation that no test can detect this clause today — + // the ledger guard makes it always true — so it is stated here rather + // than left to a reader to re-derive. + let desired = session_only.projected(false); + if desired.is_none() && self.apply_marker(&wal, None) { + // The only site that commits its change *after* the disk agrees. + // Delivery is not done until the record is gone: dropping the + // claim on a failed unlink would take the row away while the + // marker stood, and the warning would come back on the next + // launch with the retry. + self.inherited_owed.store(false, Ordering::SeqCst); + } + } + + /// Whether a lost-deletion report from a previous session is still owed. + /// + /// The single authority for whether the status row belongs on screen. Both + /// an acknowledgement the engine could not complete and a wipe that failed + /// before its commit point leave it owed, so one question answers for both + /// call sites. + fn deletion_report_owed(&self) -> bool { + self.inherited_owed.load(Ordering::SeqCst) + } + /// Durability problems that hold right now, most severe first. /// /// Polled by the UI (the status menu re-reads on every open), so it must @@ -407,6 +674,95 @@ impl LexUserHistory { } impl LexUserHistory { + /// Make the marker say `desired`, and report whether the file now agrees. + /// + /// The only place the marker is written or removed. Sites decide what the + /// claims are; this projects them. On failure the caller's claim stays in + /// memory, so the next site to project re-asserts it — which is what makes + /// a transient write failure recover without anyone remembering to retry. + fn apply_marker( + &self, + wal: &MutexGuard<'_, HistoryWal>, + desired: Option, + ) -> bool { + if lock_recover(&self.claims).flushed.satisfies(desired) { + // The disk already says what it should. Symmetric — `None == None` + // skips too — which is sound only because `flushed` is seeded from + // what recovery observed rather than assumed: an asymmetric skip + // was how a healthy compaction still paid two `unlink` syscalls + // inside the wal critical section for a file that was never there. + // + // The `Some` side matters more: the claim is re-projected on every + // raise while it is outstanding, and each write is an F_FULLFSYNC + // on the key-processing thread. + // + // A cost, not a behaviour: removing this skip is invisible to the + // tests by construction, which is why the *recording* of what the + // disk holds is what they pin instead. + return true; + } + match desired { + Some(claim) => match deletion_marker::merge_write(wal.checkpoint_path(), claim) { + Ok(belief) => { + // The writer's belief, not one synthesized from "it + // returned Ok". Two facts are folded in and neither is + // recoverable here. *What* landed, because the write merges + // — a request of `Unflushed` over a surviving `Lost` leaves + // `Lost` on disk, and recording the request would be a + // belief the disk never held, so the next reconcile would + // find it satisfied and skip. And *whether* the name for it + // is durable, because a rename whose parent-dir fsync + // failed can roll this path back to what it held before — + // for a promotion, the weaker claim the promotion existed + // to replace. That comes back as `Unknown`, which no + // desired state satisfies, so the projection keeps + // re-asserting and the freeze below keeps appends off the + // WAL until the claim is durably named. + lock_recover(&self.claims).flushed = belief; + matches!(belief, MarkerState::Holds(_)) + } + Err(e) => { + // The belief is now *unknown*, not simply stale. A write + // that failed may still have left a truncated orphan — a + // short write on ENOSPC — and the previous value does not + // describe that. Keeping it let a later `Absent` satisfy a + // desired `None`, skip the removal, and leave the malformed + // orphan for the next startup to decode as `Lost` and + // report against a deletion the checkpoint had persisted. + warn!("failed to record the unpersisted deletion for the next start: {e}"); + lock_recover(&self.claims).flushed = MarkerState::Unknown; + false + } + }, + None => { + let cleared = deletion_marker::remove(wal.checkpoint_path()); + lock_recover(&self.claims).flushed = if cleared { + MarkerState::Absent + } else { + // Same rule: a removal that did not complete leaves the + // path in a state this process has not observed, and only + // `Unknown` says so. Holding the old value would let a + // later projection decide it matched. + MarkerState::Unknown + }; + cleared + } + } + } + + /// Project the current claims onto disk. Sites that settle a claim + /// unconditionally — a cover, a wipe — use this; only the acknowledgement + /// needs to know whether the disk agreed. + /// Returns whether the disk now says what the projection wants. + fn project_marker(&self, wal: &MutexGuard<'_, HistoryWal>) -> bool { + // The guard is released before the I/O — see the field docs: every + // holder of `claims` must be instruction-length, because the status + // menu reads it. + let desired = + lock_recover(&self.claims).projected(self.inherited_owed.load(Ordering::SeqCst)); + self.apply_marker(wal, desired) + } + /// Record what this batch failed to make durable (#295 / #288). /// /// `memory_only` — at least one effect was applied with no WAL frame, so @@ -425,17 +781,44 @@ impl LexUserHistory { /// - under the wal mutex, so it cannot land inside `clear_impl`'s /// read-then-cover window. A raise slipping in there would outlive a /// wipe that made it vacuously true, leaving a privacy warning on a - /// history that is provably empty. The guard is taken as an unused - /// parameter so that half is checked rather than merely documented. + /// history that is provably empty. The guard is a parameter so that half + /// is checked by the compiler rather than merely documented — it is also + /// what names the checkpoint the marker sits beside, so the requirement + /// and the use are the same object. fn raise_unpersisted( &self, - _wal: &MutexGuard<'_, HistoryWal>, + wal: &MutexGuard<'_, HistoryWal>, memory_only: bool, - deletion_breach: bool, + deletion_breach: Option, ) { - if !memory_only && !deletion_breach { + if !memory_only && deletion_breach.is_none() { return; } + // On disk before the ledger, and before the synchronous checkpoint + // fallback the caller runs next (§5.4) — the same write-ahead + // discipline the WAL itself follows. Writing after the fallback would + // open a crash window whose only outcome is the silent one; writing + // first can only over-report, since a fallback that succeeds unlinks + // the marker through the cover below. + // Projected on every raise, not only when this one carries a breach. + // A write that failed leaves the claim in memory, and the next raise + // re-asserts it — including a memory-only one, which is the shape that + // used to skip the retry entirely and let a recovered disk go + // unrecorded. + { + let mut claims = lock_recover(&self.claims); + if let Some(breach) = deletion_breach { + // Through `note_breach`, which exists so the rule that makes + // `Lost` absorbing is written once rather than at each arm + // that raises. + note_breach(&mut claims.session, breach); + } + let desired = claims.projected(self.inherited_owed.load(Ordering::SeqCst)); + drop(claims); + if desired.is_some() { + self.apply_marker(wal, desired); + } + } let mut current = self.durability_ledger.load(Ordering::SeqCst); loop { let mem = raised_memory_only_of(current); @@ -452,7 +835,7 @@ impl LexUserHistory { let next = (mem.max(del).max(covered_of(current)) + 1).min(GEN_MASK); let updated = pack_ledger( if memory_only { next } else { mem }, - if deletion_breach { next } else { del }, + if deletion_breach.is_some() { next } else { del }, covered_of(current), ); match self.durability_ledger.compare_exchange_weak( @@ -505,18 +888,53 @@ impl LexUserHistory { /// A CAS loop rather than a store: it must not clobber a raise that /// landed since the load (the retry picks up the new generations), and it /// must never walk `covered` backwards. - fn cover_unpersisted(&self, generation: u64) { + /// + /// The on-disk marker is re-projected here, in the same call and under the + /// same wal guard as the CAS that settles the ledger — not as a follow-up + /// statement in the caller. Re-projected rather than unlinked: an + /// inherited claim nobody has delivered yet still has to be on disk, and + /// this checkpoint is no authority over it. Between a successful CAS and a + /// separate unlink there is a window of a few instructions in which a new + /// raise can write a marker that the unlink then destroys, dropping the + /// report for a deletion that is still outstanding. That window is not + /// something a deterministic test can pin (#317 proved twice that tests + /// over such windows pass under mutation), so it is removed by + /// construction: the guard witness makes "cover without the wal mutex" not + /// compile, and every raise takes the same mutex. + /// + /// **Unconditionally, including on the early-return path**, and that is + /// not a cost the steady state pays: a projection whose write or unlink + /// failed has nothing else to revisit it — the ledger is covered, so later + /// covers return early, and a healthy session raises nothing — so the + /// durable checkpoint is the retry. `apply_marker` skips when the disk + /// already agrees, which on a healthy history is every time, so no syscall + /// is issued inside the critical section the key thread waits on. Having + /// the caller do it instead left "every cover is followed by a projection + /// under the same guard" as an unenforced convention. + fn cover_unpersisted(&self, wal: &MutexGuard<'_, HistoryWal>, generation: u64) { + self.settle_ledger(generation); + // Paired here, so it cannot be forgotten at a call site: the two + // together are what "a durable checkpoint reconciles the record" means. + self.project_marker(wal); + } + + /// Move `covered` to `generation`, settling the session's claim if this + /// checkpoint is what settled it. Split out only so the projection above + /// runs on every path, including the already-covered early return — the + /// retry has to happen whether or not this particular call moved anything. + fn settle_ledger(&self, generation: u64) { let mut current = self.durability_ledger.load(Ordering::SeqCst); loop { if covered_of(current) >= generation { return; } + let covered = generation.min(GEN_MASK - 1); let updated = pack_ledger( raised_memory_only_of(current), raised_deletion_of(current), // One below the raise ceiling, so a saturated generation // stays outstanding rather than being covered by accident. - generation.min(GEN_MASK - 1), + covered, ); match self.durability_ledger.compare_exchange_weak( current, @@ -524,7 +942,21 @@ impl LexUserHistory { Ordering::SeqCst, Ordering::SeqCst, ) { - Ok(_) => return, + Ok(_) => { + // Decided from the values this CAS itself exchanged, not + // from a fresh load: a re-read could see a raise that + // landed after the swap and mistake it for one this + // checkpoint covered. + let raised = raised_deletion_of(current); + if raised > covered_of(current) && raised <= covered { + // The session's own claim is settled by this durable + // checkpoint. The inherited one is not — a checkpoint + // written now persists the *resurrected* entry — and + // the projection keeps the file if that is still owed. + lock_recover(&self.claims).session = None; + } + return; + } Err(observed) => current = observed, } } @@ -604,10 +1036,80 @@ impl LexUserHistory { // A Tombstone whose WAL durability failed (SyncFailed or Io): the // deletion must be checkpointed synchronously before returning (§5.4), // not left to an async scrub that a crash could preempt. - let mut durability_failed = false; + // + // Accumulated across the batch as a `DeletionBreach` rather than a + // bool, because the two halves are not interchangeable on the next + // start: `Io` has no frame and no heal, while `Unflushed` is settled + // by replay. `merge` keeps the claim that covers the whole batch — + // `Lost` absorbs, and two unflushed frames keep the higher seq. + let mut durability_failed: Option = None; let mut needs_threshold_compact = false; if !wal_records.is_empty() { let mut wal = lock_recover(&self.wal); + // Reconcile the marker with what the projection wants, before this + // batch touches the WAL. **This is where the projection's + // correctness lives**; every other projection point is either + // write-ahead ordering or promptness. + // + // Six review rounds arrived here one event at a time — the ack, the + // wipe, the compaction's cover, recovery's removal, recovery's + // promotion, and then the compaction that recovery schedules, each + // one a place where a failed write had nothing to revisit it. The + // answer was never another trigger. A record that is only + // reconciled at *some* events is not a projection, it is a cache + // with invalidation, and the design calls this the ledger's on-disk + // projection. So it reconciles wherever the process is running and + // able to act, which is here. + // + // Free in the steady state, and only because `flushed` is seeded + // from what recovery observed: `apply_marker` compares two + // `Option` under a mutex this thread already holds + // and returns. No syscall on a healthy history, which is what makes + // a key-path reconcile affordable at all. + // + // Before the appends, not after: the harm this closes is a WAL that + // advances past an un-promoted witness, so promoting after the + // append would leave the same window one batch wide. + // Unconditionally, and its result decides the freeze below — + // `&&` would short-circuit the reconcile away on the healthy path, + // which is the one place it has to happen. + let projected = self.project_marker(&wal); + if !projected + && self.inherited_owed.load(Ordering::SeqCst) + && lock_recover(&self.claims).flushed != MarkerState::Holds(DeletionBreach::Lost) + { + // The one case where appending is genuinely unsafe: a report is + // owed and the disk does not yet say so *unconditionally* — + // either nobody could read it, or it still holds the + // `Unflushed{seq}` the promotion failed to replace. + // + // A witness is a claim about one frame in one WAL file, and it + // is answered by `seq > applied_seq` — an **inequality** over a + // high water mark. Gaps are legal, so once that file has been + // replaced *any* later frame above the witness answers it + // "applied", whether or not the tombstone ever existed in the + // new lineage. Numbering cannot fix this — a floor was tried, + // and skipping one number changes nothing — so promotion to the + // lineage-independent `Lost` is mandatory, and until it lands + // the state that would answer the witness must not advance. + // + // Freezing is not a new mechanism: `frozen` already means + // "this file is not in a state where appending is safe", and + // numbering that may alias a live claim is exactly that. The + // batch becomes memory-only — reported as `LearningMemoryOnly` + // and healed by the compaction that rewrites both files, the + // same path an unrepairable tail takes. + // + // Deliberately *not* the broader "freeze whenever the marker + // write fails": a sidecar must not stop learning, the same rule + // that keeps a read failure from failing the open. What is + // exempt is a disk that already says `Lost` — unsatisfiable by + // any sequence, so nothing it could answer is at risk — and a + // claim this session raised about the file it is still + // appending to. An *inherited* witness is the dangerous one, + // which is why the condition names it. + wal.freeze(); + } let mut sequenced: Vec<(WalRecord, Option)> = Vec::with_capacity(wal_records.len()); for record in wal_records { @@ -671,7 +1173,7 @@ impl LexUserHistory { // the user a *deletion* did not persist — a privacy // claim about an operation they never requested. if matches!(record, WalRecord::Tombstone { .. }) { - durability_failed = true; + note_breach(&mut durability_failed, DeletionBreach::Unflushed { seq }); } sequenced.push((record, Some(seq))); } @@ -691,7 +1193,7 @@ impl LexUserHistory { // via the async heal a re-learnable Committed loss can // wait for. if matches!(record, WalRecord::Tombstone { .. }) { - durability_failed = true; + note_breach(&mut durability_failed, DeletionBreach::Lost); } sequenced.push((record, None)); } @@ -724,7 +1226,7 @@ impl LexUserHistory { self.append_commit_log(line); } - if durability_failed { + if durability_failed.is_some() { // A Tombstone could not be made durable through the WAL. Write // the checkpoint synchronously before returning so the deletion // survives a crash instead of resurrecting if an async scrub is @@ -773,12 +1275,49 @@ impl LexUserHistory { // Consumed only after the commit point: the wipe supersedes every // scrub request posted so far. self.scrub_pending.store(false, Ordering::SeqCst); + // …and so does the compaction veto, for the same reason and by the same + // act. The save above wrote a v2 checkpoint over the path a failed + // migration had left in v1 form, which is exactly what the veto existed + // to prevent a compaction from doing — except here it is intended: a + // wipe supersedes the history, including the copy nothing had migrated + // yet. Leaving it set would refuse the heal this very call posts below + // when a physical step fails, so a partial wipe with a frozen WAL could + // never be retried in this process. + self.v1_checkpoint_retained.store(false, Ordering::SeqCst); + // A wipe settles every claim before the cover, not after it: they said + // an entry might be back, and now nothing is. Ordering matters — the + // cover projects, so resetting afterwards would have it write the + // pre-wipe claim to disk and then need a second projection to take it + // straight back off. + // + // The reset holds whether or not the file can be unlinked: an + // unremovable marker is stale, not owed. What does *not* follow, and + // was claimed here until the design re-gate, is that the next startup + // reaches the same verdict on its own — it only does so while the + // history is still empty, and the user typing one thing before the + // restart makes replay non-empty and the stale `Lost` report again. + // `flushed` carries the disagreement instead, so the projection keeps + // retrying; the heal below is what makes the retry prompt. + // `session` only. `flushed` is not a claim to be settled, it is what + // this process knows about the disk, and a wipe does not make the file + // disappear — clearing it here would have the projection conclude the + // disk already agrees and skip the very removal this is for. + lock_recover(&self.claims).session = None; + self.inherited_owed.store(false, Ordering::SeqCst); // Likewise for the durability ledger (#295). An empty durable set // contains no un-deleted entry, so every raised deletion is now // vacuously persisted. Without this second cover point, wiping // everything would leave a standing "a deletion did not persist" // warning on a history that provably holds nothing. - self.cover_unpersisted(covered_gen); + // + // Not routed through `remove_recovery_artifacts`: that helper returns + // on its first error and only reaches the `.corrupt-*` files + // afterwards, so a marker that refuses to unlink would skip the + // deletion of files that do hold the user's input text. This one holds + // none (magic, version, flags, a seq), which is also why its failure + // stays a log line rather than joining `deferred`. + self.cover_unpersisted(&wal, covered_gen); + let marker_stuck = lock_recover(&self.claims).flushed != MarkerState::Absent; // Physical deletions below are deferred-error: the logical clear is // committed, so every step runs (the memory reset especially — @@ -851,7 +1390,17 @@ impl LexUserHistory { drop(wal); match deferred { - None => Ok(()), + None => { + if marker_stuck { + // Promptness only. Correctness is the commit-path + // reconcile: the next commit retires this record whether or + // not the compaction below ever runs. Without it a user who + // wipes and then stops typing would keep a stale file until + // they resumed — harmless, but a wipe should finish. + self.spawn_compact(); + } + Ok(()) + } Some(e) => { // Partial physical failure: the logical clear is done // (memory and checkpoint are empty) but some bytes remain. @@ -864,6 +1413,13 @@ impl LexUserHistory { } } + /// Whether a compaction may not run right now because the checkpoint path + /// still holds v1 bytes that only the migration commit knows how to + /// preserve. See [`Self::v1_checkpoint_retained`]. + fn compaction_barred(&self) -> bool { + self.v1_checkpoint_retained.load(Ordering::SeqCst) + } + /// Acquire the compaction gate. Poison recovery is trivially safe here: /// the gate protects no data (see the field docs). fn lock_gate(&self) -> MutexGuard<'_, ()> { @@ -888,6 +1444,15 @@ impl LexUserHistory { /// Spawn a threshold compaction (the caller has already observed /// `needs_compact()` under the wal lock). fn spawn_threshold_compact(self: &Arc) { + // Nothing to gain from a worker whose first act is `compaction_barred`. + // Not a second guard — the refusal in `run_compact_impl` is what makes + // this safe, and this cell is the same one it reads. It matters because + // a veto that holds never clears the threshold, so *every* commit from + // there on would spawn a thread that immediately gives up, on the key + // path, for the life of a degraded session. + if self.compaction_barred() { + return; + } // §4: threshold compactions skip when one is in flight — the // threshold stays exceeded and the next commit retries. Advisory // pre-check to avoid spawning a thread per commit while a @@ -898,6 +1463,8 @@ impl LexUserHistory { Err(std::sync::TryLockError::WouldBlock) => return, Err(std::sync::TryLockError::Poisoned(_)) => {} } + #[cfg(test)] + self.compact_threads.fetch_add(1, Ordering::SeqCst); let this = Arc::clone(self); if let Err(e) = std::thread::Builder::new() .name("lexime-history-compact".into()) @@ -951,6 +1518,16 @@ impl LexUserHistory { /// redundant run. fn spawn_compact(self: &Arc) { self.scrub_pending.store(true, Ordering::SeqCst); + // Posted first and then not run: the request is real and stays posted + // for whoever lifts the veto (only `clear` can, and it consumes the + // flag at its commit point before posting its own). What is skipped is + // the worker — see `spawn_threshold_compact` for why that is worth + // skipping rather than letting it park and refuse. + if self.compaction_barred() { + return; + } + #[cfg(test)] + self.compact_threads.fetch_add(1, Ordering::SeqCst); let this = Arc::clone(self); if let Err(e) = std::thread::Builder::new() .name("lexime-history-compact".into()) @@ -971,6 +1548,21 @@ impl LexUserHistory { /// whether the checkpoint became durable and whether a follow-up is /// warranted (see [`CompactOutcome`]). fn run_compact_impl(&self) -> CompactOutcome { + if self.compaction_barred() { + // The one invariant that has to live *here* rather than at the + // schedulers: a compaction writes a v2 checkpoint over the v1 file + // with none of the migration commit's steps, so while that file is + // the only copy no compaction may run — whichever of the five + // schedulers asked for it. Guarding them one at a time took three + // review rounds and still left the scrub branch open; this is the + // single site that performs the write. + // + // `Failed` rather than `Done`: the scrub really is still pending, + // and saying otherwise would drop it. The next launch retries the + // migration properly and everything proceeds from there. + warn!("compaction refused: a v1 checkpoint is awaiting its migration commit"); + return CompactOutcome::Failed; + } // 1. Clone history under read lock (brief), taking the generation // this checkpoint can vouch for under the same guard (#295). let (covered_gen, snapshot) = self.snapshot_to_cover(); @@ -982,19 +1574,22 @@ impl LexUserHistory { return CompactOutcome::Failed; } - // The deletion is persisted the moment this full-snapshot checkpoint - // is durable — before the truncation below, and regardless of whether - // it runs. Truncation is the physical scrub of superseded frames, not - // what makes the deletion survive a restart, so tying the cover to it - // would leave a permanent warning whenever frames land mid-run - // (FollowUp) or the truncate fails on an otherwise durable write. - self.cover_unpersisted(covered_gen); - // 3. Truncate WAL (brief lock) — conditionally (§5.3): only frames // provably covered by the durable checkpoint (seq <= applied_seq at // snapshot time) may be destroyed. let mut wal = lock_recover(&self.wal); + // The deletion is persisted the moment the full-snapshot checkpoint + // above became durable — before the truncation below, and regardless + // of whether it runs. Truncation is the physical scrub of superseded + // frames, not what makes the deletion survive a restart, so tying the + // cover to it would leave a permanent warning whenever frames land + // mid-run (FollowUp) or the truncate fails on an otherwise durable + // write. Only the *placement* moved under this guard, and only so the + // ledger update and the marker unlink cannot be split by a concurrent + // raise; the condition being covered is unchanged. + self.cover_unpersisted(&wal, covered_gen); + // The checkpoint is a full snapshot, so everything it contains is // now both on disk and in the state it was cloned from: the residue // keys that snapshot carried are settled. Keys raised *since* the @@ -1105,7 +1700,16 @@ mod tests { /// too, or the witness parameter would be documenting nothing. fn raise_under_wal(hist: &LexUserHistory) { let wal = lock_recover(&hist.wal); - hist.raise_unpersisted(&wal, true, true); + hist.raise_unpersisted(&wal, true, Some(DeletionBreach::Lost)); + } + + /// Both halves, as every durable checkpoint does them: the cover settles + /// the ledger, the projection is what reaches the disk. Splitting them + /// here would let a test pass against a pairing production does not have. + fn cover_under_wal(hist: &LexUserHistory, generation: u64) { + let wal = lock_recover(&hist.wal); + hist.cover_unpersisted(&wal, generation); + hist.project_marker(&wal); } fn gen_under_wal(hist: &LexUserHistory) -> u64 { @@ -1113,6 +1717,24 @@ mod tests { hist.deletion_gen_under_wal_lock(&wal) } + /// Plant a marker the way a startup hands one over: the file **and** the + /// runtime's record of what the file holds. + /// + /// Writing only the file is a state production cannot reach. Recovery + /// reports what it read (`OpenReport::marker_on_disk`) and `open` seeds + /// `flushed` from it, so the process never believes the disk is clear + /// while bytes sit there. A fixture that skips the second half is testing + /// the projection against a lie — and since the skip is symmetric, it + /// would simply decline to project at all. + fn plant_marker(hist: &LexUserHistory, cp: &Path, breach: DeletionBreach) { + deletion_marker::merge_write(cp, breach).unwrap(); + lock_recover(&hist.claims).flushed = MarkerState::Holds(breach); + } + + fn marker(cp: &Path) -> Option { + deletion_marker::read(cp).map(|o| o.breach) + } + fn committed(reading: &str, surface: &str) -> LearningRecord { LearningRecord::Committed { reading: reading.to_string(), @@ -1157,6 +1779,25 @@ mod tests { cp: &Path, io: Box, ) -> Arc { + hist_with_io_reporting(cp, io, false) + } + + /// As `hist_with_io`, but with a startup report that carries an inherited + /// lost-deletion claim — the state an acknowledgement acts on. + fn hist_with_io_reporting( + cp: &Path, + io: Box, + deletion_lost: bool, + ) -> Arc { + // The whole startup state, not two thirds of it: a report is owed + // because recovery read a marker and left it in place, so the file has + // to exist alongside `inherited_owed` and `flushed`. Setting only the + // in-memory halves describes a disk that never matched them, and the + // projection — which skips when it believes the disk already agrees — + // would then decline to write the marker a raise is meant to record. + if deletion_lost { + deletion_marker::merge_write(cp, DeletionBreach::Lost).unwrap(); + } let wal = HistoryWal::with_io(cp, io); Arc::new(LexUserHistory { inner: Arc::new(RwLock::new(UserHistory::new())), @@ -1178,8 +1819,61 @@ mod tests { quarantined_paths: Vec::new(), replayed_deletion: false, compaction_recommended: false, + v1_checkpoint_retained: false, + // Paired with `deletion_lost`, because recovery cannot produce + // one without the other: the report is owed *because* a marker + // was read and deliberately left in place. A fixture that + // reported the loss while claiming a clear disk would have the + // projection skip the removal it exists to perform, and the ack + // would settle against nothing. + marker_on_disk: if deletion_lost { + MarkerState::Holds(DeletionBreach::Lost) + } else { + MarkerState::Absent + }, + deletion_lost, + deletion_pending_checkpoint: false, }, durability_ledger: AtomicU64::new(0), + claims: Mutex::new(MarkerClaims { + session: None, + flushed: if deletion_lost { + MarkerState::Holds(DeletionBreach::Lost) + } else { + MarkerState::Absent + }, + }), + inherited_owed: AtomicBool::new(deletion_lost), + v1_checkpoint_retained: AtomicBool::new(false), + compact_threads: AtomicU64::new(0), + }) + } + + /// As `hist_with_io`, but opened the way a session is when a failed + /// migration left a v1 checkpoint on disk: the report recovery produced + /// *and* the runtime cell it seeds both say so. Both, because the two + /// answer different questions — what open time saw, and what the path holds + /// now — and a fixture that set only one would describe a session no + /// startup can produce. + fn hist_with_retained_v1(cp: &Path) -> Arc { + let base = hist_with_io(cp, FaultyIo::default().boxed()); + let mut report = base.report.clone(); + report.v1_checkpoint_retained = true; + Arc::new(LexUserHistory { + inner: Arc::clone(&base.inner), + wal: Mutex::new(HistoryWal::new(cp)), + compact_gate: Mutex::new(()), + scrub_pending: AtomicBool::new(false), + commit_log: Mutex::new(CommitLog { + path: cp.with_file_name("commit-log.jsonl"), + file: None, + }), + report, + durability_ledger: AtomicU64::new(0), + claims: Mutex::new(MarkerClaims::default()), + inherited_owed: AtomicBool::new(false), + v1_checkpoint_retained: AtomicBool::new(true), + compact_threads: AtomicU64::new(0), }) } @@ -1613,156 +2307,1217 @@ mod tests { std::fs::create_dir(blocker).unwrap(); } + /// Fail the *checkpoint write only*, leaving every sibling in the family + /// writable — including the marker, and including the checkpoint file + /// itself, which a restart still has to read. + /// + /// `blocked_checkpoint` cannot serve here: it makes the whole parent + /// directory a file, so the marker (same directory) cannot be written + /// either, and a test built on it would observe "no marker" and pass for + /// the wrong reason — the #312 case would have no test at all. This + /// instead puts a *directory* at the tmp path `write_atomic` needs, so + /// `File::create` fails with EISDIR and nothing else is disturbed. + /// + /// Idempotent: an `unwrap` here raced a startup compaction that a test's + /// own `open` had scheduled, which creates and renames that same tmp path. + /// It passed locally and failed in CI. What the caller needs is the + /// obstacle to be present, not to have been the one who placed it. + fn block_checkpoint_write(cp: &Path) { + match std::fs::create_dir(crate::user_history::checkpoint_tmp_path(cp)) { + Ok(()) => {} + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {} + Err(e) => panic!("could not block the checkpoint write: {e}"), + } + } + + /// Make every marker write fail, without disturbing a read: a directory + /// at the tmp `write_atomic` writes through is refused by `create_regular` + /// (EISDIR) while the canonical path is untouched. + fn block_marker_write(cp: &Path) { + std::fs::create_dir(crate::user_history::checkpoint_tmp_path( + &deletion_marker::marker_path(cp), + )) + .unwrap(); + } + + fn unblock_checkpoint_write(cp: &Path) { + std::fs::remove_dir(crate::user_history::checkpoint_tmp_path(cp)).unwrap(); + } + + // ----------------------------------------------------------------------- + // The unpersisted-deletion marker (#312): the runtime ledger dies with the + // process, so before this the `Io` half went unreported on exactly the + // restart where the entry comes back. + // ----------------------------------------------------------------------- + + /// Learn something, then delete it with the tombstone's append failing — + /// the `Io` half, where no frame exists at all. + fn lose_a_deletion(hist: &Arc, io: &FaultyIo) { + hist.apply_records(&[committed("きょう", "今日")]); + io.fail_appends.store(true, Ordering::SeqCst); + hist.apply_records(&[deletion("きょう", "今日")]); + } + #[test] - fn test_append_failure_keeps_memory_and_heals() { + fn test_lost_deletion_is_recorded_for_the_next_start() { let dir = tempfile::tempdir().unwrap(); let cp = dir.path().join("history.lxud"); let io = FaultyIo::default(); - io.fail_appends.store(true, Ordering::SeqCst); let hist = hist_with_io(&cp, io.boxed()); + block_checkpoint_write(&cp); - hist.apply_records(&[committed("きょう", "今日")]); + lose_a_deletion(&hist, &io); - // §5.2: memory keeps the record (immediate quality), applied_seq - // does not advance (the frame cannot replay), the WAL freezes, and - // a heal is posted. - assert_eq!(learned(&hist, "きょう"), vec!["今日".to_string()]); - assert_eq!(read_recover(&hist.inner).applied_seq(), 0); - assert!(lock_recover(&hist.wal).is_frozen()); - // F10 (#288): the frozen window is what makes this session's learning - // memory-only, and it is reportable while it lasts — not only after - // the fact via the next startup's OpenReport. + assert!(hist.has_unpersisted_deletion(), "runtime row still holds"); assert_eq!( - hist.durability_issues(), - vec![LexHistoryDurabilityIssue::LearningMemoryOnly] - ); - - // The healing compaction checkpoints memory (covering the effect) - // and re-truncates the WAL back to appendable form. - io.fail_appends.store(false, Ordering::SeqCst); - wait_until( - || !lock_recover(&hist.wal).is_frozen(), - "heal compaction to unfreeze the WAL", - ); - assert!(cp.exists(), "healed checkpoint persists the effect"); - assert!(io.truncates.load(std::sync::atomic::Ordering::SeqCst) >= 1); - // F11: the issue is clearable, not a latch — which is exactly why it - // is polled rather than folded into Swift's `initFailures`. - assert!( - hist.durability_issues().is_empty(), - "the heal must clear the report" + marker(&cp), + Some(DeletionBreach::Lost), + "a deletion with no frame and no checkpoint must reach the disk as Lost" ); - - // Nothing was lost: reopen from disk sees the entry (via checkpoint). - drop(hist); - let hist2 = open_hist(&cp); - assert_eq!(learned(&hist2, "きょう"), vec!["今日".to_string()]); } #[test] - fn test_tombstone_sync_failure_applies_with_real_seq() { + fn test_a_covering_checkpoint_retracts_the_marker() { + // The fallback checkpoint (§5.4) succeeding *is* the deletion being + // persisted, so nothing should be reported next start. let dir = tempfile::tempdir().unwrap(); let cp = dir.path().join("history.lxud"); let io = FaultyIo::default(); - io.fail_full_sync.store(true, Ordering::SeqCst); let hist = hist_with_io(&cp, io.boxed()); - hist.apply_records(&[committed("きょう", "今日")]); - let commit_seq = lock_recover(&hist.wal).last_appended_seq(); - hist.apply_records(&[deletion("きょう", "今日")]); + lose_a_deletion(&hist, &io); - // The frame is on disk (mock append succeeded); the failed flush - // must not leave it uncovered, or conditional truncation would - // stall until an unrelated append. - assert!( - learned(&hist, "きょう").is_empty(), - "memory delete still runs" - ); - assert!( - read_recover(&hist.inner).applied_seq() > commit_seq, - "tombstone must be covered by its real seq despite the sync failure" - ); - assert!(!lock_recover(&hist.wal).is_frozen()); - // §5.4: because the tombstone's WAL durability failed, the checkpoint - // is written SYNCHRONOUSLY before apply_records returns (no wait for a - // background thread) — the deletion cannot resurrect if a crash - // preempts an async scrub. - assert_eq!( - lock_recover(&hist.wal).entry_count(), - 0, - "durability-failed tombstone must checkpoint synchronously, not via async scrub" - ); - drop(hist); - let hist2 = open_hist(&cp); - assert!( - learned(&hist2, "きょう").is_empty(), - "synchronously-checkpointed deletion survives reopen" - ); + assert!(cp.exists(), "the fallback checkpoint must have landed"); + assert_eq!(marker(&cp), None, "a covered deletion leaves no record"); + assert!(!hist.has_unpersisted_deletion()); } #[test] - fn test_tombstone_append_failure_checkpoints_synchronously() { - // Io on a tombstone append: the frame never reaches disk, so memory - // is the only copy of the deletion. It must be checkpointed - // synchronously before returning — a crash before an async scrub - // would otherwise resurrect the deleted entry from the old checkpoint. + fn test_a_failed_compaction_leaves_the_marker_standing() { let dir = tempfile::tempdir().unwrap(); let cp = dir.path().join("history.lxud"); let io = FaultyIo::default(); let hist = hist_with_io(&cp, io.boxed()); + block_checkpoint_write(&cp); - hist.apply_records(&[committed("きょう", "今日")]); - hist.run_compact(); // the entry now lives in the checkpoint - assert!(cp.exists()); + lose_a_deletion(&hist, &io); + assert_eq!(marker(&cp), Some(DeletionBreach::Lost)); - // The tombstone's frame fails to append (frozen WAL, memory-only delete). - io.fail_appends.store(true, Ordering::SeqCst); - hist.apply_records(&[deletion("きょう", "今日")]); - assert!(learned(&hist, "きょう").is_empty(), "memory delete runs"); - // F1: the deletion *was* raised, and the synchronous fallback - // checkpoint covered it before returning — so nothing is reported. - // The WAL is still frozen, which is its own (accurate) issue. - assert!( - !hist - .durability_issues() - .contains(&LexHistoryDurabilityIssue::DeletionNotPersisted), - "a deletion the fallback persisted must not be reported as lost" - ); + // Disk heals: the next compaction writes a checkpoint that no longer + // contains the entry, which is what makes the deletion durable. + io.fail_appends.store(false, Ordering::SeqCst); + unblock_checkpoint_write(&cp); + hist.scrub_pending.store(true, Ordering::SeqCst); + hist.run_gated_compact(); - // Synchronous durability: reopening immediately (no wait for a - // background scrub) must not resurrect the deletion. - drop(hist); - let hist2 = open_hist(&cp); - assert!( - learned(&hist2, "きょう").is_empty(), - "durability-failed tombstone is checkpointed before return, not resurrected" + assert_eq!( + marker(&cp), + None, + "a durable checkpoint retracts the record along with the ledger" ); } #[test] - fn test_durability_failure_with_failed_checkpoint_keeps_scrub_pending() { - // R2: if the synchronous durability checkpoint ALSO fails (stuck - // disk), the scrub must stay pending so a later compaction / startup - // retries it — the deletion must not be silently dropped with the - // pending flag consumed. + fn test_a_durable_checkpoint_retracts_even_if_the_truncate_fails() { + // Retraction is owed to the checkpoint, not to the WAL truncation that + // follows it (AGENTS (d)). Gating on the truncation would leave a + // standing warning whenever the truncate fails on an otherwise durable + // write, or frames land mid-run. + // + // Uses the truncate-failure outcome rather than FollowUp: both reach + // the same question, and this one is deterministic — FollowUp needs a + // frame to land between a compaction's own snapshot and its truncate, + // which no fixture can schedule. let dir = tempfile::tempdir().unwrap(); - let cp = blocked_checkpoint(dir.path()); + let cp = dir.path().join("history.lxud"); let io = FaultyIo::default(); - io.fail_full_sync.store(true, Ordering::SeqCst); let hist = hist_with_io(&cp, io.boxed()); + block_checkpoint_write(&cp); - // Learn, then delete: the tombstone's sync_full fails (SyncFailed) - // AND the synchronous checkpoint save fails (blocked parent). hist.apply_records(&[committed("きょう", "今日")]); + io.fail_full_sync.store(true, Ordering::SeqCst); hist.apply_records(&[deletion("きょう", "今日")]); + io.fail_full_sync.store(false, Ordering::SeqCst); + assert!(matches!( + marker(&cp), + Some(DeletionBreach::Unflushed { .. }) + )); - assert!( - learned(&hist, "きょう").is_empty(), - "memory delete still runs" - ); - assert!( - hist.scrub_pending.load(Ordering::SeqCst), - "a failed durability checkpoint must keep the scrub pending for retry" + unblock_checkpoint_write(&cp); + io.fail_truncates.store(true, Ordering::SeqCst); + assert!(matches!(hist.run_compact(), CompactOutcome::Done)); + assert_eq!( + marker(&cp), + None, + "a durable checkpoint persists the deletion; the truncation is only the scrub" + ); + } + + #[test] + fn test_lost_survives_a_later_unflushed_raise() { + // The reachable route to an `Unflushed` landing on an outstanding + // `Lost`, which is what the read-modify-write merge exists for. It is + // not the obvious one: an `Io` append freezes the WAL, and the frozen + // guard turns every later append into `Io` too. What lifts the freeze + // is a compaction whose cover generation predates the raise — it + // leaves the marker standing — after which the next tombstone can + // append and fail its flush. + // + // An earlier version drove "both orders" through fail_appends / + // fail_full_sync and claimed to cover this one; the freeze made that + // iteration produce `Lost` twice, and a merge rewritten to let + // `Unflushed` win stayed green. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let io = FaultyIo::default(); + let hist = hist_with_io(&cp, io.boxed()); + block_checkpoint_write(&cp); + + hist.apply_records(&[committed("きょう", "今日"), committed("あす", "明日")]); + io.fail_appends.store(true, Ordering::SeqCst); + hist.apply_records(&[deletion("きょう", "今日")]); + assert_eq!(marker(&cp), Some(DeletionBreach::Lost)); + assert!(lock_recover(&hist.wal).is_frozen()); + + // A cover carrying a generation older than the raise settles nothing, + // and the truncation that follows it is what thaws the file. + io.fail_appends.store(false, Ordering::SeqCst); + { + let wal = lock_recover(&hist.wal); + hist.cover_unpersisted(&wal, 0); + } + lock_recover(&hist.wal).truncate_wal().unwrap(); + assert!(!lock_recover(&hist.wal).is_frozen()); + assert_eq!( + marker(&cp), + Some(DeletionBreach::Lost), + "a stale cover must not settle the claim" + ); + + io.fail_full_sync.store(true, Ordering::SeqCst); + hist.apply_records(&[deletion("あす", "明日")]); + assert_eq!( + marker(&cp), + Some(DeletionBreach::Lost), + "the unhealable claim must not be downgraded to a suppressible witness" + ); + } + + #[test] + fn test_a_failed_marker_write_does_not_drop_the_claim() { + // The merge runs against the file, so a write that failed leaves + // nothing to merge with, and the next weaker raise would start from an + // empty read — on the failing disk where the write fails, which is the + // only disk this path runs on. The session keeps its own claim. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let io = FaultyIo::default(); + let hist = hist_with_io(&cp, io.boxed()); + block_checkpoint_write(&cp); + hist.apply_records(&[committed("きょう", "今日"), committed("あす", "明日")]); + + // A *non-empty* directory at the marker path fails that write and + // nothing else. An empty one would not: the writer owns this path and + // clears a placeholder out of its way. What it will not do is delete + // someone else's contents. + // At the *tmp*, not the canonical name: a directory at the canonical + // name no longer fails the write, because `write_atomic` flushes the + // tmp before renaming and `read` merges that orphan — the claim lands. + // Blocking the tmp stops `create_regular` outright, which is what + // "the write failed" now means. + let marker_dir = + crate::user_history::checkpoint_tmp_path(&deletion_marker::marker_path(&cp)); + std::fs::create_dir(&marker_dir).unwrap(); + std::fs::write(marker_dir.join("restored"), b"not ours").unwrap(); + io.fail_appends.store(true, Ordering::SeqCst); + hist.apply_records(&[deletion("きょう", "今日")]); + assert_eq!( + marker(&cp), + Some(DeletionBreach::Lost), + "unreadable reads as Lost" + ); + std::fs::remove_dir_all(&marker_dir).unwrap(); + // The rename failed, so the atomic write's tmp is still beside the + // absent marker holding the claim — and `read` merges it, which is + // what makes an orphan strengthen rather than hide. Clearing it here + // isolates what this test is about: the claim surviving in *memory*. + assert_eq!( + marker(&cp), + None, + "and now there is genuinely nothing on disk" + ); + + // The disk accepts the marker again, and a weaker breach arrives. + io.fail_appends.store(false, Ordering::SeqCst); + lock_recover(&hist.wal).truncate_wal().unwrap(); + io.fail_full_sync.store(true, Ordering::SeqCst); + hist.apply_records(&[deletion("あす", "明日")]); + + assert_eq!( + marker(&cp), + Some(DeletionBreach::Lost), + "the claim whose write failed must be re-asserted, not forgotten" + ); + } + + #[test] + fn test_unflushed_raises_keep_the_higher_seq() { + // The suppression test asks whether the loaded state reached the + // witness, so the lower of two seqs can be covered while the higher is + // still missing. Keeping the minimum would suppress a real loss. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let io = FaultyIo::default(); + let hist = hist_with_io(&cp, io.boxed()); + block_checkpoint_write(&cp); + + hist.apply_records(&[committed("きょう", "今日"), committed("あす", "明日")]); + io.fail_full_sync.store(true, Ordering::SeqCst); + hist.apply_records(&[deletion("きょう", "今日")]); + let first = match marker(&cp) { + Some(DeletionBreach::Unflushed { seq }) => seq, + other => panic!("expected an unflushed witness, got {other:?}"), + }; + hist.apply_records(&[deletion("あす", "明日")]); + + match marker(&cp) { + Some(DeletionBreach::Unflushed { seq }) => { + assert!( + seq > first, + "the later frame's seq must win ({seq} > {first})" + ); + // Joined to the seq the WAL actually assigned, not just to the + // other witness. Without this, recording `seq + 1` or `seq - 1` + // passes every test: one latches a false privacy alarm, the + // other silently suppresses a genuine loss, and a comparison + // between two witnesses sees neither. + assert_eq!( + seq, + lock_recover(&hist.wal).last_appended_seq(), + "the witness must be the frame's own seq" + ); + } + other => panic!("expected an unflushed witness, got {other:?}"), + } + } + + #[test] + fn test_a_replayed_unflushed_deletion_is_a_live_problem_until_checkpointed() { + // The cross-crate half of "startup never retracts". Recovery hands the + // claim over instead of unlinking, and this is where it becomes a live + // durability problem: replay applied the deletion out of the page + // cache, so until a checkpoint covers it, power loss still undoes it. + // Without the hand-off the row is silent and the marker gets settled by + // whatever the next compaction happens to do. + // + // Built against real files rather than `hist_with_io`: that fixture + // mocks every WAL write, so a tombstone frame never reaches the disk + // and no reopen can replay one. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + + let mut history = UserHistory::new(); + history.record_at( + &[("きょう".to_string(), "今日".to_string())], + crate::user_history::now_epoch(), + ); + history.save(&cp).unwrap(); + + // A real tombstone frame, as a SyncFailed append leaves it: on disk, + // its flush unconfirmed. + let seq = { + let mut wal = HistoryWal::new(&cp); + wal.append_record(&WalRecord::Tombstone { + segments: vec![("きょう".to_string(), "今日".to_string())], + timestamp: crate::user_history::now_epoch(), + }) + .unwrap() + }; + // Written before the open, so it reaches the runtime the production + // way — through recovery, which reports what it read. + deletion_marker::merge_write(&cp, DeletionBreach::Unflushed { seq }).unwrap(); + + let reopened = open_hist(&cp); + assert!( + learned(&reopened, "きょう").is_empty(), + "replay must have applied the deletion" + ); + assert!( + !reopened.open_report().deletion_lost, + "so nothing is owed to the user as a past loss" + ); + assert!( + reopened + .durability_issues() + .contains(&LexHistoryDurabilityIssue::DeletionNotPersisted), + "but it is not durable yet, and the runtime row is what says so" + ); + assert!( + marker(&cp).is_some(), + "the record stands until a checkpoint covers it" + ); + + reopened.scrub_pending.store(true, Ordering::SeqCst); + reopened.run_gated_compact(); + assert!(reopened.durability_issues().is_empty()); + assert_eq!( + marker(&cp), + None, + "a durable checkpoint is what retracts it — the only thing that can" + ); + } + + #[test] + fn test_a_projection_that_did_not_flush_is_retried() { + // The skip is keyed on having *flushed* the value, not on the file's + // bytes matching. Matching bytes prove the content reached the page + // cache, so keying on them would read a failed `sync_all` back as + // up-to-date and never retry it — silently reopening the power-loss + // window `merge_write` refuses to open for a 0.3ms saving. + // + // Modelled by a write that fails outright: the bytes are absent, the + // claim is retained, and the next projection must try again rather + // than conclude anything from the previous attempt. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let io = FaultyIo::default(); + let hist = hist_with_io(&cp, io.boxed()); + block_checkpoint_write(&cp); + hist.apply_records(&[committed("きょう", "今日")]); + + // At the *tmp*, not the canonical name: a directory at the canonical + // name no longer fails the write, because `write_atomic` flushes the + // tmp before renaming and `read` merges that orphan — the claim lands. + // Blocking the tmp stops `create_regular` outright, which is what + // "the write failed" now means. + let marker_dir = + crate::user_history::checkpoint_tmp_path(&deletion_marker::marker_path(&cp)); + std::fs::create_dir(&marker_dir).unwrap(); + std::fs::write(marker_dir.join("restored"), b"not ours").unwrap(); + io.fail_appends.store(true, Ordering::SeqCst); + hist.apply_records(&[deletion("きょう", "今日")]); + // `Unknown`, not `Absent`: nothing may be remembered as flushed, and + // "the path is clear" is itself a claim this process cannot make after + // a write that failed — the same write could have left a truncated + // orphan behind, and only `Unknown` refuses to satisfy anything. + assert_eq!(lock_recover(&hist.claims).flushed, MarkerState::Unknown); + + std::fs::remove_dir_all(&marker_dir).unwrap(); + hist.apply_records(&[committed("あした", "明日")]); + assert_eq!( + marker(&cp), + Some(DeletionBreach::Lost), + "the retry must happen because the value was never flushed" + ); + assert_eq!( + lock_recover(&hist.claims).flushed, + MarkerState::Holds(DeletionBreach::Lost), + "and only now is it remembered as flushed" + ); + } + + #[test] + fn test_a_later_memory_only_raise_re_asserts_a_failed_claim() { + // A `Lost` write that failed leaves the claim in memory, and the next + // raise re-asserts it — including a memory-only one, which carries no + // breach of its own. The projection is of the *claims*, not of the + // breach this particular raise happened to bring, so there is no site + // that can forget to retry. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let io = FaultyIo::default(); + let hist = hist_with_io(&cp, io.boxed()); + block_checkpoint_write(&cp); + hist.apply_records(&[committed("きょう", "今日")]); + + // The marker write fails: a non-empty directory is the one shape the + // writer will not clear out of its way. + // At the *tmp*, not the canonical name: a directory at the canonical + // name no longer fails the write, because `write_atomic` flushes the + // tmp before renaming and `read` merges that orphan — the claim lands. + // Blocking the tmp stops `create_regular` outright, which is what + // "the write failed" now means. + let marker_dir = + crate::user_history::checkpoint_tmp_path(&deletion_marker::marker_path(&cp)); + std::fs::create_dir(&marker_dir).unwrap(); + std::fs::write(marker_dir.join("restored"), b"not ours").unwrap(); + io.fail_appends.store(true, Ordering::SeqCst); + hist.apply_records(&[deletion("きょう", "今日")]); + std::fs::remove_dir_all(&marker_dir).unwrap(); + // Same as above: the failed rename leaves the atomic write's tmp, and + // `read` merges it. Clear it so the assertion is about memory. + assert_eq!(marker(&cp), None, "nothing reached the disk"); + + // A later *commit* against the frozen WAL — no deletion, so no breach. + hist.apply_records(&[committed("あした", "明日")]); + + assert_eq!( + marker(&cp), + Some(DeletionBreach::Lost), + "the retained claim must be re-asserted on any raise, not only on another deletion" + ); + } + + #[test] + fn test_an_ack_whose_removal_fails_keeps_the_report_owed() { + // The acknowledgement clears the flag only when the record is really + // gone. A path the engine cannot clear — here a directory something + // else filled — used to be settled anyway, so the row disappeared + // while the marker stood and the warning came back next launch with + // the retry gone. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let io = FaultyIo::default(); + let hist = hist_with_io_reporting(&cp, io.boxed(), true); + // Replace the marker the helper left with something the engine cannot + // clear. Reachable: recovery reads a non-regular path conservatively as + // `Lost` and leaves it, so the report is owed against a record no + // unlink will retire. + let marker_path = deletion_marker::marker_path(&cp); + std::fs::remove_file(&marker_path).unwrap(); + std::fs::create_dir(&marker_path).unwrap(); + std::fs::write(marker_path.join("restored"), b"not ours").unwrap(); + + hist.ack_open_report(); + + assert!( + hist.deletion_report_owed(), + "an acknowledgement that could not clear the record still owes it" + ); + assert!( + marker_path.join("restored").exists(), + "and it did not delete what it does not own" + ); + } + + #[test] + fn test_a_cover_leaves_an_inherited_report_that_was_never_delivered() { + // The third authority error, and the narrowest reopening of #312. A + // session that inherits a report, raises a breach of its own, and then + // heals gets a cover — but a checkpoint written here persists the + // *resurrected* entry rather than removing it, so it settles nothing + // about the inherited claim. The two share one file, so unlinking on + // the ledger alone destroys a report the user never saw. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let io = FaultyIo::default(); + let hist = hist_with_io_reporting(&cp, io.boxed(), true); + plant_marker(&hist, &cp, DeletionBreach::Lost); + block_checkpoint_write(&cp); + + // This session loses a deletion of its own, then the disk recovers. + hist.apply_records(&[committed("きょう", "今日")]); + io.fail_appends.store(true, Ordering::SeqCst); + hist.apply_records(&[deletion("きょう", "今日")]); + assert!(hist.has_unpersisted_deletion()); + io.fail_appends.store(false, Ordering::SeqCst); + unblock_checkpoint_write(&cp); + hist.scrub_pending.store(true, Ordering::SeqCst); + hist.run_gated_compact(); + + assert!( + !hist.has_unpersisted_deletion(), + "this session's breach is genuinely covered" + ); + assert_eq!( + marker(&cp), + Some(DeletionBreach::Lost), + "but the undelivered report from the previous session is not" + ); + + // Delivery is what settles it, and then a later cover may reclaim it. + hist.ack_open_report(); + assert!( + !hist.deletion_report_owed(), + "a clean ack retires the report" + ); + assert_eq!(marker(&cp), None); + } + + #[test] + fn test_ack_leaves_a_marker_this_session_raised() { + // `report.deletion_lost` is frozen at open, so acknowledging on it + // alone deletes whatever marker is on disk *now* — including one a + // raise wrote minutes later, whose breach is still outstanding. That + // session's own report would then be the thing that goes missing. + // + // Reachable as soon as the ack moves to the row's click handler, which + // is exactly where it had to move so probe launches — and menu builds + // nobody sees — stop consuming reports. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let io = FaultyIo::default(); + let hist = hist_with_io_reporting(&cp, io.boxed(), true); + block_checkpoint_write(&cp); + + hist.apply_records(&[committed("きょう", "今日")]); + io.fail_appends.store(true, Ordering::SeqCst); + hist.apply_records(&[deletion("きょう", "今日")]); + assert_eq!(marker(&cp), Some(DeletionBreach::Lost)); + assert!(hist.has_unpersisted_deletion()); + + hist.ack_open_report(); + assert!( + hist.deletion_report_owed(), + "an ack that retires nothing leaves the report owed, or the caller drops the row that is the only way to retry it" + ); + assert_eq!( + marker(&cp), + Some(DeletionBreach::Lost), + "an outstanding breach of this session's own must survive the ack" + ); + } + + #[test] + fn test_memory_only_learning_writes_no_marker() { + // The marker is a claim about a *deletion*. A Committed append that + // never reached the WAL is memory-only learning — reportable while it + // lasts, but not something the user asked to erase. Writing a marker + // for it would latch 「前回のセッションの削除が保存されていません」 at the + // next launch for a deletion nobody requested, and the ledger + // assertions alone cannot see that. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let io = FaultyIo::default(); + io.fail_appends.store(true, Ordering::SeqCst); + let hist = hist_with_io(&cp, io.boxed()); + + hist.apply_records(&[committed("きょう", "今日")]); + + assert_eq!( + hist.durability_issues(), + vec![LexHistoryDurabilityIssue::LearningMemoryOnly] + ); + assert_eq!(marker(&cp), None, "learning is not a deletion"); + } + + #[test] + fn test_a_replayed_witness_is_not_unlinked_before_a_checkpoint_covers_it() { + // The ledger and the claims have to agree about what is outstanding. + // `deletion_pending_checkpoint` raises the ledger for a deletion replay + // applied but no checkpoint covers; with `session` left empty the + // projection computed a desired state of `None`, and once every commit + // reconciles, the first ordinary commit unlinked the witness before + // anything had persisted the deletion — a power loss would then restore + // the entry with no record left to report it. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let seq = { + let mut wal = HistoryWal::new(&cp); + wal.append_record(&WalRecord::Tombstone { + segments: vec![("きょう".to_string(), "今日".to_string())], + timestamp: crate::user_history::now_epoch(), + }) + .unwrap() + }; + deletion_marker::merge_write(&cp, DeletionBreach::Unflushed { seq }).unwrap(); + + // Before the open, not after: `open` schedules a startup compaction for + // the replayed deletion, and that compaction *is* a covering + // checkpoint. Blocking afterwards left the two racing — the test + // passed locally and failed in CI. + block_checkpoint_write(&cp); + let hist = open_hist(&cp); + assert!( + hist.has_unpersisted_deletion(), + "the replayed witness is a live durability problem" + ); + hist.apply_records(&[committed("あした", "明日")]); + + assert_eq!( + marker(&cp), + Some(DeletionBreach::Unflushed { seq }), + "only a covering checkpoint may retire it, not an unrelated commit" + ); + } + + #[test] + fn test_a_retained_v1_checkpoint_refuses_every_compaction() { + // The guard has to be here, not at the schedulers. Three rounds went + // into gating them one at a time — the marker feeders, then the + // startup hint — and the scrub branch still reached `spawn_compact()` + // after the directory recovered, overwriting the v1 checkpoint with a + // v2 snapshot that skips the migration commit's `.v1.bak` and + // `Migrated` steps. There are five schedulers and nothing stops a + // sixth; this is the single site that performs the write. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let guarded = hist_with_retained_v1(&cp); + // The state recovery reports when a v1 checkpoint is still awaiting its + // commit and no legacy WAL was consumed. + let v1 = b"not a v2 checkpoint at all".to_vec(); + std::fs::write(&cp, &v1).unwrap(); + + // Reach the executor directly: going through a scheduler would test + // that scheduler, which is exactly the level this moved away from. + assert!( + matches!(guarded.run_compact(), CompactOutcome::Failed), + "the executor must refuse, not merely not be asked" + ); + assert_eq!( + std::fs::read(&cp).unwrap(), + v1, + "and the v1 bytes — the only copy — must still be there" + ); + } + + #[test] + fn test_a_committed_clear_releases_the_compaction_veto() { + // The veto protects a v1 checkpoint that only the migration commit + // knows how to preserve — and `clear` overwrites that same path with an + // empty v2 checkpoint, on purpose: a wipe supersedes the history, + // including the copy nothing had migrated yet. Keyed on the startup + // report the veto would outlive the bytes it protects and refuse the + // heal `clear` itself posts for a failed physical step, leaving a + // frozen WAL and a half-finished wipe with no retry short of a restart + // — on a flow that deliberately does not restart. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let hist = hist_with_retained_v1(&cp); + std::fs::write(&cp, b"not a v2 checkpoint at all").unwrap(); + + hist.clear_impl().expect("the wipe itself must succeed"); + + assert!( + matches!(hist.run_compact(), CompactOutcome::Done), + "the v1 bytes are gone — nothing is left for the veto to protect" + ); + } + + #[test] + fn test_no_compaction_worker_is_created_while_the_veto_holds() { + // A veto that holds never clears the threshold, so every commit from + // there on asks for a compaction that cannot run. The refusal in + // `run_compact_impl` keeps that correct; what it cannot do is keep a + // degraded session from creating an OS thread per commit on the key + // path. Counted rather than inferred: a worker that spawns and then + // refuses leaves exactly the state one that never spawned leaves. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let hist = hist_with_retained_v1(&cp); + std::fs::write(&cp, b"not a v2 checkpoint at all").unwrap(); + + hist.spawn_compact(); + hist.spawn_threshold_compact(); + assert_eq!( + hist.compact_threads.load(Ordering::SeqCst), + 0, + "no worker may be created while every one of them would refuse" + ); + assert!( + hist.scrub_pending.load(Ordering::SeqCst), + "the request itself stays posted for whoever lifts the veto" + ); + + // …and the skip really is the veto's, not an unconditional one. + hist.clear_impl().unwrap(); + hist.spawn_compact(); + assert_eq!( + hist.compact_threads.load(Ordering::SeqCst), + 1, + "and once the veto lifts the same call does create one" + ); + } + + #[test] + fn test_a_stronger_claim_on_disk_satisfies_a_weaker_one() { + // Without this the projection livelocks. A stale `Lost` survives a + // failed cleanup, a later `SyncFailed` deletion makes the desired + // state `Unflushed`, and `merge_write` absorbs that request straight + // back into `Lost` — so under exact equality the desired state is + // unreachable and *every* commit pays another key-thread full sync, + // forever, while the restart still reports the loss. + // + // Claims are one-directional, so a stronger record covers a weaker + // want: it reports more, never less, which is the only direction this + // format may fail in. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let hist = hist_with_io(&cp, FaultyIo::default().boxed()); + plant_marker(&hist, &cp, DeletionBreach::Lost); + + // Blocked, which is what makes the difference observable: under exact + // equality the projection would try to write and fail, so a `true` + // here means it recognised the disk as already sufficient and issued + // no syscall at all. Asserting on the resulting state cannot see this + // — both paths end at `Holds(Lost)`, since the write merges back. + block_marker_write(&cp); + { + let wal = lock_recover(&hist.wal); + assert!( + hist.apply_marker(&wal, Some(DeletionBreach::Unflushed { seq: 7 })), + "the disk already says more than this asks for, so nothing is written" + ); + } + assert_eq!( + lock_recover(&hist.claims).flushed, + MarkerState::Holds(DeletionBreach::Lost), + "and the belief is unchanged — nothing needed writing" + ); + + // The converse must not hold: a weaker record does not cover a stronger + // want, which is what keeps a failed promotion retrying rather than + // deciding a witness is good enough for an unconditional claim. + assert!( + !MarkerState::Holds(DeletionBreach::Unflushed { seq: 7 }) + .satisfies(Some(DeletionBreach::Lost)), + "a witness must never be taken to cover an unconditional claim" + ); + } + + #[test] + fn test_the_belief_records_what_the_write_actually_persisted() { + // `merge_write` merges, so a request is not what lands. With a + // surviving `Lost` on disk and a later `SyncFailed` deletion asking for + // `Unflushed`, the write correctly keeps the stronger `Lost` — and a + // caller that recorded its own request would hold a belief the disk + // never had. The next reconcile would then find the desired witness + // "already satisfied", skip, and leave the stronger claim standing for + // the next startup to report against a deletion replay had applied. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let hist = hist_with_io(&cp, FaultyIo::default().boxed()); + plant_marker(&hist, &cp, DeletionBreach::Lost); + + { + let wal = lock_recover(&hist.wal); + hist.apply_marker(&wal, Some(DeletionBreach::Unflushed { seq: 7 })); + } + + assert_eq!( + marker(&cp), + Some(DeletionBreach::Lost), + "the merge keeps the stronger claim on disk" + ); + assert_eq!( + lock_recover(&hist.claims).flushed, + MarkerState::Holds(DeletionBreach::Lost), + "and the belief must say so, not repeat the request" + ); + } + + #[test] + fn test_an_unknown_marker_is_not_mistaken_for_an_absent_one() { + // The residue of a startup that refuted an *unreadable* marker and + // could not unlink it. Recording that as absence made the projection + // find `Absent == None`, skip, and leave the file standing — so once + // the user learned anything, the survivor produced a false + // lost-deletion report on the next start. `Unknown` never satisfies + // any desired state, so the removal is retried instead. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let hist = hist_with_io(&cp, FaultyIo::default().boxed()); + deletion_marker::merge_write(&cp, DeletionBreach::Lost).unwrap(); + lock_recover(&hist.claims).flushed = MarkerState::Unknown; + assert!(!hist.deletion_report_owed(), "nothing is owed"); + + hist.apply_records(&[committed("きょう", "今日")]); + + assert_eq!( + marker(&cp), + None, + "a file nobody could read is not a clear path; the removal must be retried" + ); + assert_eq!( + lock_recover(&hist.claims).flushed, + MarkerState::Absent, + "and only a removal that succeeded may record absence" + ); + } + + #[test] + fn test_an_unknown_marker_that_will_not_yield_freezes_appends() { + // A marker file nobody can read may hold an `Unflushed{seq}` naming a + // number no floor could protect — recovery never saw the seq. While + // the write that would replace it with an unconditional `Lost` keeps + // failing, issuing more sequence numbers risks handing out that one, + // and a later startup would read it as evidence the deletion replayed. + // + // So this batch goes memory-only instead: reported, and healed by the + // compaction that rewrites both files. Not the broader "freeze on any + // failed marker write" — a disk already saying `Lost` cannot be + // satisfied by any sequence, and a claim this session raised is about + // the file it is still appending to. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let hist = hist_with_io(&cp, FaultyIo::default().boxed()); + lock_recover(&hist.claims).flushed = MarkerState::Unknown; + // Inherited, which is the whole point: a witness from a *previous* WAL + // lineage is the one nothing can answer. This session's own claim is + // about the file it is still appending to. + hist.inherited_owed.store(true, Ordering::SeqCst); + // Nothing can be written into a directory that will not take one. + block_marker_write(&cp); + + hist.apply_records(&[committed("きょう", "今日")]); + + assert!( + hist.durability_issues() + .contains(&LexHistoryDurabilityIssue::LearningMemoryOnly), + "the batch must be memory-only rather than consuming sequence numbers" + ); + } + + #[test] + fn test_an_ordinary_commit_reconciles_the_marker_both_ways() { + // Where the projection's correctness actually lives, after six review + // rounds spent adding one retry trigger at a time (the ack, the wipe, + // the compaction's cover, recovery's removal, recovery's promotion, + // and the compaction recovery schedules). None of those is needed for + // *correctness* any more: whatever left the disk disagreeing, the next + // ordinary commit fixes it, because a projection that only reconciles + // at chosen events is a cache with invalidation rather than a + // projection. + // + // No compaction anywhere in this test — one record is nowhere near the + // threshold — which is the whole point. + // + // What this cannot pin is the *placement*: reconciling after the + // appends instead of before passes every test here, because the only + // difference is a crash landing between the append and the reconcile, + // one batch wide. Confirmed undetectable by measurement, so the + // ordering is carried by the comment at the call site and by the + // argument for it — a WAL that advances past an un-promoted witness is + // exactly the harm being closed. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let hist = hist_with_io(&cp, FaultyIo::default().boxed()); + + // Direction 1: the disk holds a record nothing claims — an unlink that + // failed, wherever it failed. + plant_marker(&hist, &cp, DeletionBreach::Lost); + assert!(!hist.deletion_report_owed(), "nothing is owed"); + hist.apply_records(&[committed("きょう", "今日")]); + assert_eq!( + marker(&cp), + None, + "a commit must retire a record no claim stands behind" + ); + + // Direction 2: a claim is owed and the disk does not say so — the + // startup promotion that could not write. Left unreconciled, ordinary + // commits advance the WAL past the witness and the next start reads it + // as replayed. + let owed = hist_with_io_reporting(&cp, FaultyIo::default().boxed(), true); + std::fs::remove_file(deletion_marker::marker_path(&cp)).unwrap(); + lock_recover(&owed.claims).flushed = MarkerState::Absent; + owed.apply_records(&[committed("あした", "明日")]); + assert_eq!( + marker(&cp), + Some(DeletionBreach::Lost), + "a commit must re-assert a claim the disk is missing" + ); + } + + #[test] + fn test_a_checkpoint_retries_a_marker_removal_that_failed_earlier() { + // The residue of an unlink that was refused between a successful save + // and the removal that should have followed: the claim is settled, so + // nothing raises it again, and every later cover early-returns off the + // covered ledger. Without an unconditional projection here, the stale + // file has nothing left to revisit it, and the next launch reports a + // previous-session loss against a deletion this very checkpoint made + // durable — a warning telling the user to go delete an entry that is + // already gone. + // + // The fixture is that residue exactly, and it is the state that tells + // it apart from an inherited claim (the test above): a file on disk + // with no claim behind it and nothing owed. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let hist = hist_with_io(&cp, FaultyIo::default().boxed()); + plant_marker(&hist, &cp, DeletionBreach::Lost); + assert!(!hist.deletion_report_owed(), "nothing is owed to the user"); + assert!(!hist.has_unpersisted_deletion(), "and no claim is standing"); + + hist.apply_records(&[committed("きょう", "今日")]); + assert!( + matches!(hist.run_compact(), CompactOutcome::Done), + "fixture needs a durable save" + ); + + assert_eq!( + marker(&cp), + None, + "a checkpoint must retry a removal nothing else would" + ); + } + + #[test] + fn test_a_cover_for_memory_only_learning_leaves_an_inherited_marker() { + // The ledger shares one generation sequence, so a memory-only raise + // moves it without raising `raised_deletion`. Covering that must not + // unlink a marker this session never claimed: an inherited report is + // owed to the user until it is shown, and nothing here settles the + // deletion it describes. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let io = FaultyIo::default(); + io.fail_appends.store(true, Ordering::SeqCst); + let hist = hist_with_io(&cp, io.boxed()); + // Both halves, because recovery only ever produces them together: the + // file, and the in-memory record that it is owed to the user. Writing + // the file alone would be a state production cannot reach — the + // session would hold no claim at all — and the projection would then + // be right to unlink it. + plant_marker(&hist, &cp, DeletionBreach::Lost); + hist.inherited_owed.store(true, Ordering::SeqCst); + + hist.apply_records(&[committed("きょう", "今日")]); + let generation = gen_under_wal(&hist); + assert!(generation > 0, "the memory-only raise must move the ledger"); + cover_under_wal(&hist, generation); + + assert_eq!( + marker(&cp), + Some(DeletionBreach::Lost), + "covering learning must not retract a deletion report" + ); + } + + #[test] + fn test_clear_removes_a_marker_it_did_not_raise() { + // A marker from a *previous* session raises nothing in this one, so + // the ledger is zero and the cover early-returns without touching it. + // Only the unconditional wipe in clear_impl removes it — and it must, + // or a full wipe would keep reporting a lost deletion against a + // history that provably holds nothing. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let hist = open_hist(&cp); + plant_marker(&hist, &cp, DeletionBreach::Lost); + + hist.clear_impl().unwrap(); + + assert_eq!(marker(&cp), None, "a wipe must retire a stale marker too"); + } + + #[test] + fn test_clear_removes_a_marker_it_did_raise() { + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let io = FaultyIo::default(); + let hist = hist_with_io(&cp, io.boxed()); + block_checkpoint_write(&cp); + lose_a_deletion(&hist, &io); + assert_eq!(marker(&cp), Some(DeletionBreach::Lost)); + + unblock_checkpoint_write(&cp); + io.fail_appends.store(false, Ordering::SeqCst); + hist.clear_impl().unwrap(); + + assert_eq!(marker(&cp), None); + } + + #[test] + fn test_a_lost_deletion_survives_the_restart_that_resurrects_it() { + // The end-to-end shape of #312, and the only test that exercises the + // writer and the reader against one real file: delete, lose it, close + // the process, reopen. The entry comes back — and this time the + // report comes back with it. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + + { + let io = FaultyIo::default(); + let hist = hist_with_io(&cp, io.boxed()); + // A real checkpoint first, so the reopen below has something to + // load that still holds the entry. + hist.apply_records(&[committed("きょう", "今日")]); + hist.scrub_pending.store(true, Ordering::SeqCst); + hist.run_gated_compact(); + assert!(cp.exists()); + + block_checkpoint_write(&cp); + io.fail_appends.store(true, Ordering::SeqCst); + hist.apply_records(&[deletion("きょう", "今日")]); + assert!(learned(&hist, "きょう").is_empty(), "memory delete runs"); + assert!(hist.has_unpersisted_deletion()); + } + unblock_checkpoint_write(&cp); + + let reopened = open_hist(&cp); + assert_eq!( + learned(&reopened, "きょう"), + vec!["今日".to_string()], + "the deletion did not persist, so the entry is back" + ); + assert!( + reopened.open_report().deletion_lost, + "and the report is back with it — this is the whole of #312" + ); + // No disk recovery retracts it — only a user action does — so it must + // not be on the channel that retracts itself. + assert!( + !reopened + .durability_issues() + .contains(&LexHistoryDurabilityIssue::DeletionNotPersisted), + "a past loss is not a live durability problem" + ); + + // "It appears" is half the property; "it goes away once delivered" is + // the other half. Without the second, a permanently latched row would + // pass the first. + assert_eq!(marker(&cp), Some(DeletionBreach::Lost)); + reopened.ack_open_report(); + assert!( + !reopened.deletion_report_owed(), + "a clean ack retires the report" + ); + assert_eq!(marker(&cp), None); + assert!(!open_hist(&cp).open_report().deletion_lost); + } + + #[test] + fn test_append_failure_keeps_memory_and_heals() { + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let io = FaultyIo::default(); + io.fail_appends.store(true, Ordering::SeqCst); + let hist = hist_with_io(&cp, io.boxed()); + + hist.apply_records(&[committed("きょう", "今日")]); + + // §5.2: memory keeps the record (immediate quality), applied_seq + // does not advance (the frame cannot replay), the WAL freezes, and + // a heal is posted. + assert_eq!(learned(&hist, "きょう"), vec!["今日".to_string()]); + assert_eq!(read_recover(&hist.inner).applied_seq(), 0); + assert!(lock_recover(&hist.wal).is_frozen()); + // F10 (#288): the frozen window is what makes this session's learning + // memory-only, and it is reportable while it lasts — not only after + // the fact via the next startup's OpenReport. + assert_eq!( + hist.durability_issues(), + vec![LexHistoryDurabilityIssue::LearningMemoryOnly] + ); + + // The healing compaction checkpoints memory (covering the effect) + // and re-truncates the WAL back to appendable form. + io.fail_appends.store(false, Ordering::SeqCst); + wait_until( + || !lock_recover(&hist.wal).is_frozen(), + "heal compaction to unfreeze the WAL", + ); + assert!(cp.exists(), "healed checkpoint persists the effect"); + assert!(io.truncates.load(std::sync::atomic::Ordering::SeqCst) >= 1); + // F11: the issue is clearable, not a latch — which is exactly why it + // is polled rather than folded into Swift's `initFailures`. + assert!( + hist.durability_issues().is_empty(), + "the heal must clear the report" + ); + + // Nothing was lost: reopen from disk sees the entry (via checkpoint). + drop(hist); + let hist2 = open_hist(&cp); + assert_eq!(learned(&hist2, "きょう"), vec!["今日".to_string()]); + } + + #[test] + fn test_tombstone_sync_failure_applies_with_real_seq() { + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let io = FaultyIo::default(); + io.fail_full_sync.store(true, Ordering::SeqCst); + let hist = hist_with_io(&cp, io.boxed()); + + hist.apply_records(&[committed("きょう", "今日")]); + let commit_seq = lock_recover(&hist.wal).last_appended_seq(); + hist.apply_records(&[deletion("きょう", "今日")]); + + // The frame is on disk (mock append succeeded); the failed flush + // must not leave it uncovered, or conditional truncation would + // stall until an unrelated append. + assert!( + learned(&hist, "きょう").is_empty(), + "memory delete still runs" + ); + assert!( + read_recover(&hist.inner).applied_seq() > commit_seq, + "tombstone must be covered by its real seq despite the sync failure" + ); + assert!(!lock_recover(&hist.wal).is_frozen()); + // §5.4: because the tombstone's WAL durability failed, the checkpoint + // is written SYNCHRONOUSLY before apply_records returns (no wait for a + // background thread) — the deletion cannot resurrect if a crash + // preempts an async scrub. + assert_eq!( + lock_recover(&hist.wal).entry_count(), + 0, + "durability-failed tombstone must checkpoint synchronously, not via async scrub" + ); + drop(hist); + let hist2 = open_hist(&cp); + assert!( + learned(&hist2, "きょう").is_empty(), + "synchronously-checkpointed deletion survives reopen" + ); + } + + #[test] + fn test_tombstone_append_failure_checkpoints_synchronously() { + // Io on a tombstone append: the frame never reaches disk, so memory + // is the only copy of the deletion. It must be checkpointed + // synchronously before returning — a crash before an async scrub + // would otherwise resurrect the deleted entry from the old checkpoint. + let dir = tempfile::tempdir().unwrap(); + let cp = dir.path().join("history.lxud"); + let io = FaultyIo::default(); + let hist = hist_with_io(&cp, io.boxed()); + + hist.apply_records(&[committed("きょう", "今日")]); + hist.run_compact(); // the entry now lives in the checkpoint + assert!(cp.exists()); + + // The tombstone's frame fails to append (frozen WAL, memory-only delete). + io.fail_appends.store(true, Ordering::SeqCst); + hist.apply_records(&[deletion("きょう", "今日")]); + assert!(learned(&hist, "きょう").is_empty(), "memory delete runs"); + // F1: the deletion *was* raised, and the synchronous fallback + // checkpoint covered it before returning — so nothing is reported. + // The WAL is still frozen, which is its own (accurate) issue. + assert!( + !hist + .durability_issues() + .contains(&LexHistoryDurabilityIssue::DeletionNotPersisted), + "a deletion the fallback persisted must not be reported as lost" + ); + + // Synchronous durability: reopening immediately (no wait for a + // background scrub) must not resurrect the deletion. + drop(hist); + let hist2 = open_hist(&cp); + assert!( + learned(&hist2, "きょう").is_empty(), + "durability-failed tombstone is checkpointed before return, not resurrected" + ); + } + + #[test] + fn test_durability_failure_with_failed_checkpoint_keeps_scrub_pending() { + // R2: if the synchronous durability checkpoint ALSO fails (stuck + // disk), the scrub must stay pending so a later compaction / startup + // retries it — the deletion must not be silently dropped with the + // pending flag consumed. + let dir = tempfile::tempdir().unwrap(); + let cp = blocked_checkpoint(dir.path()); + let io = FaultyIo::default(); + io.fail_full_sync.store(true, Ordering::SeqCst); + let hist = hist_with_io(&cp, io.boxed()); + + // Learn, then delete: the tombstone's sync_full fails (SyncFailed) + // AND the synchronous checkpoint save fails (blocked parent). + hist.apply_records(&[committed("きょう", "今日")]); + hist.apply_records(&[deletion("きょう", "今日")]); + + assert!( + learned(&hist, "きょう").is_empty(), + "memory delete still runs" + ); + assert!( + hist.scrub_pending.load(Ordering::SeqCst), + "a failed durability checkpoint must keep the scrub pending for retry" ); } @@ -2069,7 +3824,7 @@ mod tests { let (generation, _snapshot) = hist.snapshot_to_cover(); raise_under_wal(&hist); // lands after the pair was taken - hist.cover_unpersisted(generation); + cover_under_wal(&hist, generation); assert!( hist.has_unpersisted_deletion(), @@ -2077,7 +3832,7 @@ mod tests { ); // The next compaction, pairing a fresh generation, does clear it. let (generation, _snapshot) = hist.snapshot_to_cover(); - hist.cover_unpersisted(generation); + cover_under_wal(&hist, generation); assert!(!hist.has_unpersisted_deletion()); } @@ -2100,16 +3855,25 @@ mod tests { let observed = gen_under_wal(&hist); // A second deletion fails while the first cover is being computed. raise_under_wal(&hist); - hist.cover_unpersisted(observed); + cover_under_wal(&hist, observed); assert!( hist.has_unpersisted_deletion(), "covering generation 1 must not settle the deletion raised after it" ); + // And the marker must survive with it. This is the only place the + // `raised <= covered` half of the unlink condition is exercised + // against a genuinely stale cover: elsewhere the early return fires + // first, so the condition is never evaluated at all. + assert_eq!( + marker(&cp), + Some(DeletionBreach::Lost), + "a stale cover must not retract a still-outstanding claim" + ); // A stale cover must not un-settle newer work. - hist.cover_unpersisted(gen_under_wal(&hist)); + cover_under_wal(&hist, gen_under_wal(&hist)); assert!(!hist.has_unpersisted_deletion()); - hist.cover_unpersisted(observed); + cover_under_wal(&hist, observed); assert!( !hist.has_unpersisted_deletion(), "a late cover carrying an older generation must not reopen it"