From 02fe37561717ff62d7e4658117acb52b90249aa0 Mon Sep 17 00:00:00 2001 From: ykiko Date: Tue, 7 Jul 2026 12:31:56 +0800 Subject: [PATCH] fix(server): bounded navigation wait and buffer desync detection --- src/server/compiler/compiler.cpp | 13 +++ src/server/compiler/compiler.h | 14 +++ src/server/service/feature_router.cpp | 105 +++++++++++++++++- src/server/service/feature_router.h | 8 ++ src/server/service/query.cpp | 20 +++- src/server/state/session.h | 7 ++ src/server/state/session_store.cpp | 13 +++ src/server/state/session_store.h | 18 +-- .../features/test_navigation_freshness.py | 33 ++++++ .../lifecycle/test_protocol_edges.py | 41 ++++++- tests/unit/server/compiler_tests.cpp | 56 ++++++++++ tests/unit/server/feature_router_tests.cpp | 58 ++++++++++ tests/unit/server/index_query_tests.cpp | 48 ++++++++ tests/unit/server/session_store_tests.cpp | 35 +++++- 14 files changed, 448 insertions(+), 21 deletions(-) create mode 100644 tests/integration/features/test_navigation_freshness.py create mode 100644 tests/unit/server/feature_router_tests.cpp create mode 100644 tests/unit/server/index_query_tests.cpp diff --git a/src/server/compiler/compiler.cpp b/src/server/compiler/compiler.cpp index 365e0b4b3..0118e24e1 100644 --- a/src/server/compiler/compiler.cpp +++ b/src/server/compiler/compiler.cpp @@ -925,6 +925,19 @@ kota::task Compiler::ensure_compiled(std::shared_ptr session) { co_return !session->ast_dirty; } +kota::task Compiler::ensure_compiled_bounded(std::shared_ptr session, + std::chrono::milliseconds timeout) { + if(!session->ast_dirty) { + co_return true; + } + // The losing branch is dropped by when_any: dropping the sleep is + // trivially safe, and dropping the ensure_compiled frame cancels only + // this wait — the compile itself runs detached in compile_tasks. + [[maybe_unused]] auto raced = + co_await kota::when_any(ensure_compiled(session), kota::sleep(timeout)); + co_return !session->ast_dirty; +} + Compiler::RawResult Compiler::forward_query(worker::QueryKind kind, std::shared_ptr session, std::optional position, diff --git a/src/server/compiler/compiler.h b/src/server/compiler/compiler.h index eb105afc3..610b310c3 100644 --- a/src/server/compiler/compiler.h +++ b/src/server/compiler/compiler.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -68,6 +69,19 @@ class Compiler { /// file_index, pch_ref, ast_deps, and publishes diagnostics. kota::task ensure_compiled(std::shared_ptr session); + /// Bounded variant of ensure_compiled for latency-sensitive consumers + /// (index-backed navigation): wait until any compile round settles or + /// the timeout elapses, whichever comes first, and return whether the + /// session is clean. Bounded staleness — the settled round is not + /// guaranteed to be the latest (an invalidation landing mid-flight + /// keeps the flag dirty; see Session::settle_compile), and a timeout + /// abandons only this wait: the detached compile keeps running and + /// settles the session for a later request. Preemption works as for + /// any compile wait — the caller's frame may itself be cancelled by + /// $/cancelRequest, leaving the compile untouched. + kota::task ensure_compiled_bounded(std::shared_ptr session, + std::chrono::milliseconds timeout); + using RawResult = kota::task; /// Forward a query to the stateful worker that holds this file's AST. diff --git a/src/server/service/feature_router.cpp b/src/server/service/feature_router.cpp index ffdf10bfa..cdd3df7b1 100644 --- a/src/server/service/feature_router.cpp +++ b/src/server/service/feature_router.cpp @@ -1,6 +1,7 @@ #include "server/service/feature_router.h" #include +#include #include #include #include @@ -16,6 +17,7 @@ #include "server/compiler/indexer.h" #include "server/protocol/serialize.h" #include "server/protocol/worker.h" +#include "support/logging.h" #include "syntax/completion.h" #include "syntax/include_resolver.h" @@ -25,11 +27,26 @@ namespace clice { using serde_raw = kota::codec::RawValue; +/// How long index-backed navigation waits for a dirty session's compile +/// before degrading to the merged shards. Interactive navigation must not +/// hang behind a slow TU, and the shards are an acceptable bounded-staleness +/// answer. An internal constant by design, not configuration: no user knob +/// can pick a better value than "one perceptible beat". +constexpr static std::chrono::milliseconds navigation_compile_wait{1000}; + /// Error response for feature requests on files with no open session. static kota::ipc::Error document_not_open() { return kota::ipc::Error{kota::ipc::protocol::ErrorCode::InvalidParams, "Document not open"}; } +/// Error response for feature requests on a session whose buffer provably +/// diverged from the client's (see Session::desynced). -32801 is LSP's +/// ContentModified — the spec's "result would describe content the client +/// no longer has" code, which kota's core JSON-RPC enum does not name. +static kota::ipc::Error document_out_of_sync() { + return kota::ipc::Error{-32801, "Document out of sync"}; +} + /// Error response when a call/type hierarchy item cannot be resolved back to /// an indexed symbol. static kota::ipc::Error item_not_resolved(llvm::StringRef kind) { @@ -37,6 +54,19 @@ static kota::ipc::Error item_not_resolved(llvm::StringRef kind) { std::format("Failed to resolve {} item", kind)}; } +kota::task FeatureRouter::await_index_freshness(std::shared_ptr session) { + if(!session || !session->ast_dirty) { + co_return true; + } + if(co_await compiler.ensure_compiled_bounded(session, navigation_compile_wait)) { + co_return true; + } + LOG_DEBUG("Navigation degrades to shards for path_id={}: no fresh compile within {}ms budget", + session->path_id, + navigation_compile_wait.count()); + co_return false; +} + const std::vector* FeatureRouter::find_preamble_links(const Session& session) { if(!session.pch_ref) @@ -74,6 +104,8 @@ std::vector kota::task, kota::ipc::Error> FeatureRouter::document_links(std::shared_ptr session) { + if(session->desynced) + co_return kota::outcome_error(document_out_of_sync()); auto result = co_await compiler.forward_document_links(session); if(!result.has_value()) co_return kota::outcome_error(std::move(result.error())); @@ -101,19 +133,27 @@ kota::task FeatureRouter::definition(std::shared_ptr session, llvm::StringRef path, const protocol::Position& pos) { + if(session && session->desynced) + co_return kota::outcome_error(document_out_of_sync()); + + // Give a dirty session's compile a bounded chance to land so the + // eager answers below see the fresh file index and PCH links. + bool fresh = co_await await_index_freshness(session); + + // An invalid didChange may have desynced the buffer while we waited; + // the worker forward below would serve provably diverged text. + if(session && session->desynced) + co_return kota::outcome_error(document_out_of_sync()); + // Preamble include lines first: they have no symbol occurrence in // the index and are invisible to the worker's AST. Dirty sessions - // skip this — the cached links may describe the pre-edit preamble — - // and retry below once the worker compile refreshed the PCH. + // skip this — the cached links may describe the pre-edit preamble. if(session && !session->ast_dirty) { if(auto directive = resolve_directive_definition(*session, pos); !directive.empty()) { co_return to_raw(directive); } } - // Dirty sessions also skip the eager index query: resolve_cursor - // would fall back to the stale merged shard and could return a - // non-empty hit for pre-edit content, bypassing the compile below. if(!session || !session->ast_dirty) { auto result = index_query.query_relations(path, pos, RelationKind::Definition, session.get()); @@ -124,6 +164,15 @@ kota::task if(!session) co_return kota::outcome_error(document_not_open()); + + // The bounded wait expired: degrade to the (possibly stale) merged + // shard rather than queueing behind the compile a second time — the + // forward below would wait unbounded on the same round. + if(!fresh) { + co_return to_raw( + index_query.query_relations(path, pos, RelationKind::Definition, session.get())); + } + auto raw = co_await compiler.forward_query(worker::QueryKind::GoToDefinition, session, pos); if(raw.has_value() && raw.value().data != "[]" && raw.value().data != "null") { co_return std::move(raw.value()); @@ -148,32 +197,46 @@ kota::task FeatureRouter::RawResult FeatureRouter::hover(std::shared_ptr session, const protocol::Position& position) { + if(session->desynced) + co_return kota::outcome_error(document_out_of_sync()); co_return co_await compiler.forward_query(worker::QueryKind::Hover, session, position); } FeatureRouter::RawResult FeatureRouter::semantic_tokens(std::shared_ptr session) { + if(session->desynced) + co_return kota::outcome_error(document_out_of_sync()); co_return co_await compiler.forward_query(worker::QueryKind::SemanticTokens, session); } FeatureRouter::RawResult FeatureRouter::inlay_hints(std::shared_ptr session, const protocol::Range& range) { + if(session->desynced) + co_return kota::outcome_error(document_out_of_sync()); co_return co_await compiler.forward_query(worker::QueryKind::InlayHints, session, {}, range); } FeatureRouter::RawResult FeatureRouter::folding_range(std::shared_ptr session) { + if(session->desynced) + co_return kota::outcome_error(document_out_of_sync()); co_return co_await compiler.forward_query(worker::QueryKind::FoldingRange, session); } FeatureRouter::RawResult FeatureRouter::document_symbol(std::shared_ptr session) { + if(session->desynced) + co_return kota::outcome_error(document_out_of_sync()); co_return co_await compiler.forward_query(worker::QueryKind::DocumentSymbol, session); } FeatureRouter::RawResult FeatureRouter::code_action(std::shared_ptr session) { + if(session->desynced) + co_return kota::outcome_error(document_out_of_sync()); co_return co_await compiler.forward_query(worker::QueryKind::CodeAction, session); } FeatureRouter::RawResult FeatureRouter::completion(std::shared_ptr session, const protocol::Position& position) { + if(session->desynced) + co_return kota::outcome_error(document_out_of_sync()); auto pause = indexer.scoped_pause(); auto path_id = session->path_id; @@ -235,17 +298,23 @@ FeatureRouter::RawResult FeatureRouter::completion(std::shared_ptr sess FeatureRouter::RawResult FeatureRouter::signature_help(std::shared_ptr session, const protocol::Position& position) { + if(session->desynced) + co_return kota::outcome_error(document_out_of_sync()); auto pause = indexer.scoped_pause(); co_return co_await compiler.forward_build(worker::BuildKind::SignatureHelp, position, session); } FeatureRouter::RawResult FeatureRouter::formatting(std::shared_ptr session) { + if(session->desynced) + co_return kota::outcome_error(document_out_of_sync()); auto pause = indexer.scoped_pause(); co_return co_await compiler.forward_format(session); } FeatureRouter::RawResult FeatureRouter::range_formatting(std::shared_ptr session, const protocol::Range& range) { + if(session->desynced) + co_return kota::outcome_error(document_out_of_sync()); auto pause = indexer.scoped_pause(); co_return co_await compiler.forward_format(session, range); } @@ -254,6 +323,9 @@ FeatureRouter::RawResult FeatureRouter::references(std::shared_ptr sess llvm::StringRef path, const protocol::Position& position, bool include_declaration) { + if(session && session->desynced) + co_return kota::outcome_error(document_out_of_sync()); + co_await await_index_freshness(session); auto locations = index_query.query_relations(path, position, RelationKind::Reference, session.get()); @@ -275,6 +347,9 @@ FeatureRouter::RawResult FeatureRouter::references(std::shared_ptr sess FeatureRouter::RawResult FeatureRouter::declaration(std::shared_ptr session, llvm::StringRef path, const protocol::Position& position) { + if(session && session->desynced) + co_return kota::outcome_error(document_out_of_sync()); + co_await await_index_freshness(session); auto locations = index_query.query_relations(path, position, RelationKind::Declaration, session.get()); auto defs = @@ -288,6 +363,9 @@ FeatureRouter::RawResult FeatureRouter::declaration(std::shared_ptr ses FeatureRouter::RawResult FeatureRouter::type_definition(std::shared_ptr session, llvm::StringRef path, const protocol::Position& position) { + if(session && session->desynced) + co_return kota::outcome_error(document_out_of_sync()); + co_await await_index_freshness(session); co_return to_raw(index_query.query_symbol_targets(path, position, RelationKind::TypeDefinition, @@ -297,6 +375,9 @@ FeatureRouter::RawResult FeatureRouter::type_definition(std::shared_ptr FeatureRouter::RawResult FeatureRouter::implementation(std::shared_ptr session, llvm::StringRef path, const protocol::Position& position) { + if(session && session->desynced) + co_return kota::outcome_error(document_out_of_sync()); + co_await await_index_freshness(session); co_return to_raw(index_query.query_symbol_targets(path, position, RelationKind::Implementation, @@ -307,6 +388,9 @@ FeatureRouter::RawResult FeatureRouter::call_hierarchy_prepare(std::shared_ptrdesynced) + co_return kota::outcome_error(document_out_of_sync()); + co_await await_index_freshness(session); auto info = index_query.lookup_symbol(uri, path, position, session.get()); if(!info) co_return serde_raw{"null"}; @@ -322,6 +406,8 @@ FeatureRouter::RawResult FeatureRouter::call_hierarchy_incoming(std::shared_ptr session, llvm::StringRef path, const protocol::CallHierarchyItem& item) { + if(session && session->desynced) + co_return kota::outcome_error(document_out_of_sync()); auto info = index_query.resolve_hierarchy_item(item.uri, path, item.range, item.data, session.get()); if(!info) @@ -334,6 +420,8 @@ FeatureRouter::RawResult FeatureRouter::call_hierarchy_outgoing(std::shared_ptr session, llvm::StringRef path, const protocol::CallHierarchyItem& item) { + if(session && session->desynced) + co_return kota::outcome_error(document_out_of_sync()); auto info = index_query.resolve_hierarchy_item(item.uri, path, item.range, item.data, session.get()); if(!info) @@ -346,6 +434,9 @@ FeatureRouter::RawResult FeatureRouter::type_hierarchy_prepare(std::shared_ptrdesynced) + co_return kota::outcome_error(document_out_of_sync()); + co_await await_index_freshness(session); auto info = index_query.lookup_symbol(uri, path, position, session.get()); if(!info) co_return serde_raw{"null"}; @@ -362,6 +453,8 @@ FeatureRouter::RawResult FeatureRouter::type_hierarchy_supertypes(std::shared_ptr session, llvm::StringRef path, const protocol::TypeHierarchyItem& item) { + if(session && session->desynced) + co_return kota::outcome_error(document_out_of_sync()); auto info = index_query.resolve_hierarchy_item(item.uri, path, item.range, item.data, session.get()); if(!info) @@ -374,6 +467,8 @@ FeatureRouter::RawResult FeatureRouter::type_hierarchy_subtypes(std::shared_ptr session, llvm::StringRef path, const protocol::TypeHierarchyItem& item) { + if(session && session->desynced) + co_return kota::outcome_error(document_out_of_sync()); auto info = index_query.resolve_hierarchy_item(item.uri, path, item.range, item.data, session.get()); if(!info) diff --git a/src/server/service/feature_router.h b/src/server/service/feature_router.h index 9b713565e..481c6a3ea 100644 --- a/src/server/service/feature_router.h +++ b/src/server/service/feature_router.h @@ -139,6 +139,14 @@ class FeatureRouter { RawResult workspace_symbol(llvm::StringRef query); private: + /// Give a dirty session's compile a bounded chance to land before an + /// index-backed navigation query, so the query sees the fresh file + /// index instead of silently answering from stale merged shards. + /// Returns whether the session is clean afterwards; on timeout (logged) + /// the caller proceeds against the shards — bounded staleness is the + /// deliberate trade: navigation must not hang behind a slow TU. + kota::task await_index_freshness(std::shared_ptr session); + /// The preamble include links of a session's active PCH, or nullptr. const std::vector* find_preamble_links(const Session& session); diff --git a/src/server/service/query.cpp b/src/server/service/query.cpp index 9405ba2e2..4705f5e88 100644 --- a/src/server/service/query.cpp +++ b/src/server/service/query.cpp @@ -24,9 +24,14 @@ namespace lsp = kota::ipc::lsp; void IndexQuery::visit_sessions(SessionVisitor visitor) const { sessions.for_each([&](std::uint32_t path_id, const Session& session) -> bool { - // FIXME: when ast_dirty, consider awaiting recompilation - // instead of silently falling back to MergedIndex. - if(session.file_index && session.symbols && !session.ast_dirty) { + // Dirty sessions are filtered out and their files served from the + // merged shards: position-resolving feature entries give the + // compile a bounded chance to land first (FeatureRouter:: + // await_index_freshness), so a session still dirty here degrades + // to the shards by design (bounded staleness). Desynced buffers + // are excluded outright — their index describes text the user is + // not looking at. + if(session.file_index && session.symbols && !session.ast_dirty && !session.desynced) { return visitor(path_id, session); } return true; @@ -78,9 +83,12 @@ bool IndexQuery::find_symbol_info(index::SymbolHash hash, IndexQuery::CursorHit IndexQuery::resolve_cursor(llvm::StringRef path, const protocol::Position& position, Session* session) { - // FIXME: when ast_dirty, we fall back to MergedIndex which may be staler. - // Consider awaiting the pending recompilation to serve fresher results. - if(session && session->file_index && !session->ast_dirty) { + // The session index is consulted only while fresh; otherwise the merged + // shard answers. Position-resolving feature entries give a dirty + // session's compile a bounded chance to land first (FeatureRouter:: + // await_index_freshness), so reaching the fallback dirty is the + // deliberate bounded-staleness degradation, not an oversight. + if(session && session->file_index && !session->ast_dirty && !session->desynced) { auto map = session->line_map(); auto offset = map.to_offset(position); if(!offset) diff --git a/src/server/state/session.h b/src/server/state/session.h index f8d56d327..427ebea0f 100644 --- a/src/server/state/session.h +++ b/src/server/state/session.h @@ -103,6 +103,13 @@ struct Session { } } + /// The client's buffer and this one have provably diverged: an + /// incremental didChange arrived whose range does not map into the + /// current text. Every derived answer would describe a buffer the user + /// is not looking at, so feature requests are refused until didOpen or + /// a whole-document change installs authoritative content again. + bool desynced = false; + /// Non-null while a compilation is in flight for this file. /// Other queries wait on the event; the compilation task itself /// runs independently and cannot be cancelled by LSP $/cancelRequest. diff --git a/src/server/state/session_store.cpp b/src/server/state/session_store.cpp index 4a2ef5dc7..20136cb3d 100644 --- a/src/server/state/session_store.cpp +++ b/src/server/state/session_store.cpp @@ -48,6 +48,9 @@ void SessionStore::apply_open(Session& session, std::string text, int version) { session.text = std::move(text); session.line_starts = lsp::build_line_starts(session.text); session.generation++; + // didOpen carries the full text: whatever divergence the previous + // buffer had, this content is authoritative again. + session.desynced = false; } void SessionStore::apply_change(Session& session, @@ -61,6 +64,9 @@ void SessionStore::apply_change(Session& session, using T = std::remove_cvref_t; if constexpr(std::is_same_v) { session.text = c.text; + // Full text is authoritative: it re-synchronizes a + // desynced buffer the same way didOpen does. + session.desynced = false; } else { auto& range = c.range; auto map = session.line_map(); @@ -68,6 +74,13 @@ void SessionStore::apply_change(Session& session, auto end = map.to_offset(range.end); if(start && end && *start <= *end) { session.text.replace(*start, *end - *start, c.text); + } else { + // The change's range does not map into the buffer: + // client and server have provably diverged, and + // dropping the edit cannot repair that. Mark the + // session so feature requests refuse to answer from + // untrusted text (see Session::desynced). + session.desynced = true; } } session.line_starts = lsp::build_line_starts(session.text); diff --git a/src/server/state/session_store.h b/src/server/state/session_store.h index 45fe76b9f..1c6172307 100644 --- a/src/server/state/session_store.h +++ b/src/server/state/session_store.h @@ -37,11 +37,14 @@ enum class ResetDepth : std::uint8_t { /// here, and every reader of an open file's text goes through the sessions /// this store hands out. /// -/// Future work: this store does not yet detect buffer desync (client and -/// server drifting out of sync) or bound the number of concurrently open -/// sessions. Those safeguards are left for later. Non-monotonic document -/// versions are warned about at the transport edge, where the protocol -/// context lives. +/// Buffer desync (an incremental change whose range does not map into the +/// buffer) is detected in apply_change and marked on the session; the +/// feature layer refuses to answer from a desynced buffer until didOpen or +/// a whole-document change restores authoritative content. +/// +/// Future work: this store does not yet bound the number of concurrently +/// open sessions. Non-monotonic document versions are warned about at the +/// transport edge, where the protocol context lives. struct SessionStore { llvm::DenseMap> sessions; @@ -66,8 +69,9 @@ struct SessionStore { /// Apply a didChange: fold the content changes into the buffer (range → /// offset mapping, in-place text replacement, line-start rebuild), then - /// bump version, generation and mark the AST dirty. Changes whose range - /// cannot be mapped to a valid offset span are silently dropped. + /// bump version, generation and mark the AST dirty. A change whose + /// range cannot be mapped to a valid offset span is dropped and marks + /// the session desynced; a whole-document change re-synchronizes it. void apply_change(Session& session, llvm::ArrayRef changes, int version); diff --git a/tests/integration/features/test_navigation_freshness.py b/tests/integration/features/test_navigation_freshness.py new file mode 100644 index 000000000..9137bc23d --- /dev/null +++ b/tests/integration/features/test_navigation_freshness.py @@ -0,0 +1,33 @@ +"""Navigation on a freshly edited buffer: the bounded compile wait must let +the fresh file index answer instead of silently using pre-edit shards.""" + +import asyncio + +import pytest + +from tests.integration.utils.workspace import did_change + + +@pytest.mark.workspace("hello_world") +async def test_definition_after_edit(client, workspace): + uri, content = await client.open_and_wait(workspace / "main.cpp") + + # Prepend two lines: only the freshly compiled index can produce the + # moved definition location; a stale shard answer is empty here. + did_change(client, uri, 2, "// one\n// two\n" + content) + + # A slow runner may see the degraded (empty) answer; retry until the + # compile settles. + locations = [] + for _ in range(60): + result = await client.definition_at(uri, 11, 17) + locations = result if isinstance(result, list) else [result] if result else [] + if locations: + break + await asyncio.sleep(0.3) + assert locations, "definition never resolved" + + assert all(loc.range.start.line == 4 for loc in locations), ( + f"expected the post-edit definition line 4, got" + f" {[(loc.uri, loc.range.start.line) for loc in locations]}" + ) diff --git a/tests/integration/lifecycle/test_protocol_edges.py b/tests/integration/lifecycle/test_protocol_edges.py index bd09ac2b3..df6e1c3da 100644 --- a/tests/integration/lifecycle/test_protocol_edges.py +++ b/tests/integration/lifecycle/test_protocol_edges.py @@ -5,7 +5,16 @@ import asyncio import pytest -from lsprotocol.types import ClientCapabilities, InitializedParams, InitializeParams +from lsprotocol.types import ( + ClientCapabilities, + DidChangeTextDocumentParams, + InitializedParams, + InitializeParams, + Position, + Range, + TextDocumentContentChangePartial, + VersionedTextDocumentIdentifier, +) from tests.conftest import check_no_anomaly, shutdown_client from tests.integration.utils.assertions import get_errors, guidance_messages @@ -163,6 +172,36 @@ async def test_no_stale_replay(request, executable, tmp_path): check_no_anomaly(request, c) +@pytest.mark.workspace("hello_world") +async def test_desync_error_and_recovery(client, workspace): + uri, content = await client.open_and_wait(workspace / "main.cpp") + + # An out-of-range incremental change proves the buffers diverged. + client.text_document_did_change( + DidChangeTextDocumentParams( + text_document=VersionedTextDocumentIdentifier(uri=uri, version=2), + content_changes=[ + TextDocumentContentChangePartial( + range=Range( + start=Position(line=9999, character=0), + end=Position(line=9999, character=1), + ), + text="x", + ) + ], + ) + ) + with pytest.raises(Exception, match="out of sync"): + await client.hover_at(uri, 2, 4) + with pytest.raises(Exception, match="out of sync"): + await client.definition_at(uri, 9, 17) + + # A whole-document change restores authoritative content. + did_change(client, uri, 3, content) + hover = await client.hover_at(uri, 2, 4) + assert hover is not None + + async def test_startup_guidance_delivered(request, executable, tmp_path): ws = tmp_path write_source(ws, "main.cpp", "int x = 1;\n") diff --git a/tests/unit/server/compiler_tests.cpp b/tests/unit/server/compiler_tests.cpp index 5b87ec7fa..4ab98b43f 100644 --- a/tests/unit/server/compiler_tests.cpp +++ b/tests/unit/server/compiler_tests.cpp @@ -1,3 +1,5 @@ +#include +#include #include #include @@ -64,6 +66,60 @@ TEST_CASE(EpochGuardsPchWrite) { EXPECT_EQ(session.pch_ref->key, std::string("key")); } +TEST_CASE(BoundedWaitTimesOut) { + kota::event_loop loop; + Workspace workspace; + ContextResolver contexts(workspace); + WorkerPool pool(loop); + Compiler compiler(loop, workspace, contexts, pool); + + auto session = std::make_shared(); + session->path_id = workspace.path_pool.intern("/proj/a.cpp"); + session->ast_dirty = true; + // An in-flight compile that never settles: the bounded wait must give + // up instead of queueing behind it indefinitely. + auto pending = std::make_shared(); + pending->generation = session->generation; + session->compiling = pending; + + bool fresh = true; + auto body = [&]() -> kota::task<> { + fresh = co_await compiler.ensure_compiled_bounded(session, std::chrono::milliseconds(50)); + }; + auto task = body(); + loop.schedule(task); + loop.run(); + + EXPECT_FALSE(fresh); + EXPECT_TRUE(session->ast_dirty); + // Only the wait was abandoned; the in-flight compile is untouched. + EXPECT_EQ(session->compiling, pending); +} + +TEST_CASE(BoundedWaitFastPath) { + kota::event_loop loop; + Workspace workspace; + ContextResolver contexts(workspace); + WorkerPool pool(loop); + Compiler compiler(loop, workspace, contexts, pool); + + auto session = std::make_shared(); + session->path_id = workspace.path_pool.intern("/proj/a.cpp"); + session->ast_dirty = false; + + bool fresh = false; + auto body = [&]() -> kota::task<> { + fresh = co_await compiler.ensure_compiled_bounded(session, std::chrono::milliseconds(50)); + }; + auto task = body(); + loop.schedule(task); + loop.run(); + + EXPECT_TRUE(fresh); + // A clean session neither waits nor launches a compile. + EXPECT_EQ(session->compiling, nullptr); +} + }; // TEST_SUITE(CompilerGuards) } // namespace diff --git a/tests/unit/server/feature_router_tests.cpp b/tests/unit/server/feature_router_tests.cpp new file mode 100644 index 000000000..1edbf70c6 --- /dev/null +++ b/tests/unit/server/feature_router_tests.cpp @@ -0,0 +1,58 @@ +#include +#include + +#include "test/test.h" +#include "server/compiler/compiler.h" +#include "server/compiler/context_resolver.h" +#include "server/compiler/indexer.h" +#include "server/service/feature_router.h" +#include "server/state/session_store.h" + +namespace clice::testing { +namespace { + +TEST_SUITE(FeatureRouter) { + +TEST_CASE(DefinitionDegradesOnTimeout) { + kota::event_loop loop; + Workspace workspace; + SessionStore store; + ContextResolver contexts(workspace); + WorkerPool pool(loop); + Compiler compiler(loop, workspace, contexts, pool); + IndexQuery index_query(workspace, store); + Indexer indexer(loop, workspace, pool, contexts, store); + FeatureRouter router(compiler, index_query, workspace, contexts, indexer); + + auto session = store.open(workspace.path_pool.intern("/proj/a.cpp")); + store.apply_open(*session, "int x;\n", 1); + // A compile that never settles: definition must return the (empty) + // shard answer once the bounded wait expires instead of queueing + // behind the worker forward without a bound. + auto pending = std::make_shared(); + pending->generation = session->generation; + session->compiling = pending; + + bool done = false; + auto body = [&]() -> kota::task<> { + auto raw = co_await router.definition(session, + "/proj/a.cpp", + protocol::Position{.line = 0, .character = 4}); + CO_ASSERT_TRUE(raw.has_value()); + EXPECT_EQ(raw.value().data, std::string("[]")); + done = true; + }; + auto task = body(); + loop.schedule(task); + loop.run(); + + ASSERT_TRUE(done); + EXPECT_TRUE(session->ast_dirty); + EXPECT_EQ(session->compiling, pending); +} + +}; // TEST_SUITE(FeatureRouter) + +} // namespace + +} // namespace clice::testing diff --git a/tests/unit/server/index_query_tests.cpp b/tests/unit/server/index_query_tests.cpp new file mode 100644 index 000000000..8931febe9 --- /dev/null +++ b/tests/unit/server/index_query_tests.cpp @@ -0,0 +1,48 @@ +#include +#include + +#include "test/test.h" +#include "server/service/query.h" +#include "server/state/session_store.h" + +namespace clice::testing { +namespace { + +TEST_SUITE(IndexQuery) { + +TEST_CASE(VisitSkipsUnusableSessions) { + Workspace workspace; + SessionStore store; + IndexQuery query(workspace, store); + + auto make_indexed = [&](std::uint32_t path_id) { + auto session = store.open(path_id); + session->file_index.emplace(); + session->symbols.emplace(); + session->ast_dirty = false; + return session; + }; + + auto clean = make_indexed(1); + auto dirty = make_indexed(2); + dirty->ast_dirty = true; + auto desynced = make_indexed(3); + desynced->desynced = true; + + // Only the clean, in-sync session may serve cross-file queries; the + // dirty one degrades to shards and the desynced one's index describes + // text the user is not looking at. + std::vector visited; + query.visit_sessions([&](std::uint32_t path_id, const Session&) -> bool { + visited.push_back(path_id); + return true; + }); + + ASSERT_EQ(visited, std::vector{clean->path_id}); +} + +}; // TEST_SUITE(IndexQuery) + +} // namespace + +} // namespace clice::testing diff --git a/tests/unit/server/session_store_tests.cpp b/tests/unit/server/session_store_tests.cpp index e247ba81a..ba62f8168 100644 --- a/tests/unit/server/session_store_tests.cpp +++ b/tests/unit/server/session_store_tests.cpp @@ -83,13 +83,14 @@ TEST_CASE(WholeDocumentReplace) { ASSERT_EQ(session->line_starts, lsp::build_line_starts(session->text)); } -TEST_CASE(InvalidRangeDropped) { +TEST_CASE(InvalidRangeMarksDesync) { SessionStore store; auto session = store.open(1); store.apply_open(*session, "short\n", 1); // A range past the end of the document cannot be mapped to offsets; - // the change is silently dropped but version bookkeeping still runs. + // the change is dropped, version bookkeeping still runs, and the + // session is marked desynced (the buffers provably diverged). auto change = partial_change(99, 0, 99, 1, "junk"); store.apply_change(*session, change, 2); @@ -97,6 +98,36 @@ TEST_CASE(InvalidRangeDropped) { ASSERT_EQ(session->version, 2); ASSERT_TRUE(session->ast_dirty); ASSERT_EQ(session->generation, 2u); + ASSERT_TRUE(session->desynced); +} + +TEST_CASE(DesyncRecovery) { + SessionStore store; + auto session = store.open(1); + store.apply_open(*session, "short\n", 1); + auto bad = partial_change(99, 0, 99, 1, "junk"); + + // A valid incremental change cannot repair the divergence: the buffer + // it edits is already wrong. + store.apply_change(*session, bad, 2); + ASSERT_TRUE(session->desynced); + auto valid = partial_change(0, 0, 0, 0, "x"); + store.apply_change(*session, valid, 3); + ASSERT_TRUE(session->desynced); + + // A whole-document change carries authoritative text: it recovers. + protocol::TextDocumentContentChangeEvent full = + protocol::TextDocumentContentChangeWholeDocument{.text = "fresh\n"}; + store.apply_change(*session, full, 4); + ASSERT_FALSE(session->desynced); + ASSERT_EQ(session->text, "fresh\n"); + + // ... and so does didOpen. + store.apply_change(*session, bad, 5); + ASSERT_TRUE(session->desynced); + store.apply_open(*session, "opened\n", 6); + ASSERT_FALSE(session->desynced); + ASSERT_EQ(session->text, "opened\n"); } TEST_CASE(ReopenBumpsGeneration) {