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)); +}