From 78d78f6f546fcec53b3eebbd62d78e6f4fc4462e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 08:16:56 +0000 Subject: [PATCH] Swap: tell the user when a journal is locked out or failing; long names - SwapManager queues user notices when a journal is held by another kte (this session's edits are then not journaled) and, once per gap, when journal writes fail. Editor shows them in the status line from its per-frame hook. Both went only to the error log, so crash recovery was silently off or incomplete. - The fallback swap name '..swp' exceeded NAME_MAX for basenames over ~233 bytes, so such files were never journaled; the basename is shortened (on a UTF-8 boundary) only in that case. - Audit doc: sixth-round findings. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB --- Editor.cc | 5 +- Swap.cc | 51 ++++++++++++++-- Swap.h | 11 ++++ docs/audits/claude-findings-20260923.md | 20 +++++++ tests/test_audit_regressions.cc | 77 +++++++++++++++++++++++++ 5 files changed, 159 insertions(+), 5 deletions(-) diff --git a/Editor.cc b/Editor.cc index b84c67a..b7f87fb 100644 --- a/Editor.cc +++ b/Editor.cc @@ -495,8 +495,11 @@ Editor::ProcessPendingOpens() try { // Also the once-per-frame hook for the journal: retry the // checkpoint of a journal that lost a record. - if (kte::SwapManager *sm = Swap()) + if (kte::SwapManager *sm = Swap()) { sm->RetryGapCheckpoints(); + if (std::string notice = sm->TakeUserNotice(); !notice.empty()) + SetStatus(notice); + } return process_pending_opens_(); } catch (const std::exception &e) { kte::ErrorHandler::Instance().Error("Editor", std::string("open failed: ") + e.what(), ""); diff --git a/Swap.cc b/Swap.cc index e076132..786d9cf 100644 --- a/Swap.cc +++ b/Swap.cc @@ -202,8 +202,18 @@ compute_swap_path_for_filename(const std::string &filename) } // Fallback: stable, shorter name based on basename + hash. - std::string base = p.filename().string(); - const std::string name = base + "." + hex_u64(fnv1a64(key)) + ".swp"; + std::string base = p.filename().string(); + const std::string suffix = "." + hex_u64(fnv1a64(key)) + ".swp"; + // A long basename made even this name exceed NAME_MAX (255), so the + // journal could never be created. Shorten only such names (the hash + // keeps them distinct), on a UTF-8 boundary. + if (base.size() + suffix.size() > 255) { + std::size_t keep = 255 - suffix.size(); + while (keep > 0 && (static_cast(base[keep]) & 0xC0) == 0x80) + --keep; + base.resize(keep); + } + const std::string name = base + suffix; return (root / name).string(); } } @@ -439,6 +449,7 @@ SwapManager::ResetJournal(Buffer &buf) was_locked_out = ctx.locked_out; ctx.header_ok = false; ctx.gap = false; // the journal restarts from the saved file + ctx.gap_notified = false; ctx.gap_chkpt_request_ns = 0; ctx.gap_unfixable_reported = false; ctx.locked_out = false; @@ -587,6 +598,7 @@ SwapManager::NotifyFilenameChanged(Buffer &buf) ctx.gap = false; ctx.gap_chkpt_request_ns = 0; ctx.gap_unfixable_reported = false; + ctx.gap_notified = false; ctx.locked_out = false; apply_base(ctx, base); ctx.path = new_path; @@ -1193,6 +1205,26 @@ SwapManager::RetryGapCheckpoints() } +std::string +SwapManager::TakeUserNotice() +{ + std::lock_guard lg(mtx_); + if (user_notices_.empty()) + return {}; + std::string msg = std::move(user_notices_.front()); + user_notices_.pop_front(); + return msg; +} + + +void +SwapManager::notify_user_locked_(std::string msg) +{ + if (user_notices_.size() < 8) + user_notices_.push_back(std::move(msg)); +} + + void SwapManager::RecordCheckpoint(Buffer &buf, const bool urgent_flush) { @@ -1327,8 +1359,16 @@ SwapManager::process_one(const Pending &p) auto mark_gap = [&]() { std::lock_guard lg(mtx_); auto it = journals_.find(p.buf); - if (it != journals_.end()) + if (it != journals_.end()) { it->second.gap = true; + if (!it->second.gap_notified) { + it->second.gap_notified = true; + const std::string &jp = it->second.path; + notify_user_locked_("Crash-recovery journal write failed" + + (jp.empty() ? std::string() : " (" + jp + ")") + + "; recent edits may not be recoverable"); + } + } }; // Check circuit breaker before processing @@ -1399,6 +1439,8 @@ SwapManager::process_one(const Pending &p) { std::lock_guard lg(mtx_); ctxp->locked_out = true; + notify_user_locked_("File is being edited in another kte; crash recovery is off here (" + + path + ")"); } report_error(open_err, p.buf); return; // not an I/O failure; later records are skipped @@ -1467,7 +1509,8 @@ SwapManager::process_one(const Pending &p) } if (p.type == SwapRecType::CHKPT) { std::lock_guard lg(mtx_); - ctxp->gap = false; + ctxp->gap = false; + ctxp->gap_notified = false; } ctxp->approx_size_bytes += static_cast(rec.size()); if (p.urgent_flush) { diff --git a/Swap.h b/Swap.h index f5e53c1..bad9ba3 100644 --- a/Swap.h +++ b/Swap.h @@ -87,6 +87,12 @@ public: // before the user stopped typing would stay behind indefinitely. void RetryGapCheckpoints(); + // Main thread: the oldest pending message for the user (a journal that + // another kte holds, or that fell behind because writes failed), or "" + // when there is none. Such failures otherwise went only to the error + // log, leaving crash recovery silently off or incomplete. + std::string TakeUserNotice(); + void SetConfig(const SwapConfig &cfg) { @@ -219,6 +225,7 @@ private: // position-based, so later ones no longer apply to the journal's state: // drop them until a checkpoint re-establishes the full content. bool gap{false}; + bool gap_notified{false}; // the user was told about the current gap std::uint64_t gap_chkpt_request_ns{0}; bool gap_unfixable_reported{false}; // gap on a buffer too large to checkpoint // Another kte process holds this journal's lock: do not write to it. @@ -316,6 +323,10 @@ private: // Error tracking (protected by mtx_) std::deque errors_; // bounded to max 100 entries + std::deque user_notices_; // see TakeUserNotice(); guarded by mtx_ + + // Queue a message for the user (mtx_ must be held). + void notify_user_locked_(std::string msg); std::size_t total_error_count_{0}; // Circuit breaker for swap operations (protected by mtx_) diff --git a/docs/audits/claude-findings-20260923.md b/docs/audits/claude-findings-20260923.md index 64ac7f7..3cd3f14 100644 --- a/docs/audits/claude-findings-20260923.md +++ b/docs/audits/claude-findings-20260923.md @@ -444,6 +444,23 @@ before the fix (unless noted). passed under a correctly instrumented ASan build with leak checks, and the stress fuzzers ran with ASan and UBSan.) +**Sixth round (real terminal, file I/O, recovery end to end)** +- UTF-8 typed or pasted over a slow link lost characters: with the + input batch read at zero timeout, ncurses gave up on a character split + across reads and discarded the rest as invalid. kte now assembles + UTF-8 itself and waits for continuation bytes. (This regression came + from this audit's own input batching.) +- Visual-line (multi-line) insert, delete, backspace and newline used + the cursor's byte column on every selected line, splitting multibyte + characters on the others; the column is now carried in characters. +- Journal failures reached only the error log: a session locked out of + the journal by another kte (which only happens when the other opened + the file before this one's first edit), or whose journal writes + failed, silently had no or partial crash recovery. The status line now + says so (once per failure). +- Basenames over about 233 bytes produced a swap name over NAME_MAX, so + such files were never journaled. + **Known limitations (not fixed)** - Catastrophic regex backtracking (e.g. `(a*)*b`) can still take very long; std::regex has no time limit. @@ -452,6 +469,9 @@ before the fix (unless noted). - GUI-only items B7, B8, B10 and highlighter edge cases B13 are unchanged; the ImGui and Qt frontends could not be built in the review environment (changed GUI files were syntax-checked where possible). +- Journals are created at the first edit, so a second kte that opens + the file before the first one edits it is not warned at open; the + session that is locked out is told at its first edit instead. - A journal whose buffer exceeds 16 MiB cannot be checkpointed; after a lost record such a journal stays incomplete until the file is saved (reported to the user). diff --git a/tests/test_audit_regressions.cc b/tests/test_audit_regressions.cc index 762861e..ef6e19f 100644 --- a/tests/test_audit_regressions.cc +++ b/tests/test_audit_regressions.cc @@ -1441,3 +1441,80 @@ TEST(Audit_VisualLine_EditsKeepUtf8Whole) ASSERT_EQ(h.Text(), std::string("a\nbcdef\n\xE4\xB8\x96\n\xE7\x95\x8C\n")); } } + + +// A session that cannot journal its buffer (another kte holds the journal, +// or writes fail) now says so in the status line; it used to go only to +// the error log, leaving crash recovery silently off. +TEST(Audit_Swap_FailuresReachTheUser) +{ + TempDir d("swap_notice"); + const std::string path = (d.path / "f.txt").string(); + std::ofstream(path) << "base\n"; + const std::string swp = kte::SwapManager::ComputeSwapPathForFilename(path); + std::filesystem::remove_all(swp); + + // Session A holds the journal; session B is locked out. + Buffer a, b; + std::string err; + ASSERT_TRUE(a.OpenFromFile(path, err)); + ASSERT_TRUE(b.OpenFromFile(path, err)); + kte::SwapManager sa, sb; + sa.Attach(&a); + a.SetSwapRecorder(sa.RecorderFor(&a)); + a.insert_text(0, 0, "A"); + sa.Flush(&a); + ASSERT_EQ(sa.TakeUserNotice(), std::string()); + sb.Attach(&b); + b.SetSwapRecorder(sb.RecorderFor(&b)); + b.insert_text(0, 0, "B"); + sb.Flush(&b); + ASSERT_TRUE(sb.TakeUserNotice().find("another kte") != std::string::npos); + ASSERT_EQ(sb.TakeUserNotice(), std::string()); + b.SetSwapRecorder(nullptr); + sb.Detach(&b, false); + a.SetSwapRecorder(nullptr); + sa.Detach(&a, true); + + // Writes fail: told once, not per keystroke. + std::filesystem::remove_all(swp); + std::filesystem::create_directories(swp); + Buffer c; + ASSERT_TRUE(c.OpenFromFile(path, err)); + kte::SwapManager sc; + sc.Attach(&c); + c.SetSwapRecorder(sc.RecorderFor(&c)); + c.insert_text(0, 0, "C"); + c.insert_text(0, 1, "D"); + sc.Flush(&c); + ASSERT_TRUE(sc.TakeUserNotice().find("journal write failed") != std::string::npos); + ASSERT_EQ(sc.TakeUserNotice(), std::string()); + c.SetSwapRecorder(nullptr); + sc.Detach(&c, false); + std::filesystem::remove_all(swp); +} + + +// A file whose basename is too long for the encoded swap name fell back to +// "..swp", which exceeded NAME_MAX: no journal at all. +TEST(Audit_Swap_LongBasenameStillJournaled) +{ + TempDir d("swap_longname"); + const std::string path = (d.path / (std::string(240, 'n') + ".txt")).string(); + std::ofstream(path) << "base\n"; + const std::string swp = kte::SwapManager::ComputeSwapPathForFilename(path); + ASSERT_TRUE(std::filesystem::path(swp).filename().string().size() <= 255); + std::filesystem::remove(swp); + Buffer a; + std::string err; + ASSERT_TRUE(a.OpenFromFile(path, err)); + kte::SwapManager sm; + sm.Attach(&a); + a.SetSwapRecorder(sm.RecorderFor(&a)); + a.insert_text(0, 0, "X"); + sm.Flush(&a); + ASSERT_TRUE(std::filesystem::exists(swp)); + a.SetSwapRecorder(nullptr); + sm.Detach(&a, true); + ASSERT_TRUE(!std::filesystem::exists(swp)); +}