From d0d915496dbe4fa507ab7e090a8b96030956f255 Mon Sep 17 00:00:00 2001 From: Taylor Date: Tue, 20 Oct 2026 18:20:26 -0700 Subject: [PATCH 1/3] fix(daemon): do not stop on a file Windows reports as access denied std::filesystem::exists(path) throws when the path cannot be stat'ed. Windows reports a file that an editor is replacing as "Access is denied", so the enrichment walk (classify_cached_file, is_valid_cached_record) and the watcher's incremental update threw, and the daemon stopped: exit 1 since #179, std::terminate before. Seen on windows-default CI as "enrichment refresh failed: ... exists: Access is denied ... docs\guide.md". Use the error_code overloads. A file that cannot be checked counts as absent for this pass, as the neighbouring file_size/last_write_time checks already treat it; the watcher reports it again once written. Regression tests reproduce the throw on POSIX with an unsearchable parent directory. Co-Authored-By: Claude Opus 5.5 --- src/engine/file_cache.cpp | 7 ++++-- src/engine/incremental_update.cpp | 8 +++++-- src/engine/semantic_chunk_plan.cpp | 3 ++- tests/smoke/file_cache_test.cpp | 23 ++++++++++++++++++ tests/smoke/incremental_update_test.cpp | 32 +++++++++++++++++++++++++ 5 files changed, 68 insertions(+), 5 deletions(-) diff --git a/src/engine/file_cache.cpp b/src/engine/file_cache.cpp index 5a9cbc72..3945c7cd 100644 --- a/src/engine/file_cache.cpp +++ b/src/engine/file_cache.cpp @@ -271,13 +271,16 @@ FileCacheClassification classify_cached_file( const std::filesystem::path& path, std::optional previous, CacheValidation mode) { - if (!std::filesystem::exists(path)) { + // Non-throwing like the stat calls below: Windows reports a file that an + // editor is replacing as "Access is denied", and the caller (the daemon's + // enrichment walk) must see one more transient miss, not an exception. + std::error_code error; + if (!std::filesystem::exists(path, error)) { return FileCacheClassification{.state = CacheState::Deleted}; } FileCacheEntry current; current.path = path; - std::error_code error; current.size = std::filesystem::file_size(path, error); if (error) { return FileCacheClassification{.state = CacheState::Deleted}; diff --git a/src/engine/incremental_update.cpp b/src/engine/incremental_update.cpp index a59aedf1..bbe78875 100644 --- a/src/engine/incremental_update.cpp +++ b/src/engine/incremental_update.cpp @@ -327,13 +327,17 @@ IncrementalUpdateResult apply_incremental_code_updates( } const auto language = detect_language(event.path); - if (language == DetectedLanguage::Unknown || !std::filesystem::exists(event.path)) { + // Non-throwing: a file mid-save reads as "Access is denied" on Windows. It + // counts as absent for this pass; the watcher reports it again once written. + std::error_code exists_error; + const bool present = std::filesystem::exists(event.path, exists_error); + if (language == DetectedLanguage::Unknown || !present) { if (language == DetectedLanguage::Unknown && index.files.contains(key)) { applied = true; changed_sources.push_back(key); continue; } - if (!std::filesystem::exists(event.path) && index.files.contains(key)) { + if (!present && index.files.contains(key)) { applied = true; changed_sources.push_back(key); continue; diff --git a/src/engine/semantic_chunk_plan.cpp b/src/engine/semantic_chunk_plan.cpp index 72cab799..b70e8e43 100644 --- a/src/engine/semantic_chunk_plan.cpp +++ b/src/engine/semantic_chunk_plan.cpp @@ -85,9 +85,10 @@ namespace { } [[nodiscard]] bool is_valid_cached_record(const SemanticCacheRecord& record, std::string_view current_hash) { + std::error_code error; // unreadable counts as missing: the record is re-planned, never thrown past return record.state == SemanticCacheState::Valid && record.content_hash == current_hash && - std::filesystem::exists(record.fragment_path) && + std::filesystem::exists(record.fragment_path, error) && !record.fragment_hash.empty() && sha256_file_hex(record.fragment_path) == record.fragment_hash; } diff --git a/tests/smoke/file_cache_test.cpp b/tests/smoke/file_cache_test.cpp index 25b430f0..0180f772 100644 --- a/tests/smoke/file_cache_test.cpp +++ b/tests/smoke/file_cache_test.cpp @@ -192,6 +192,29 @@ int main() { return 1; } +#if defined(__APPLE__) || defined(__unix__) + // A path whose existence cannot be checked classifies as Deleted instead of + // throwing out of the daemon's enrichment walk. Windows reports a file an + // editor is replacing as "Access is denied"; here an unsearchable parent + // makes the same check fail with EACCES. Root bypasses directory permissions. + if (::geteuid() != 0) { + const auto locked_dir = root / "locked"; + write_file(locked_dir / "doc.md", "# doc\n"); + std::filesystem::permissions(locked_dir, std::filesystem::perms::none, std::filesystem::perm_options::replace); + bool threw = false; + cgraph::FileCacheClassification locked; + try { + locked = cgraph::classify_cached_file(locked_dir / "doc.md", std::nullopt); + } catch (const std::exception&) { + threw = true; + } + std::filesystem::permissions(locked_dir, std::filesystem::perms::owner_all, std::filesystem::perm_options::replace); + if (threw || locked.state != cgraph::CacheState::Deleted || locked.current.has_value()) { + return 1; + } + } +#endif + // === Content validation mode === write_file(path, "print('hello')\n"); diff --git a/tests/smoke/incremental_update_test.cpp b/tests/smoke/incremental_update_test.cpp index 8081ec22..2139dbf9 100644 --- a/tests/smoke/incremental_update_test.cpp +++ b/tests/smoke/incremental_update_test.cpp @@ -14,6 +14,10 @@ #include #include +#if defined(__APPLE__) || defined(__unix__) +#include +#endif + namespace { void write_file(const std::filesystem::path& path, std::string contents) { @@ -107,6 +111,34 @@ int main() { } const auto created_root = graph->content_root; +#if defined(__APPLE__) || defined(__unix__) + // A watcher event for a path whose existence cannot be checked must not + // throw out of the serve loop: Windows reports a file mid-save as "Access is + // denied", and an unsearchable parent fails the same check with EACCES here. + // The file counts as absent for this pass. Root bypasses directory permissions. + if (::geteuid() != 0) { + const auto locked_dir = root / "locked"; + write_file(locked_dir / "gamma.py", "class Gamma:\n pass\n"); + std::filesystem::permissions(locked_dir, std::filesystem::perms::none, std::filesystem::perm_options::replace); + cgraph::FileWatchEvent locked_event[] = { + {.path = locked_dir / "gamma.py", .change = cgraph::FileWatchChange::Modified, + .kind = cgraph::WatchedFileKind::Code}, + }; + bool threw = false; + try { + (void)cgraph::apply_incremental_code_updates(state, index, locked_event); + } catch (const std::exception&) { + threw = true; + } + std::filesystem::permissions(locked_dir, std::filesystem::perms::owner_all, std::filesystem::perm_options::replace); + std::filesystem::remove_all(locked_dir); + if (threw || cgraph::read_graph_snapshot(state) != graph) { + std::cerr << "FAIL: an unreadable watcher path threw or changed the graph\n"; + return 1; + } + } +#endif + // A source that becomes unreadable after a verified publish must not replace // either the live snapshot or its incremental index with unverified state. const auto verified_snapshot = graph; From 903a881b40ff723d0b51fcb7030dc8cd997891e6 Mon Sep 17 00:00:00 2001 From: Taylor Date: Tue, 20 Oct 2026 18:25:57 -0700 Subject: [PATCH 2/3] test(daemon): read daemon-written files with FILE_SHARE_DELETE on Windows daemon_server_test polled graph.json and the stat index with std::ifstream every 20 ms. MSVC opens without FILE_SHARE_DELETE, so the daemon's MoveFileExW replace failed with access denied for its whole 10 s retry window and the test gave up (windows-release flake "incremental state re-persisted to graph.json"). Restart Manager named the test process as the only holder. read_shared opens with FILE_SHARE_DELETE on Windows, so the replace goes through while a read is in flight; POSIX is unchanged. Co-Authored-By: Claude Opus 5.5 --- tests/smoke/daemon_server_test.cpp | 49 ++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/tests/smoke/daemon_server_test.cpp b/tests/smoke/daemon_server_test.cpp index 4bf5588d..f97d6f88 100644 --- a/tests/smoke/daemon_server_test.cpp +++ b/tests/smoke/daemon_server_test.cpp @@ -23,6 +23,14 @@ #include #include #include +#else +#ifndef NOMINMAX +#define NOMINMAX +#endif +#ifndef WIN32_LEAN_AND_MEAN +#define WIN32_LEAN_AND_MEAN +#endif +#include #endif namespace { @@ -47,6 +55,33 @@ void write_file(const std::filesystem::path& path, std::string contents) { std::ofstream(path, std::ios::binary) << contents; } +// Reads a file the daemon may replace at any moment ("" if it cannot be +// opened). On Windows std::ifstream opens without FILE_SHARE_DELETE, which +// makes the daemon's MoveFileExW replace fail with access denied; a test that +// polls graph.json every few milliseconds can starve the replace for longer +// than its retry window. Sharing delete lets the replace through while the +// read keeps the old file's bytes. +std::string read_shared(const std::filesystem::path& path) { +#ifdef _WIN32 + const HANDLE file = ::CreateFileW(path.c_str(), GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); + if (file == INVALID_HANDLE_VALUE) { + return {}; + } + std::string contents; + std::array buffer{}; + DWORD read = 0; + while (::ReadFile(file, buffer.data(), static_cast(buffer.size()), &read, nullptr) && read > 0) { + contents.append(buffer.data(), read); + } + ::CloseHandle(file); + return contents; +#else + std::ifstream input(path, std::ios::binary); + return {std::istreambuf_iterator(input), std::istreambuf_iterator()}; +#endif +} + // Records a failed expectation with a name, so a CI flake says WHICH step // failed instead of a bare non-zero exit. void expect(bool& ok, bool condition, const char* what) { @@ -413,9 +448,7 @@ int main() { // once the persist interval elapses (no update op, no shutdown). bool persisted = false; for (int attempt = 0; attempt < 600 && !persisted; ++attempt) { - std::ifstream input(root / "cgraph-out" / "graph.json"); - const std::string contents((std::istreambuf_iterator(input)), std::istreambuf_iterator()); - persisted = contents.find("gamma") != std::string::npos; + persisted = read_shared(root / "cgraph-out" / "graph.json").find("gamma") != std::string::npos; if (!persisted) { std::this_thread::sleep_for(std::chrono::milliseconds(20)); } @@ -942,8 +975,7 @@ int main() { expect(ok, rc == 0, exited); }; const auto persisted_graph_has_overlay = [&]() { - std::ifstream input(graph_path, std::ios::binary); - const auto graph = nlohmann::json::parse(input, nullptr, false); + const auto graph = nlohmann::json::parse(read_shared(graph_path), nullptr, false); if (!graph.is_object()) { return true; } @@ -1062,8 +1094,7 @@ int main() { "overlay-events: live document deletion removed the invalid overlay"); bool deleted_stat_evicted = false; for (int attempt = 0; attempt < 500 && !deleted_stat_evicted; ++attempt) { - std::ifstream input(stat_index_path, std::ios::binary); - const auto stat_json = nlohmann::json::parse(input, nullptr, false); + const auto stat_json = nlohmann::json::parse(read_shared(stat_index_path), nullptr, false); if (stat_json.is_object()) { deleted_stat_evicted = true; for (const auto& entry : stat_json.value("entries", nlohmann::json::array())) { @@ -1174,8 +1205,7 @@ int main() { cov_server.join(); expect(ok, cov_rc == 0, "coverage: daemon exited cleanly"); - std::ifstream persisted(cov_root / "cgraph-out" / "graph.json", std::ios::binary); - const auto persisted_graph = nlohmann::json::parse(persisted, nullptr, false); + const auto persisted_graph = nlohmann::json::parse(read_shared(cov_root / "cgraph-out" / "graph.json"), nullptr, false); bool overlay_in_persisted = !persisted_graph.is_object(); bool code_in_persisted = false; for (const auto& node : persisted_graph.value("nodes", nlohmann::json::array())) { @@ -1190,7 +1220,6 @@ int main() { expect(ok, !overlay_in_persisted && code_in_persisted, "coverage: graph.json stayed deterministic-only across the refused update"); - persisted.close(); // Windows cannot delete a file that is still open fs::remove_all(cov_root); } From 30c58fe02a5847e35f2931f13d08465cf157da28 Mon Sep 17 00:00:00 2001 From: Taylor Date: Tue, 20 Oct 2026 18:35:30 -0700 Subject: [PATCH 3/3] fix(file-cache): a failed exists() is Unreadable, not Deleted Review of #212: classify_cached_file returned Deleted when exists() failed, so a full stat rescan that met a file mid-save on Windows dropped that file's nodes until the next edit. Unreadable makes every caller keep the last verified entry (full_stat_index_rescan refuses, the incremental path and the chunk planner skip). A clean not-found is still Deleted. incremental_update_test now indexes the file before locking it and checks its nodes survive the unreadable Modified event; a mutant that erases indexed files on a failed check fails it. Drop the redundant block in daemon_server_test (unreadable_file.hpp already includes it). Co-Authored-By: Claude Opus 5.5 --- src/engine/file_cache.cpp | 9 +++---- tests/smoke/daemon_server_test.cpp | 8 ------- tests/smoke/file_cache_test.cpp | 7 +++--- tests/smoke/incremental_update_test.cpp | 32 ++++++++++++++++++------- 4 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/engine/file_cache.cpp b/src/engine/file_cache.cpp index 3945c7cd..437acd2c 100644 --- a/src/engine/file_cache.cpp +++ b/src/engine/file_cache.cpp @@ -271,12 +271,13 @@ FileCacheClassification classify_cached_file( const std::filesystem::path& path, std::optional previous, CacheValidation mode) { - // Non-throwing like the stat calls below: Windows reports a file that an - // editor is replacing as "Access is denied", and the caller (the daemon's - // enrichment walk) must see one more transient miss, not an exception. + // Non-throwing: Windows reports a file that an editor is replacing as + // "Access is denied". A check that fails is Unreadable, not Deleted, so a + // caller holding a verified entry keeps it (a rescan must not drop the + // file's nodes) and the daemon's enrichment walk sees no exception. std::error_code error; if (!std::filesystem::exists(path, error)) { - return FileCacheClassification{.state = CacheState::Deleted}; + return FileCacheClassification{.state = error ? CacheState::Unreadable : CacheState::Deleted}; } FileCacheEntry current; diff --git a/tests/smoke/daemon_server_test.cpp b/tests/smoke/daemon_server_test.cpp index f97d6f88..b3a10299 100644 --- a/tests/smoke/daemon_server_test.cpp +++ b/tests/smoke/daemon_server_test.cpp @@ -23,14 +23,6 @@ #include #include #include -#else -#ifndef NOMINMAX -#define NOMINMAX -#endif -#ifndef WIN32_LEAN_AND_MEAN -#define WIN32_LEAN_AND_MEAN -#endif -#include #endif namespace { diff --git a/tests/smoke/file_cache_test.cpp b/tests/smoke/file_cache_test.cpp index 0180f772..dfacc745 100644 --- a/tests/smoke/file_cache_test.cpp +++ b/tests/smoke/file_cache_test.cpp @@ -193,8 +193,9 @@ int main() { } #if defined(__APPLE__) || defined(__unix__) - // A path whose existence cannot be checked classifies as Deleted instead of - // throwing out of the daemon's enrichment walk. Windows reports a file an + // A path whose existence cannot be checked classifies as Unreadable (so a + // verified entry is kept, not dropped as Deleted) instead of throwing out of + // the daemon's enrichment walk. Windows reports a file an // editor is replacing as "Access is denied"; here an unsearchable parent // makes the same check fail with EACCES. Root bypasses directory permissions. if (::geteuid() != 0) { @@ -209,7 +210,7 @@ int main() { threw = true; } std::filesystem::permissions(locked_dir, std::filesystem::perms::owner_all, std::filesystem::perm_options::replace); - if (threw || locked.state != cgraph::CacheState::Deleted || locked.current.has_value()) { + if (threw || locked.state != cgraph::CacheState::Unreadable || locked.current.has_value()) { return 1; } } diff --git a/tests/smoke/incremental_update_test.cpp b/tests/smoke/incremental_update_test.cpp index 2139dbf9..68d86f56 100644 --- a/tests/smoke/incremental_update_test.cpp +++ b/tests/smoke/incremental_update_test.cpp @@ -112,17 +112,23 @@ int main() { const auto created_root = graph->content_root; #if defined(__APPLE__) || defined(__unix__) - // A watcher event for a path whose existence cannot be checked must not - // throw out of the serve loop: Windows reports a file mid-save as "Access is - // denied", and an unsearchable parent fails the same check with EACCES here. - // The file counts as absent for this pass. Root bypasses directory permissions. + // A watcher event for an indexed file whose existence cannot be checked + // must not throw out of the serve loop, and must not drop the file's nodes: + // Windows reports a file mid-save as "Access is denied", and an unsearchable + // parent fails the same check with EACCES here. Root bypasses directory + // permissions. The file is removed again so the index matches created_root. if (::geteuid() != 0) { const auto locked_dir = root / "locked"; - write_file(locked_dir / "gamma.py", "class Gamma:\n pass\n"); + const auto delta = locked_dir / "delta.py"; + write_file(delta, "class Delta:\n pass\n"); + cgraph::FileWatchEvent delta_created[] = { + {.path = delta, .change = cgraph::FileWatchChange::Created, .kind = cgraph::WatchedFileKind::Code}, + }; + (void)cgraph::apply_incremental_code_updates(state, index, delta_created); + const bool indexed = has_node_label(*cgraph::read_graph_snapshot(state), "Delta"); std::filesystem::permissions(locked_dir, std::filesystem::perms::none, std::filesystem::perm_options::replace); cgraph::FileWatchEvent locked_event[] = { - {.path = locked_dir / "gamma.py", .change = cgraph::FileWatchChange::Modified, - .kind = cgraph::WatchedFileKind::Code}, + {.path = delta, .change = cgraph::FileWatchChange::Modified, .kind = cgraph::WatchedFileKind::Code}, }; bool threw = false; try { @@ -131,9 +137,17 @@ int main() { threw = true; } std::filesystem::permissions(locked_dir, std::filesystem::perms::owner_all, std::filesystem::perm_options::replace); + const bool kept = has_node_label(*cgraph::read_graph_snapshot(state), "Delta"); std::filesystem::remove_all(locked_dir); - if (threw || cgraph::read_graph_snapshot(state) != graph) { - std::cerr << "FAIL: an unreadable watcher path threw or changed the graph\n"; + cgraph::FileWatchEvent delta_deleted[] = { + {.path = delta, .change = cgraph::FileWatchChange::Deleted, .kind = cgraph::WatchedFileKind::Code}, + }; + (void)cgraph::apply_incremental_code_updates(state, index, delta_deleted); + graph = cgraph::read_graph_snapshot(state); + if (!indexed || threw || !kept || has_node_label(*graph, "Delta") || + graph->content_root.sha256 != created_root.sha256) { + std::cerr << "FAIL: an unreadable indexed file threw or lost its nodes (indexed=" << indexed + << " threw=" << threw << " kept=" << kept << ")\n"; return 1; } }