From 7299ae9a072aae7993a7cc8936c74e12a98b4e22 Mon Sep 17 00:00:00 2001 From: ykiko Date: Fri, 12 Jun 2026 11:16:08 +0800 Subject: [PATCH 1/6] chore(deps): bump kotatsu for TOML error locations Pulls clice-io/kotatsu#168: TOML syntax errors carry description and line/column in no-exceptions builds, and unknown-field errors get the offending node's location. --- cmake/package.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/package.cmake b/cmake/package.cmake index b3bc885e..d7d0e6d3 100644 --- a/cmake/package.cmake +++ b/cmake/package.cmake @@ -41,7 +41,7 @@ set(FLATBUFFERS_BUILD_FLATHASH OFF CACHE BOOL "" FORCE) FetchContent_Declare( kotatsu GIT_REPOSITORY https://github.com/clice-io/kotatsu - GIT_TAG 2a8c147579d36d33ff50f31d0bce49b0ab67381b + GIT_TAG 60661c15e0a13e29e59443b470e83c3b2b60fb0d ) set(KOTA_ENABLE_ZEST ON) From a30f3d324f1ea6dd47958c99f53a5667c25cd2e3 Mon Sep 17 00:00:00 2001 From: ykiko Date: Fri, 12 Jun 2026 11:16:08 +0800 Subject: [PATCH 2/6] feat(support): anomaly and guidance reporting channels LOG_ANOMALY(id, ...) is a soft assertion for internal invariants: Debug builds abort after logging (CLICE_ANOMALY_NO_TRAP escapes for tests), Release builds log with a stable [anomaly:] marker, forward it to a notify hook (master: window/logMessage) and continue. Reports are rate-limited per ID; the gate runs before format arguments are evaluated, so suppressed reports cost nothing. LOG_GUIDANCE(...) is the user-actionable channel ([guidance] marker, Warning logMessage) for situations that are not clice bugs. Unit tests lock the lazy-evaluation contract, rate limiting, markers and the trap mock point. --- src/support/anomaly.cpp | 108 ++++++++++++++++++++ src/support/anomaly.h | 110 ++++++++++++++++++++ tests/unit/support/anomaly_tests.cpp | 147 +++++++++++++++++++++++++++ 3 files changed, 365 insertions(+) create mode 100644 src/support/anomaly.cpp create mode 100644 src/support/anomaly.h create mode 100644 tests/unit/support/anomaly_tests.cpp diff --git a/src/support/anomaly.cpp b/src/support/anomaly.cpp new file mode 100644 index 00000000..488cc5e6 --- /dev/null +++ b/src/support/anomaly.cpp @@ -0,0 +1,108 @@ +#include "support/anomaly.h" + +#include +#include +#include + +#include "support/logging.h" + +#include "llvm/Support/Process.h" + +namespace clice::logging { + +namespace { + +std::array, anomaly_id_count> report_counts; + +std::function notify_hook; + +std::function testing_trap; + +[[maybe_unused]] bool trap_disabled_by_env() { + static bool disabled = llvm::sys::Process::GetEnv("CLICE_ANOMALY_NO_TRAP").has_value(); + return disabled; +} + +void trap(AnomalyId id) { + if(testing_trap) { + testing_trap(id); + return; + } +#ifndef NDEBUG + /// Debug builds treat anomalies as assertion failures: flush the log so + /// the report survives, then abort. CLICE_ANOMALY_NO_TRAP exists for + /// integration tests that intentionally trigger anomalies and verify the + /// Release behavior (report and continue). + if(!trap_disabled_by_env()) { + spdlog::shutdown(); + std::abort(); + } +#endif +} + +} // namespace + +std::string_view anomaly_name(AnomalyId id) { + switch(id) { + case AnomalyId::PchBuildFail: return "pch_build_fail"; + case AnomalyId::PcmBuildFail: return "pcm_build_fail"; + case AnomalyId::CompileFail: return "compile_fail"; + case AnomalyId::WorkerRequestFail: return "worker_request_fail"; + case AnomalyId::WorkerCrash: return "worker_crash"; + case AnomalyId::WorkerSpawnFail: return "worker_spawn_fail"; + case AnomalyId::PositionMapFail: return "position_map_fail"; + } + return "unknown"; +} + +bool anomaly_should_report(AnomalyId id) { + if(options.level > Level::err) + return false; + + auto& count = report_counts[static_cast(id)]; + auto previous = count.fetch_add(1, std::memory_order_relaxed); + if(previous < anomaly_report_limit) + return true; + + if(previous == anomaly_report_limit) { + logging::err("[anomaly:{}] report limit ({}) reached, suppressing further reports", + anomaly_name(id), + anomaly_report_limit); + } + return false; +} + +void report_anomaly(AnomalyId id, std::string message, std::source_location location) { + auto text = std::format("[anomaly:{}] {}", anomaly_name(id), message); + logging::log(spdlog::level::err, location, "{}", text); + if(notify_hook) + notify_hook(NotifyLevel::Error, text); + trap(id); +} + +bool guidance_should_report() { + return options.level <= Level::warn; +} + +void report_guidance(std::string message, std::source_location location) { + auto text = std::format("[guidance] {}", message); + logging::log(spdlog::level::warn, location, "{}", text); + if(notify_hook) + notify_hook(NotifyLevel::Warning, text); +} + +void set_notify_hook(std::function hook) { + notify_hook = std::move(hook); +} + +void set_anomaly_trap_for_testing(std::function trap) { + testing_trap = std::move(trap); +} + +void reset_anomaly_for_testing() { + for(auto& count: report_counts) + count.store(0, std::memory_order_relaxed); + testing_trap = nullptr; +} + +} // namespace clice::logging diff --git a/src/support/anomaly.h b/src/support/anomaly.h new file mode 100644 index 00000000..540790a9 --- /dev/null +++ b/src/support/anomaly.h @@ -0,0 +1,110 @@ +#pragma once + +#include +#include +#include +#include +#include +#include + +namespace clice::logging { + +/// Stable identifiers for anomalies — internal states that should be +/// unreachable when clice works correctly. Reaching one indicates a bug in +/// clice itself, NOT a problem with user input or user code: those are +/// reported as LSP errors, guidance messages or plain logs instead. +/// +/// Debug builds abort on the first occurrence so the CI Debug matrix and +/// local development surface the bug as early as possible. Release builds +/// log the occurrence with an `[anomaly:]` marker, forward it to the +/// notify hook (master process: `window/logMessage`) and continue running. +enum class AnomalyId : std::uint8_t { + /// PCH build failed without a user-code error to explain it. + PchBuildFail, + /// PCM build failed without a user-code error to explain it. + PcmBuildFail, + /// AST compile request to a stateful worker failed at the IPC layer. + CompileFail, + /// Feature query/build forwarded to a worker failed at the IPC layer. + WorkerRequestFail, + /// A worker process died unexpectedly. + WorkerCrash, + /// Worker pool failed to spawn or respawn a worker process. + WorkerSpawnFail, + /// An internally-produced offset or range failed to map to a position. + PositionMapFail, +}; + +constexpr inline std::size_t anomaly_id_count = + static_cast(AnomalyId::PositionMapFail) + 1; + +/// Stable snake_case name used in the `[anomaly:]` marker. Integration +/// tests match on these strings — renaming an enumerator must not silently +/// change them. +std::string_view anomaly_name(AnomalyId id); + +/// Per-ID report cap per process; further occurrences are suppressed. +constexpr inline std::uint32_t anomaly_report_limit = 8; + +/// Gate evaluated BEFORE the format arguments of LOG_ANOMALY: checks the log +/// level and bumps the per-ID rate-limit counter. Logs a final suppression +/// notice when the cap is reached. +bool anomaly_should_report(AnomalyId id); + +/// Log "[anomaly:] " at err level, forward it to the notify +/// hook, then trap: Debug builds abort (after flushing the log) unless the +/// trap is overridden for tests or CLICE_ANOMALY_NO_TRAP is set in the +/// environment; Release builds continue. +void report_anomaly(AnomalyId id, + std::string message, + std::source_location location = std::source_location::current()); + +/// Gate evaluated BEFORE the format arguments of LOG_GUIDANCE. +bool guidance_should_report(); + +/// Log "[guidance] " at warn level and forward it to the notify hook. +/// For user-actionable situations that are not clice bugs (missing CDB, +/// invalid configuration, ...). +void report_guidance(std::string message, + std::source_location location = std::source_location::current()); + +/// Severity forwarded to the notify hook; values mirror LSP MessageType. +enum class NotifyLevel : std::uint8_t { + Error = 1, + Warning = 2, +}; + +/// Process-wide hook that pushes anomaly/guidance messages to the LSP client. +/// The master sets it once a client peer is available; workers never set it. +/// Invoked synchronously from the reporting call site — all current call +/// sites in the master run on the event-loop thread. +void set_notify_hook(std::function hook); + +/// Replace the debug trap so unit tests can observe anomalies (in any build +/// type) without aborting the test process. +void set_anomaly_trap_for_testing(std::function trap); + +/// Clear rate-limit counters and the testing trap. +void reset_anomaly_for_testing(); + +} // namespace clice::logging + +/// Soft assertion for internal invariants (see AnomalyId). The gate runs +/// before the format arguments are evaluated, so suppressed reports cost +/// nothing — arguments must not carry needed side effects. +#define LOG_ANOMALY(id, fmt, ...) \ + do { \ + if(clice::logging::anomaly_should_report(clice::logging::AnomalyId::id)) { \ + clice::logging::report_anomaly(clice::logging::AnomalyId::id, \ + std::format(fmt __VA_OPT__(, ) __VA_ARGS__)); \ + } \ + } while(0) + +/// User-facing guidance via window/logMessage (master) and the log file. +/// Same lazy-evaluation contract as LOG_ANOMALY. +#define LOG_GUIDANCE(fmt, ...) \ + do { \ + if(clice::logging::guidance_should_report()) { \ + clice::logging::report_guidance(std::format(fmt __VA_OPT__(, ) __VA_ARGS__)); \ + } \ + } while(0) diff --git a/tests/unit/support/anomaly_tests.cpp b/tests/unit/support/anomaly_tests.cpp new file mode 100644 index 00000000..bce237c0 --- /dev/null +++ b/tests/unit/support/anomaly_tests.cpp @@ -0,0 +1,147 @@ +#include +#include + +#include "test/test.h" +#include "support/anomaly.h" +#include "support/logging.h" + +namespace clice::testing { +namespace { + +using logging::AnomalyId; +using logging::NotifyLevel; + +/// RAII fixture: capture the notify hook and trap, restore globals afterwards. +struct AnomalyCapture { + std::vector> notified; + std::vector trapped; + logging::Level saved_level; + + AnomalyCapture() : saved_level(logging::options.level) { + logging::reset_anomaly_for_testing(); + logging::set_notify_hook([this](NotifyLevel level, std::string_view message) { + notified.emplace_back(level, std::string(message)); + }); + logging::set_anomaly_trap_for_testing([this](AnomalyId id) { trapped.push_back(id); }); + } + + ~AnomalyCapture() { + logging::set_notify_hook(nullptr); + logging::reset_anomaly_for_testing(); + logging::options.level = saved_level; + } +}; + +TEST_SUITE(Anomaly) { + +TEST_CASE(MarkerAndNotify) { + AnomalyCapture capture; + + LOG_ANOMALY(PchBuildFail, "stale build for {}", "main.cpp"); + + ASSERT_EQ(capture.notified.size(), 1u); + auto& [level, message] = capture.notified.front(); + EXPECT_EQ(level, NotifyLevel::Error); + EXPECT_EQ(message, "[anomaly:pch_build_fail] stale build for main.cpp"); +} + +TEST_CASE(TrapInvokedPerReport) { + /// The trap fires once per reported (non-suppressed) anomaly. In Debug + /// builds the default trap aborts the process; the override used here is + /// the mock point that lets us observe it in any build type. + AnomalyCapture capture; + + LOG_ANOMALY(WorkerCrash, "worker {} died", 1); + LOG_ANOMALY(WorkerCrash, "worker {} died", 2); + + ASSERT_EQ(capture.trapped.size(), 2u); + EXPECT_EQ(capture.trapped[0], AnomalyId::WorkerCrash); +} + +TEST_CASE(RateLimitSuppresses) { + AnomalyCapture capture; + + for(std::uint32_t i = 0; i < logging::anomaly_report_limit + 5; ++i) { + LOG_ANOMALY(CompileFail, "occurrence {}", i); + } + + EXPECT_EQ(capture.notified.size(), logging::anomaly_report_limit); + EXPECT_EQ(capture.trapped.size(), logging::anomaly_report_limit); +} + +TEST_CASE(RateLimitIsPerId) { + AnomalyCapture capture; + + for(std::uint32_t i = 0; i < logging::anomaly_report_limit + 5; ++i) { + LOG_ANOMALY(CompileFail, "occurrence {}", i); + } + LOG_ANOMALY(PcmBuildFail, "different id still reports"); + + EXPECT_EQ(capture.notified.size(), logging::anomaly_report_limit + 1); +} + +TEST_CASE(SuppressedArgsNotEvaluated) { + /// Locks the lazy-evaluation contract: once the rate limit gate fails, + /// the format arguments must not be evaluated at all. + AnomalyCapture capture; + + int evaluations = 0; + auto observe = [&evaluations] { + ++evaluations; + return 42; + }; + + for(std::uint32_t i = 0; i < logging::anomaly_report_limit + 5; ++i) { + LOG_ANOMALY(PositionMapFail, "value {}", observe()); + } + + EXPECT_EQ(evaluations, static_cast(logging::anomaly_report_limit)); +} + +TEST_CASE(LevelGateSkipsEvaluation) { + AnomalyCapture capture; + logging::options.level = logging::Level::off; + + int evaluations = 0; + auto observe = [&evaluations] { + ++evaluations; + return 42; + }; + LOG_ANOMALY(PchBuildFail, "value {}", observe()); + + EXPECT_EQ(evaluations, 0); + EXPECT_EQ(capture.notified.size(), 0u); + EXPECT_EQ(capture.trapped.size(), 0u); +} + +TEST_CASE(GuidanceMarkerAndLevel) { + AnomalyCapture capture; + + LOG_GUIDANCE("no compilation database found in {}", "/tmp/ws"); + + ASSERT_EQ(capture.notified.size(), 1u); + auto& [level, message] = capture.notified.front(); + EXPECT_EQ(level, NotifyLevel::Warning); + EXPECT_EQ(message, "[guidance] no compilation database found in /tmp/ws"); + EXPECT_EQ(capture.trapped.size(), 0u); +} + +TEST_CASE(GuidanceLazyAtLevel) { + AnomalyCapture capture; + logging::options.level = logging::Level::off; + + int evaluations = 0; + auto observe = [&evaluations] { + ++evaluations; + return 1; + }; + LOG_GUIDANCE("value {}", observe()); + + EXPECT_EQ(evaluations, 0); + EXPECT_EQ(capture.notified.size(), 0u); +} + +}; // TEST_SUITE(Anomaly) + +} // namespace +} // namespace clice::testing From 51616f45c3f859b80e12cb6f4346ef46a7b1c916 Mon Sep 17 00:00:00 2001 From: ykiko Date: Fri, 12 Jun 2026 11:16:31 +0800 Subject: [PATCH 3/6] feat(server): error feedback channels and command source tracking Reimplements the intent of #436 on current main (superseding it): - fill_compile_args() reports a CommandSource (CdbExact / IncludeGraph / Inferred / Fallback) and emits a per-file decision log (tiers tried, tier hit, command hash). Real CDB entries are now distinguished from the synthesized default command via has_entry(), which also revives the automatic include-graph header context tier that the synthesis had shadowed. - Guessed commands whose diagnostics contain file-not-found errors get a file-top Warning guidance diagnostic (source: clice, code links the setup guide, resolving the DiagnosticSource::Clice TODO). Exact CDB matches never get it. - clice.toml problems become diagnostics on the config file's URI with line/column from the TOML decoder: decode failures as Error (defaults apply), unknown keys as Warning via a strict second decode pass. initializationOptions now overlay BEFORE apply_defaults so derived fields (logging_dir, index_dir) follow overridden values. - Feature requests on closed documents and unresolvable hierarchy items return LSP errors instead of silent null; client-provided positions and ranges that fail to map return InvalidParams. Empty hierarchy and workspace-symbol results return empty arrays instead of null. - Internal failures report anomalies: unexpected PCH/PCM build failures (user-code errors are distinguished via BuildResult.has_user_errors), compile/query IPC failures, worker crash/spawn failures, and internally-produced offsets failing to map (feature::to_position now clamps instead of dereferencing an empty optional). - Master pushes anomaly/guidance to the client via window/logMessage; workers only log (their files are scanned by tests). --- src/compile/diagnostic.cpp | 6 +- src/feature/code_completion.cpp | 4 +- src/feature/diagnostics.cpp | 4 +- src/feature/feature.h | 18 +- src/feature/folding_ranges.cpp | 4 +- src/feature/formatting.cpp | 4 +- src/feature/inlay_hints.cpp | 2 +- src/feature/semantic_tokens.cpp | 4 +- src/server/compiler/compiler.cpp | 244 ++++++++++++++++++++----- src/server/compiler/compiler.h | 39 +++- src/server/compiler/indexer.cpp | 5 +- src/server/protocol/worker.h | 3 + src/server/service/agent_client.cpp | 5 +- src/server/service/lsp_client.cpp | 109 ++++++++--- src/server/service/lsp_client.h | 3 + src/server/service/master_server.cpp | 21 ++- src/server/service/master_server.h | 8 + src/server/worker/stateless_worker.cpp | 12 +- src/server/worker/worker_pool.cpp | 40 ++-- src/server/workspace/config.cpp | 51 +++++- src/server/workspace/config.h | 40 +++- tests/unit/server/config_tests.cpp | 36 ++++ 22 files changed, 530 insertions(+), 132 deletions(-) diff --git a/src/compile/diagnostic.cpp b/src/compile/diagnostic.cpp index 73ad23a6..840e4444 100644 --- a/src/compile/diagnostic.cpp +++ b/src/compile/diagnostic.cpp @@ -70,7 +70,11 @@ std::optional DiagnosticID::diagnostic_document_uri() const { } case DiagnosticSource::Clice: { - /// TODO: Add diagnostic for clice. + // clice's own guidance diagnostics link to the setup guide that + // explains how to provide a compilation database. + if(name == "inferred-compile-command") { + return "https://clice.io/en/guide/quick-start"; + } return std::nullopt; } } diff --git a/src/feature/code_completion.cpp b/src/feature/code_completion.cpp index b883d5ab..11d81241 100644 --- a/src/feature/code_completion.cpp +++ b/src/feature/code_completion.cpp @@ -257,8 +257,8 @@ class CodeCompletionCollector final : public clang::CodeCompleteConsumer { PositionMapper converter(content, encoding); auto replace_range = protocol::Range{ - .start = *converter.to_position(prefix.range.begin), - .end = *converter.to_position(prefix.range.end), + .start = to_position(converter, prefix.range.begin), + .end = to_position(converter, prefix.range.end), }; std::vector collected; diff --git a/src/feature/diagnostics.cpp b/src/feature/diagnostics.cpp index 7eb5cbf7..e7df1dd3 100644 --- a/src/feature/diagnostics.cpp +++ b/src/feature/diagnostics.cpp @@ -154,8 +154,8 @@ auto diagnostics(CompilationUnitRef unit, PositionEncoding encoding) auto offset = unit.file_offset(include_location); auto end_offset = offset + unit.token_spelling(include_location).size(); diagnostic.range = protocol::Range{ - .start = *main_converter.to_position(offset), - .end = *main_converter.to_position(end_offset), + .start = to_position(main_converter, offset), + .end = to_position(main_converter, end_offset), }; current = std::move(diagnostic); diff --git a/src/feature/feature.h b/src/feature/feature.h index 86eda11e..a86f2761 100644 --- a/src/feature/feature.h +++ b/src/feature/feature.h @@ -8,6 +8,7 @@ #include "compile/compilation.h" #include "compile/compilation_unit.h" #include "semantic/symbol_kind.h" +#include "support/anomaly.h" #include "support/markup.h" #include "kota/ipc/lsp/position.h" @@ -21,10 +22,23 @@ using kota::ipc::lsp::PositionEncoding; using kota::ipc::lsp::PositionMapper; using kota::ipc::lsp::parse_position_encoding; +/// Map an internally-produced byte offset to a protocol position. Offsets +/// here come from clice's own analysis of the same content the mapper was +/// built from, so a failed mapping is a clice bug: report an anomaly and +/// clamp to the document start instead of dereferencing an empty optional. +inline auto to_position(const PositionMapper& converter, std::uint32_t offset) + -> protocol::Position { + if(auto position = converter.to_position(offset)) { + return *position; + } + LOG_ANOMALY(PositionMapFail, "offset {} cannot be mapped to a position", offset); + return protocol::Position{.line = 0, .character = 0}; +} + inline auto to_range(const PositionMapper& converter, LocalSourceRange range) -> protocol::Range { return protocol::Range{ - .start = *converter.to_position(range.begin), - .end = *converter.to_position(range.end), + .start = to_position(converter, range.begin), + .end = to_position(converter, range.end), }; } diff --git a/src/feature/folding_ranges.cpp b/src/feature/folding_ranges.cpp index 6f53fb99..2e70a5d0 100644 --- a/src/feature/folding_ranges.cpp +++ b/src/feature/folding_ranges.cpp @@ -355,8 +355,8 @@ auto folding_ranges(CompilationUnitRef unit, PositionEncoding encoding) result.reserve(collected.size()); for(const auto& item: collected) { - auto start = *converter.to_position(item.range.begin); - auto end = *converter.to_position(item.range.end); + auto start = to_position(converter, item.range.begin); + auto end = to_position(converter, item.range.end); protocol::FoldingRange range{ .start_line = start.line, diff --git a/src/feature/formatting.cpp b/src/feature/formatting.cpp index 0be63702..0487d5b7 100644 --- a/src/feature/formatting.cpp +++ b/src/feature/formatting.cpp @@ -57,8 +57,8 @@ auto document_format(llvm::StringRef file, for(const auto& replacement: *replacements) { protocol::TextEdit edit; - edit.range.start = *converter.to_position(replacement.getOffset()); - edit.range.end = *converter.to_position(replacement.getOffset() + replacement.getLength()); + edit.range.start = to_position(converter, replacement.getOffset()); + edit.range.end = to_position(converter, replacement.getOffset() + replacement.getLength()); edit.new_text = replacement.getReplacementText().str(); edits.push_back(std::move(edit)); } diff --git a/src/feature/inlay_hints.cpp b/src/feature/inlay_hints.cpp index 2e21901b..24440cc6 100644 --- a/src/feature/inlay_hints.cpp +++ b/src/feature/inlay_hints.cpp @@ -935,7 +935,7 @@ auto inlay_hints(CompilationUnitRef unit, for(const auto& hint: collected) { protocol::InlayHint out{ - .position = *converter.to_position(hint.offset), + .position = to_position(converter, hint.offset), .label = hint.label, }; diff --git a/src/feature/semantic_tokens.cpp b/src/feature/semantic_tokens.cpp index 9c4db1e0..4008713d 100644 --- a/src/feature/semantic_tokens.cpp +++ b/src/feature/semantic_tokens.cpp @@ -492,8 +492,8 @@ class SemanticTokenEncoder { auto begin = token.range.begin; auto end = token.range.end; - auto begin_position = *converter.to_position(begin); - auto end_position = *converter.to_position(end); + auto begin_position = to_position(converter, begin); + auto end_position = to_position(converter, end); auto begin_line = static_cast(begin_position.line); auto begin_char = static_cast(begin_position.character); auto end_line = static_cast(end_position.line); diff --git a/src/server/compiler/compiler.cpp b/src/server/compiler/compiler.cpp index 5f3075e2..6805de95 100644 --- a/src/server/compiler/compiler.cpp +++ b/src/server/compiler/compiler.cpp @@ -1,12 +1,15 @@ #include "server/compiler/compiler.h" +#include #include #include #include #include "command/search_config.h" +#include "compile/diagnostic.h" #include "index/tu_index.h" #include "server/protocol/worker.h" +#include "support/anomaly.h" #include "support/filesystem.h" #include "support/logging.h" #include "syntax/include_resolver.h" @@ -92,8 +95,7 @@ void Compiler::init_compile_graph() { worker::BuildParams bp; bp.kind = worker::BuildKind::BuildPCM; bp.file = file_path; - if(!fill_compile_args(file_path, bp.directory, bp.arguments)) - co_return false; + fill_compile_args(file_path, bp.directory, bp.arguments); // Compute deterministic content-addressed PCM path. auto safe_module_name = mod_it->second; @@ -124,9 +126,14 @@ void Compiler::init_compile_graph() { auto result = co_await pool.send_stateless(bp); if(!result.has_value() || !result.value().success) { - LOG_WARN("BuildPCM failed for module {}: {}", - mod_it->second, - result.has_value() ? result.value().error : result.error().message); + if(result.has_value() && result.value().has_user_errors) { + LOG_WARN("BuildPCM failed for module {}: {}", mod_it->second, result.value().error); + } else { + LOG_ANOMALY(PcmBuildFail, + "PCM build failed for module {}: {}", + mod_it->second, + result.has_value() ? result.value().error : result.error().message); + } co_return false; } @@ -151,33 +158,93 @@ void Compiler::init_compile_graph() { LOG_INFO("CompileGraph initialized with {} module(s)", workspace.path_to_module.size()); } -bool Compiler::fill_compile_args(llvm::StringRef path, - std::string& directory, - std::vector& arguments, - Session* session) { +llvm::StringRef command_source_name(CommandSource source) { + switch(source) { + case CommandSource::CdbExact: return "cdb_exact"; + case CommandSource::IncludeGraph: return "include_graph"; + case CommandSource::Inferred: return "inferred"; + case CommandSource::Fallback: return "fallback"; + } + return "unknown"; +} + +/// Per-file command selection decision log: which tiers were tried, which one +/// was hit, and a hash of the final command for correlating later failures. +static void log_command_decision(llvm::StringRef path, + llvm::ArrayRef tried, + CommandSource source, + llvm::ArrayRef arguments) { + if(clice::logging::options.level > clice::logging::Level::info) + return; + std::string tried_text; + for(auto tier: tried) { + if(!tried_text.empty()) + tried_text += ","; + tried_text += tier; + } + std::string joined; + for(auto& arg: arguments) { + joined += arg; + joined += '\0'; + } + LOG_INFO("compile_args: file={} tried=[{}] source={} args_hash={:016x}", + path, + tried_text, + command_source_name(source), + llvm::xxh3_64bits(llvm::StringRef(joined))); +} + +CommandSource Compiler::fill_compile_args(llvm::StringRef path, + std::string& directory, + std::vector& arguments, + Session* session) { auto path_id = workspace.path_pool.intern(path); + llvm::SmallVector tried; // 1. If the session has an active header context via switchContext, // use the host source's CDB entry with file path replaced and preamble injected. if(session && session->active_context.has_value()) { - return fill_header_context_args(path, path_id, directory, arguments, session); - } - - // 2. Normal CDB lookup for the file itself. - // Apply rules from config (append/remove flags based on file patterns). - std::vector rule_append, rule_remove; - workspace.config.match_rules(path, rule_append, rule_remove); - auto results = workspace.cdb.lookup(path, {.remove = rule_remove, .append = rule_append}); - if(!results.empty()) { + tried.push_back("switch_context"); + if(fill_header_context_args(path, path_id, directory, arguments, session)) { + log_command_decision(path, tried, CommandSource::IncludeGraph, arguments); + return CommandSource::IncludeGraph; + } + } else if(workspace.cdb.has_entry(path)) { + // 2. Real CDB entry for the file itself. + // Apply rules from config (append/remove flags based on file patterns). + tried.push_back("cdb"); + std::vector rule_append, rule_remove; + workspace.config.match_rules(path, rule_append, rule_remove); + auto results = workspace.cdb.lookup(path, {.remove = rule_remove, .append = rule_append}); workspace.toolchain.resolve_or_warn(results.front()); auto& cmd = results.front(); directory = cmd.resolved.directory.str(); arguments = cmd.to_string_argv(); - return true; + log_command_decision(path, tried, CommandSource::CdbExact, arguments); + return CommandSource::CdbExact; + } else { + // 3. No CDB entry — try automatic header context resolution. + tried.push_back("cdb"); + tried.push_back("include_graph"); + if(fill_header_context_args(path, path_id, directory, arguments, session)) { + log_command_decision(path, tried, CommandSource::IncludeGraph, arguments); + return CommandSource::IncludeGraph; + } } - // 3. No CDB entry — try automatic header context resolution. - return fill_header_context_args(path, path_id, directory, arguments, session); + // 4. Nothing matched — use the default command the CDB layer synthesizes + // for unknown files, so the file still compiles and produces + // diagnostics instead of failing silently. + tried.push_back("fallback"); + std::vector rule_append, rule_remove; + workspace.config.match_rules(path, rule_append, rule_remove); + auto results = workspace.cdb.lookup(path, {.remove = rule_remove, .append = rule_append}); + workspace.toolchain.resolve_or_warn(results.front()); + auto& cmd = results.front(); + directory = cmd.resolved.directory.str(); + arguments = cmd.to_string_argv(); + log_command_decision(path, tried, CommandSource::Fallback, arguments); + return CommandSource::Fallback; } bool Compiler::fill_header_context_args(llvm::StringRef path, @@ -410,9 +477,48 @@ std::string uri_to_path(const std::string& uri) { return uri; } +/// Clang diagnostics indicating that an input file could not be found — +/// the symptom of a guessed compile command missing include paths. +static bool is_file_not_found(const protocol::Diagnostic& diagnostic) { + if(!diagnostic.code.has_value()) + return false; + auto* code = std::get_if(&*diagnostic.code); + return code && (*code == "err_pp_file_not_found" || *code == "err_pp_error_opening_file" || + *code == "err_module_not_found"); +} + +/// File-top warning explaining that diagnostics were produced with a guessed +/// compile command, pointing at the compilation database documentation. +static protocol::Diagnostic make_inferred_command_diagnostic(CommandSource source) { + DiagnosticID id{ + .value = 0, + .level = DiagnosticLevel::Warning, + .source = DiagnosticSource::Clice, + .name = "inferred-compile-command", + }; + + protocol::Diagnostic diagnostic; + diagnostic.range = protocol::Range{ + .start = protocol::Position{.line = 0, .character = 0}, + .end = protocol::Position{.line = 0, .character = 0}, + }; + diagnostic.severity = protocol::DiagnosticSeverity::Warning; + diagnostic.code = id.name.str(); + if(auto uri = id.diagnostic_document_uri()) { + diagnostic.code_description = protocol::CodeDescription{.href = std::move(*uri)}; + } + diagnostic.source = "clice"; + diagnostic.message = std::format( + "No compilation database entry for this file (compile command was {}), so some includes " "may not be found. Configure compile_commands.json for accurate diagnostics.", + source == CommandSource::Fallback ? "synthesized from defaults" + : "inferred from an including file"); + return diagnostic; +} + void Compiler::publish_diagnostics(const std::string& uri, int version, - const kota::codec::RawValue& diagnostics_json) { + const kota::codec::RawValue& diagnostics_json, + CommandSource source) { if(!peer) return; std::vector diagnostics; @@ -422,6 +528,13 @@ void Compiler::publish_diagnostics(const std::string& uri, LOG_WARN("Failed to deserialize diagnostics JSON for {}", uri); } } + + // Guidance (and only when it can explain something): an exact CDB match + // never gets the note, and neither does a guessed command that worked. + if(source != CommandSource::CdbExact && std::ranges::any_of(diagnostics, is_file_not_found)) { + diagnostics.insert(diagnostics.begin(), make_inferred_command_diagnostic(source)); + } + protocol::PublishDiagnosticsParams params; params.uri = uri; params.version = version; @@ -528,9 +641,17 @@ kota::task Compiler::ensure_pch(Session& session, auto result = co_await pool.send_stateless(bp); if(!result.has_value() || !result.value().success) { - LOG_WARN("PCH build failed for {}: {}", - path, - result.has_value() ? result.value().error : result.error().message); + // User-code errors in the preamble are routine (broken includes while + // typing) and resurface as diagnostics from the main compile; anything + // else means the PCH pipeline itself broke. + if(result.has_value() && result.value().has_user_errors) { + LOG_WARN("PCH build failed for {}: {}", path, result.value().error); + } else { + LOG_ANOMALY(PchBuildFail, + "PCH build failed for {}: {}", + path, + result.has_value() ? result.value().error : result.error().message); + } workspace.pch_cache[path_id].building.reset(); completion->set(); co_return false; @@ -668,10 +789,7 @@ kota::task<> Compiler::run_compile(std::uint32_t pid, std::shared_ptrversion; params.text = sess->text; - if(!fill_compile_args(file_path, params.directory, params.arguments, sess)) { - finish_compile(); - co_return; - } + auto source = fill_compile_args(file_path, params.directory, params.arguments, sess); if(!co_await ensure_deps(*sess, params.directory, params.arguments, params.pch, params.pcms)) { LOG_WARN("Dependency preparation failed for {}, skipping compile", uri_str); @@ -703,7 +821,9 @@ kota::task<> Compiler::run_compile(std::uint32_t pid, std::shared_ptr Compiler::run_compile(std::uint32_t pid, std::shared_ptrversion; finish_compile(); - publish_diagnostics(uri_str, version, result.value().diagnostics); + publish_diagnostics(uri_str, version, result.value().diagnostics, source); if(on_indexing_needed) on_indexing_needed(); } @@ -810,7 +930,11 @@ Compiler::RawResult Compiler::forward_query(worker::QueryKind kind, } auto sit = sessions.find(path_id); - if(sit == sessions.end() || sit->second.ast_dirty) { + if(sit == sessions.end()) { + co_return kota::outcome_error(kota::ipc::Error{"Document was closed during compilation"}); + } + if(sit->second.ast_dirty) { + // Concurrent edit landed while compiling; the next request retries. co_return serde_raw{"null"}; } @@ -822,22 +946,32 @@ Compiler::RawResult Compiler::forward_query(worker::QueryKind kind, if(position) { auto offset = mapper.to_offset(*position); - if(!offset) - co_return serde_raw{"null"}; + if(!offset) { + co_return kota::outcome_error(kota::ipc::Error{ + kota::ipc::protocol::ErrorCode::InvalidParams, + std::format("Invalid position {}:{}", position->line, position->character)}); + } wp.offset = *offset; } if(range) { auto start = mapper.to_offset(range->start); auto end = mapper.to_offset(range->end); - if(start && end) { - wp.range = {*start, *end}; + if(!start || !end) { + co_return kota::outcome_error( + kota::ipc::Error{kota::ipc::protocol::ErrorCode::InvalidParams, "Invalid range"}); } + wp.range = {*start, *end}; } auto result = co_await pool.send_stateful(path_id, wp); if(!result.has_value()) { - co_return serde_raw{}; + LOG_ANOMALY(WorkerRequestFail, + "query (kind={}) failed for {}: {}", + static_cast(kind), + path, + result.error().message); + co_return kota::outcome_error(std::move(result.error())); } co_return std::move(result.value()); } @@ -855,28 +989,35 @@ Compiler::RawResult Compiler::forward_build(worker::BuildKind kind, // if didClose erases the entry from the sessions map during suspension. wp.version = session.version; wp.text = session.text; - if(!fill_compile_args(path, wp.directory, wp.arguments, &session)) { - co_return serde_raw{}; - } + fill_compile_args(path, wp.directory, wp.arguments, &session); if(!co_await ensure_deps(session, wp.directory, wp.arguments, wp.pch, wp.pcms)) { - co_return serde_raw{}; + LOG_WARN("forward_build: dependency preparation failed for {}", path); + co_return kota::outcome_error(kota::ipc::Error{"Dependency preparation failed"}); } // After co_await, verify session still exists. if(sessions.find(path_id) == sessions.end()) { - co_return serde_raw{}; + co_return kota::outcome_error(kota::ipc::Error{"Document was closed during compilation"}); } lsp::PositionMapper mapper(wp.text, lsp::PositionEncoding::UTF16); auto offset = mapper.to_offset(position); - if(!offset) - co_return serde_raw{"null"}; + if(!offset) { + co_return kota::outcome_error(kota::ipc::Error{ + kota::ipc::protocol::ErrorCode::InvalidParams, + std::format("Invalid position {}:{}", position.line, position.character)}); + } wp.offset = *offset; auto result = co_await pool.send_stateless(wp); if(!result.has_value()) { - co_return serde_raw{}; + LOG_ANOMALY(WorkerRequestFail, + "build (kind={}) failed for {}: {}", + static_cast(kind), + path, + result.error().message); + co_return kota::outcome_error(std::move(result.error())); } co_return std::move(result.value().result_json); } @@ -895,14 +1036,18 @@ Compiler::RawResult Compiler::forward_format(Session& session, lsp::PositionMapper mapper(wp.text, lsp::PositionEncoding::UTF16); auto begin = mapper.to_offset(range->start); auto end = mapper.to_offset(range->end); - if(!begin || !end) - co_return serde_raw{"null"}; + if(!begin || !end) { + co_return kota::outcome_error( + kota::ipc::Error{kota::ipc::protocol::ErrorCode::InvalidParams, + "Invalid formatting range"}); + } wp.format_range = {*begin, *end}; } auto result = co_await pool.send_stateless(wp); if(!result.has_value()) { - co_return serde_raw{"null"}; + LOG_ANOMALY(WorkerRequestFail, "format failed for {}: {}", path, result.error().message); + co_return kota::outcome_error(std::move(result.error())); } co_return std::move(result.value().result_json); } @@ -920,8 +1065,7 @@ Compiler::RawResult Compiler::handle_completion(const protocol::Position& positi pctx.kind == CompletionContext::IncludeAngled) { std::string directory; std::vector arguments; - if(!fill_compile_args(path, directory, arguments)) - co_return serde_raw{"[]"}; + fill_compile_args(path, directory, arguments); std::vector args_ptrs; args_ptrs.reserve(arguments.size()); diff --git a/src/server/compiler/compiler.h b/src/server/compiler/compiler.h index 7ffd840f..8dccf06f 100644 --- a/src/server/compiler/compiler.h +++ b/src/server/compiler/compiler.h @@ -30,6 +30,25 @@ namespace protocol = kota::ipc::protocol; /// Convert a file:// URI to a local file path. std::string uri_to_path(const std::string& uri); +/// Where the compile command for a file came from. Anything other than +/// CdbExact means the command was guessed to some degree, which is why +/// diagnostics produced with it may deserve a guidance note (see +/// Compiler::publish_diagnostics). +enum class CommandSource : std::uint8_t { + /// Direct compilation database entry for the file. + CdbExact, + /// Header compiled in the context of a host source found through the + /// include graph (automatic or via clice/switchContext). + IncludeGraph, + /// Reserved for command transfer heuristics (e.g. nearest CDB entry); + /// no producer yet. + Inferred, + /// Synthesized default command — no CDB entry and no usable host source. + Fallback, +}; + +llvm::StringRef command_source_name(CommandSource source); + /// Compilation service — drives worker processes to build ASTs, PCHs, and PCMs. /// /// Compiler holds no persistent state of its own. All project-wide data @@ -62,12 +81,15 @@ class Compiler { void init_compile_graph(); - /// Fill compile arguments for a file (CDB lookup + header context fallback). + /// Fill compile arguments for a file and report where they came from. + /// Tries, in order: CDB entry, header context through the include graph, + /// and finally a synthesized fallback command — so it always succeeds. + /// Emits a per-file decision log (tiers tried, tier hit, command hash). /// @param session If non-null, used for header context resolution on open files. - bool fill_compile_args(llvm::StringRef path, - std::string& directory, - std::vector& arguments, - Session* session = nullptr); + CommandSource fill_compile_args(llvm::StringRef path, + std::string& directory, + std::vector& arguments, + Session* session = nullptr); /// Compile an open file's AST if dirty. On success, updates session's /// file_index, pch_ref, ast_deps, and publishes diagnostics. @@ -122,9 +144,14 @@ class Compiler { bool is_stale(const Session& session); void record_deps(Session& session, llvm::ArrayRef deps); + /// Deserialize worker-produced diagnostics and publish them. When the + /// compile command was not an exact CDB match and the diagnostics contain + /// file-not-found class errors, a file-top guidance diagnostic explaining + /// the inferred command is merged into the same publish. void publish_diagnostics(const std::string& uri, int version, - const kota::codec::RawValue& diags); + const kota::codec::RawValue& diags, + CommandSource source); std::optional resolve_header_context(std::uint32_t header_path_id, Session* session); diff --git a/src/server/compiler/indexer.cpp b/src/server/compiler/indexer.cpp index 4248574e..86aac3e0 100644 --- a/src/server/compiler/indexer.cpp +++ b/src/server/compiler/indexer.cpp @@ -827,7 +827,10 @@ kota::task<> Indexer::index_one(std::uint32_t server_path_id) { worker::BuildParams params; params.kind = worker::BuildKind::Index; params.file = file_path; - if(!compiler.fill_compile_args(file_path, params.directory, params.arguments, nullptr)) + // Bulk background indexing sticks to real commands; synthesized fallback + // commands would fill the index with guesses. + if(compiler.fill_compile_args(file_path, params.directory, params.arguments, nullptr) == + CommandSource::Fallback) co_return; workspace.fill_pcm_deps(params.pcms); diff --git a/src/server/protocol/worker.h b/src/server/protocol/worker.h index 4fe9e715..60914568 100644 --- a/src/server/protocol/worker.h +++ b/src/server/protocol/worker.h @@ -101,6 +101,9 @@ struct BuildParams { struct BuildResult { bool success = true; std::string error; + /// On failure: whether `error` carries user-code compile errors. A failure + /// withOUT user errors indicates clice infrastructure breakage (anomaly). + bool has_user_errors = false; std::string output_path; ///< PCH or PCM path std::vector deps; std::string tu_index_data; diff --git a/src/server/service/agent_client.cpp b/src/server/service/agent_client.cpp index 69c7a81b..8004db17 100644 --- a/src/server/service/agent_client.cpp +++ b/src/server/service/agent_client.cpp @@ -197,10 +197,7 @@ AgentClient::AgentClient(MasterServer& server, kota::ipc::JsonPeer& peer) : const CompileCommandParams& params) -> RequestResult { std::string directory; std::vector arguments; - if(!srv.compiler.fill_compile_args(params.path, directory, arguments)) { - co_return kota::outcome_error( - kota::ipc::Error{std::format("no compile command found for {}", params.path)}); - } + srv.compiler.fill_compile_args(params.path, directory, arguments); co_return CompileCommandResult{ .file = params.path, diff --git a/src/server/service/lsp_client.cpp b/src/server/service/lsp_client.cpp index 76b8a5ce..2705792a 100644 --- a/src/server/service/lsp_client.cpp +++ b/src/server/service/lsp_client.cpp @@ -10,6 +10,7 @@ #include "server/protocol/extension.h" #include "server/protocol/worker.h" #include "server/service/master_server.h" +#include "support/anomaly.h" #include "support/filesystem.h" #include "support/logging.h" @@ -36,10 +37,24 @@ static serde_raw to_raw(const T& value) { return serde_raw{json ? std::move(*json) : "null"}; } +/// 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"}; +} + LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(server), peer(peer) { server.compiler.set_peer(&peer); server.indexer.set_peer(&peer); + // Anomaly/guidance messages from the master process reach the client as + // window/logMessage notifications from here on. + logging::set_notify_hook([&peer](logging::NotifyLevel level, std::string_view message) { + peer.send_notification(protocol::LogMessageParams{ + static_cast(level), + std::string(message), + }); + }); + using StringVec = std::vector; peer.on_request([this](RequestContext& ctx, const protocol::InitializeParams& params) @@ -143,6 +158,7 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s peer.on_notification([this]([[maybe_unused]] const protocol::InitializedParams& params) { this->server.initialize(); + this->publish_config_diagnostics(); }); peer.on_request( @@ -249,7 +265,7 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s auto path_id = srv.workspace.path_pool.intern(path); auto* session = srv.find_session(path_id); if(!session) - co_return serde_raw{"null"}; + co_return kota::outcome_error(document_not_open()); co_return co_await srv.compiler.forward_query( worker::QueryKind::Hover, *session, @@ -263,7 +279,7 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s auto path_id = srv.workspace.path_pool.intern(path); auto* session = srv.find_session(path_id); if(!session) - co_return serde_raw{"null"}; + co_return kota::outcome_error(document_not_open()); co_return co_await srv.compiler.forward_query(worker::QueryKind::SemanticTokens, *session); }); @@ -274,7 +290,7 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s auto path_id = srv.workspace.path_pool.intern(path); auto* session = srv.find_session(path_id); if(!session) - co_return serde_raw{"null"}; + co_return kota::outcome_error(document_not_open()); co_return co_await srv.compiler.forward_query(worker::QueryKind::InlayHints, *session, {}, @@ -288,7 +304,7 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s auto path_id = srv.workspace.path_pool.intern(path); auto* session = srv.find_session(path_id); if(!session) - co_return serde_raw{"null"}; + co_return kota::outcome_error(document_not_open()); co_return co_await srv.compiler.forward_query(worker::QueryKind::FoldingRange, *session); }); @@ -299,7 +315,7 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s auto path_id = srv.workspace.path_pool.intern(path); auto* session = srv.find_session(path_id); if(!session) - co_return serde_raw{"null"}; + co_return kota::outcome_error(document_not_open()); co_return co_await srv.compiler.forward_query(worker::QueryKind::DocumentSymbol, *session); }); @@ -310,11 +326,11 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s auto path_id = srv.workspace.path_pool.intern(path); auto* session = srv.find_session(path_id); if(!session) - co_return serde_raw{"null"}; + co_return kota::outcome_error(document_not_open()); auto result = co_await srv.compiler.forward_query(worker::QueryKind::DocumentLink, *session); if(!result.has_value()) - co_return serde_raw{"null"}; + co_return kota::outcome_error(std::move(result.error())); auto& links = result.value(); auto* session2 = srv.find_session(path_id); if(session2 && session2->pch_ref) { @@ -341,7 +357,7 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s auto path_id = srv.workspace.path_pool.intern(path); auto* session = srv.find_session(path_id); if(!session) - co_return serde_raw{"null"}; + co_return kota::outcome_error(document_not_open()); co_return co_await srv.compiler.forward_query(worker::QueryKind::CodeAction, *session); }); @@ -393,7 +409,7 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s auto path_id = srv.workspace.path_pool.intern(path); auto* session = srv.find_session(path_id); if(!session) - co_return serde_raw{"null"}; + co_return kota::outcome_error(document_not_open()); co_return co_await srv.compiler.forward_query(worker::QueryKind::GoToDefinition, *session, pos); @@ -440,7 +456,7 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s auto path_id = srv.workspace.path_pool.intern(path); auto* session = srv.find_session(path_id); if(!session) - co_return serde_raw{"null"}; + co_return kota::outcome_error(document_not_open()); auto pause = srv.indexer.scoped_pause(); auto result = co_await srv.compiler.handle_completion(params.text_document_position_params.position, @@ -455,7 +471,7 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s auto path_id = srv.workspace.path_pool.intern(path); auto* session = srv.find_session(path_id); if(!session) - co_return serde_raw{"null"}; + co_return kota::outcome_error(document_not_open()); auto pause = srv.indexer.scoped_pause(); auto result = co_await srv.compiler.forward_build(worker::BuildKind::SignatureHelp, @@ -471,7 +487,7 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s auto path_id = srv.workspace.path_pool.intern(path); auto* session = srv.find_session(path_id); if(!session) - co_return serde_raw{"null"}; + co_return kota::outcome_error(document_not_open()); auto pause = srv.indexer.scoped_pause(); co_return co_await srv.compiler.forward_format(*session); }); @@ -483,7 +499,7 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s auto path_id = srv.workspace.path_pool.intern(path); auto* session = srv.find_session(path_id); if(!session) - co_return serde_raw{"null"}; + co_return kota::outcome_error(document_not_open()); auto pause = srv.indexer.scoped_pause(); co_return co_await srv.compiler.forward_format(*session, params.range); }); @@ -510,10 +526,10 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s const protocol::CallHierarchyIncomingCallsParams& params) -> RawResult { auto info = resolve_item(params.item.uri, params.item.range, params.item.data); if(!info) - co_return serde_raw{"null"}; + co_return kota::outcome_error( + kota::ipc::Error{kota::ipc::protocol::ErrorCode::InvalidParams, + "Failed to resolve call hierarchy item"}); auto results = this->server.indexer.find_incoming_calls(info->hash); - if(results.empty()) - co_return serde_raw{"null"}; co_return to_raw(results); }); @@ -522,10 +538,10 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s const protocol::CallHierarchyOutgoingCallsParams& params) -> RawResult { auto info = resolve_item(params.item.uri, params.item.range, params.item.data); if(!info) - co_return serde_raw{"null"}; + co_return kota::outcome_error( + kota::ipc::Error{kota::ipc::protocol::ErrorCode::InvalidParams, + "Failed to resolve call hierarchy item"}); auto results = this->server.indexer.find_outgoing_calls(info->hash); - if(results.empty()) - co_return serde_raw{"null"}; co_return to_raw(results); }); @@ -552,10 +568,10 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s const protocol::TypeHierarchySupertypesParams& params) -> RawResult { auto info = resolve_item(params.item.uri, params.item.range, params.item.data); if(!info) - co_return serde_raw{"null"}; + co_return kota::outcome_error( + kota::ipc::Error{kota::ipc::protocol::ErrorCode::InvalidParams, + "Failed to resolve type hierarchy item"}); auto results = this->server.indexer.find_supertypes(info->hash); - if(results.empty()) - co_return serde_raw{"null"}; co_return to_raw(results); }); @@ -564,18 +580,16 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s const protocol::TypeHierarchySubtypesParams& params) -> RawResult { auto info = resolve_item(params.item.uri, params.item.range, params.item.data); if(!info) - co_return serde_raw{"null"}; + co_return kota::outcome_error( + kota::ipc::Error{kota::ipc::protocol::ErrorCode::InvalidParams, + "Failed to resolve type hierarchy item"}); auto results = this->server.indexer.find_subtypes(info->hash); - if(results.empty()) - co_return serde_raw{"null"}; co_return to_raw(results); }); peer.on_request( [this](RequestContext& ctx, const protocol::WorkspaceSymbolParams& params) -> RawResult { auto results = this->server.indexer.search_symbols(params.query); - if(results.empty()) - co_return serde_raw{"null"}; co_return to_raw(results); }); @@ -706,7 +720,48 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s }); } +/// Publish clice.toml load problems as diagnostics on the config file's URI. +/// The file is usually not open in the editor — publishing to a closed file +/// is fine, the client shows it in the problems panel. A clean load publishes +/// an empty list so diagnostics from a previous (broken) state clear. +void LSPClient::publish_config_diagnostics() { + if(server.config_path.empty()) + return; + + auto uri = lsp::URI::from_file_path(server.config_path); + if(!uri) { + LOG_WARN("Cannot build URI for config file {}", server.config_path); + return; + } + + protocol::PublishDiagnosticsParams params; + params.uri = uri->str(); + for(auto& issue: server.config_issues) { + // rich_error positions are 1-based; LSP wants 0-based. An unknown + // position (0) maps to the file top. The range spans a single + // character — clients render it as the whole token anyway. + auto line = issue.line > 0 ? issue.line - 1 : 0; + auto character = issue.column > 0 ? issue.column - 1 : 0; + + protocol::Diagnostic diagnostic; + diagnostic.range = protocol::Range{ + .start = protocol::Position{line, character }, + .end = protocol::Position{line, character + 1}, + }; + diagnostic.severity = issue.severity == ConfigIssue::Severity::Error + ? protocol::DiagnosticSeverity::Error + : protocol::DiagnosticSeverity::Warning; + diagnostic.source = "clice"; + diagnostic.message = issue.message; + params.diagnostics.push_back(std::move(diagnostic)); + + LOG_GUIDANCE("Configuration problem in {}: {}", issue.file, issue.message); + } + peer.send_notification(params); +} + LSPClient::~LSPClient() { + logging::set_notify_hook(nullptr); server.compiler.set_peer(nullptr); server.indexer.set_peer(nullptr); } diff --git a/src/server/service/lsp_client.h b/src/server/service/lsp_client.h index 9e8a449e..0e365f2b 100644 --- a/src/server/service/lsp_client.h +++ b/src/server/service/lsp_client.h @@ -16,6 +16,9 @@ class LSPClient { private: using RawResult = kota::task; + /// Push clice.toml load problems as diagnostics on the config file URI. + void publish_config_diagnostics(); + MasterServer& server; kota::ipc::JsonPeer& peer; }; diff --git a/src/server/service/master_server.cpp b/src/server/service/master_server.cpp index 4248f312..b1cdc0c3 100644 --- a/src/server/service/master_server.cpp +++ b/src/server/service/master_server.cpp @@ -16,6 +16,7 @@ #include "server/protocol/worker.h" #include "server/service/agent_client.h" #include "server/service/lsp_client.h" +#include "support/anomaly.h" #include "support/filesystem.h" #include "support/logging.h" @@ -53,16 +54,24 @@ MasterServer::MasterServer(kota::event_loop& loop, std::string self_path) : MasterServer::~MasterServer() = default; void MasterServer::initialize() { - workspace.config = Config::load_from_workspace(workspace_root); + config_issues.clear(); + config_path.clear(); + // Load clice.toml raw and overlay initializationOptions BEFORE computing + // defaults: derived fields (logging_dir, index_dir, ...) must follow the + // final merged values (e.g. a cache_dir overridden by the client). + workspace.config = Config::load_from_workspace(workspace_root, + &config_issues, + &config_path, + /*with_defaults=*/false); if(!init_options_json.empty()) { if(auto ov = kota::codec::json::parse(init_options_json, workspace.config); !ov) { - LOG_WARN("Failed to apply initializationOptions: {}", ov.error().to_string()); + LOG_GUIDANCE("Failed to apply initializationOptions: {}", ov.error().to_string()); } else { - workspace.config.apply_defaults(workspace_root); LOG_INFO("Applied initializationOptions overlay"); } init_options_json.clear(); } + workspace.config.apply_defaults(workspace_root); auto& cfg = workspace.config.project; @@ -86,7 +95,7 @@ void MasterServer::initialize() { pool_opts.worker_memory_limit = cfg.worker_memory_limit; pool_opts.log_dir = session_log_dir; if(!pool.start(pool_opts)) { - LOG_ERROR("Failed to start worker pool"); + LOG_ANOMALY(WorkerSpawnFail, "Failed to start worker pool"); return; } @@ -278,7 +287,9 @@ void MasterServer::load_workspace() { } if(cdb_path.empty()) { - LOG_WARN("No compile_commands.json found in workspace {}", workspace_root); + LOG_GUIDANCE( + "No compile_commands.json found in workspace {}. Compile commands will be " "guessed; see https://clice.io/en/guide/quick-start for setup.", + workspace_root); return; } diff --git a/src/server/service/master_server.h b/src/server/service/master_server.h index e0987ad8..d9b1941a 100644 --- a/src/server/service/master_server.h +++ b/src/server/service/master_server.h @@ -2,11 +2,13 @@ #include #include +#include #include "server/compiler/compiler.h" #include "server/compiler/indexer.h" #include "server/service/session.h" #include "server/worker/worker_pool.h" +#include "server/workspace/config.h" #include "server/workspace/workspace.h" #include "kota/async/async.h" @@ -117,6 +119,12 @@ class MasterServer { std::string workspace_root; std::string session_log_dir; std::string init_options_json; + + /// Problems found while loading clice.toml during initialize(), kept so + /// LSPClient can publish them as diagnostics on the config file's URI. + std::vector config_issues; + /// Path of the config file that was found (empty when none). + std::string config_path; }; int run_server_mode(const ServerOptions& opts, const char* self_path); diff --git a/src/server/worker/stateless_worker.cpp b/src/server/worker/stateless_worker.cpp index e5910a00..26e808c1 100644 --- a/src/server/worker/stateless_worker.cpp +++ b/src/server/worker/stateless_worker.cpp @@ -136,7 +136,11 @@ static worker::BuildResult handle_build_pch(const worker::BuildParams& params) { } else { LOG_WARN("BuildPCH failed: file={}, {}ms, errors=[{}]", params.file, timer.ms(), errors); fs::remove(tmp_path); - return {false, errors.empty() ? "PCH compilation failed" : errors}; + worker::BuildResult result; + result.success = false; + result.error = errors.empty() ? "PCH compilation failed" : errors; + result.has_user_errors = !errors.empty(); + return result; } } @@ -193,7 +197,11 @@ static worker::BuildResult handle_build_pcm(const worker::BuildParams& params) { timer.ms(), errors); fs::remove(tmp_path); - return {false, errors.empty() ? "PCM compilation failed" : errors}; + worker::BuildResult result; + result.success = false; + result.error = errors.empty() ? "PCM compilation failed" : errors; + result.has_user_errors = !errors.empty(); + return result; } } diff --git a/src/server/worker/worker_pool.cpp b/src/server/worker/worker_pool.cpp index 213b2275..af225f8d 100644 --- a/src/server/worker/worker_pool.cpp +++ b/src/server/worker/worker_pool.cpp @@ -3,6 +3,7 @@ #include #include +#include "support/anomaly.h" #include "support/logging.h" #include "kota/ipc/transport.h" @@ -79,9 +80,10 @@ bool WorkerPool::spawn_worker(const std::string& self_path, auto result = kota::process::spawn(opts, loop); if(!result) { - LOG_ERROR("Failed to spawn {} worker: {}", - stateful ? "stateful" : "stateless", - result.error().message()); + LOG_ANOMALY(WorkerSpawnFail, + "Failed to spawn {} worker: {}", + stateful ? "stateful" : "stateless", + result.error().message()); return false; } @@ -241,21 +243,24 @@ kota::task<> WorkerPool::monitor_worker(std::size_t index, bool stateful) { if(result.has_value()) { auto& exit = result.value(); if(exit.term_signal != 0) { - LOG_ERROR("Worker {} killed by signal {} (restarts: {})", - name, - exit.term_signal, - w.restart_count); + LOG_ANOMALY(WorkerCrash, + "Worker {} killed by signal {} (restarts: {})", + name, + exit.term_signal, + w.restart_count); } else { - LOG_ERROR("Worker {} exited with code {} (restarts: {})", - name, - exit.status, - w.restart_count); + LOG_ANOMALY(WorkerCrash, + "Worker {} exited with code {} (restarts: {})", + name, + exit.status, + w.restart_count); } } else { - LOG_ERROR("Worker {} lost: {} (restarts: {})", - name, - result.error().message(), - w.restart_count); + LOG_ANOMALY(WorkerCrash, + "Worker {} lost: {} (restarts: {})", + name, + result.error().message(), + w.restart_count); } if(stateful) @@ -306,7 +311,10 @@ bool WorkerPool::respawn_worker(std::size_t index, bool stateful) { auto result = kota::process::spawn(opts, loop); if(!result) { - LOG_ERROR("Failed to respawn worker {}: {}", worker_name, result.error().message()); + LOG_ANOMALY(WorkerSpawnFail, + "Failed to respawn worker {}: {}", + worker_name, + result.error().message()); return false; } diff --git a/src/server/workspace/config.cpp b/src/server/workspace/config.cpp index 0bd567c4..7ee983f4 100644 --- a/src/server/workspace/config.cpp +++ b/src/server/workspace/config.cpp @@ -138,7 +138,29 @@ void Config::match_rules(llvm::StringRef file_path, } } -std::optional Config::load(llvm::StringRef path, llvm::StringRef workspace_root) { +/// Codec config for the strict validation pass: reject unknown keys. +struct DenyUnknownKeys { + constexpr static bool deny_unknown_fields = true; +}; + +static ConfigIssue make_issue(ConfigIssue::Severity severity, + llvm::StringRef path, + const kota::codec::rich_error& error) { + ConfigIssue issue; + issue.severity = severity; + issue.file = path.str(); + issue.message = error.to_string(); + if(error.location) { + issue.line = static_cast(error.location->line); + issue.column = static_cast(error.location->column); + } + return issue; +} + +std::optional Config::load(llvm::StringRef path, + llvm::StringRef workspace_root, + std::vector* issues, + bool with_defaults) { auto content = fs::read(path); if(!content) return std::nullopt; @@ -146,11 +168,25 @@ std::optional Config::load(llvm::StringRef path, llvm::StringRef workspa auto result = kota::codec::toml::parse(*content); if(!result) { LOG_ERROR("Invalid clice.toml {}: {}", path, result.error().to_string()); + if(issues) + issues->push_back(make_issue(ConfigIssue::Severity::Error, path, result.error())); return std::nullopt; } + // Second, strict decode pass that rejects unknown keys. The lenient + // result above still applies — this only surfaces typos (e.g. a + // misspelled option silently doing nothing) as Warning issues. + if(issues) { + Config probe{}; + if(auto strict = kota::codec::toml::from_toml(*content, probe); !strict) { + LOG_WARN("clice.toml {}: {}", path, strict.error().to_string()); + issues->push_back(make_issue(ConfigIssue::Severity::Warning, path, strict.error())); + } + } + auto config = std::move(*result); - config.apply_defaults(workspace_root); + if(with_defaults) + config.apply_defaults(workspace_root); LOG_INFO("Loaded config from {}", path); return config; } @@ -168,13 +204,18 @@ std::optional Config::load_from_json(llvm::StringRef json, llvm::StringR return config; } -Config Config::load_from_workspace(llvm::StringRef workspace_root) { +Config Config::load_from_workspace(llvm::StringRef workspace_root, + std::vector* issues, + std::string* loaded_path, + bool with_defaults) { if(!workspace_root.empty()) { for(auto* name: {"clice.toml", ".clice/config.toml"}) { auto config_path = path::join(workspace_root, name); if(!llvm::sys::fs::exists(config_path)) continue; - if(auto config = load(config_path, workspace_root)) + if(loaded_path) + *loaded_path = config_path; + if(auto config = load(config_path, workspace_root, issues, with_defaults)) return std::move(*config); // Present but malformed: fall through to defaults, but surface // the situation clearly so users know their config wasn't applied. @@ -183,6 +224,8 @@ Config Config::load_from_workspace(llvm::StringRef workspace_root) { } Config config; + if(!with_defaults) + return config; config.apply_defaults(workspace_root); LOG_INFO( "No clice.toml found, using default configuration " "(stateful={}, stateless={}, memory_limit={}MB)", diff --git a/src/server/workspace/config.h b/src/server/workspace/config.h index 6d398491..ec173d60 100644 --- a/src/server/workspace/config.h +++ b/src/server/workspace/config.h @@ -47,6 +47,25 @@ struct CompiledRule { std::vector remove; }; +/// A problem found while loading a configuration file, carrying enough +/// structure to publish an LSP diagnostic on the file's URI. +struct ConfigIssue { + enum class Severity : std::uint8_t { + /// The configuration was rejected and defaults are in effect. + Error, + /// The configuration still applies (e.g. an unknown key was ignored). + Warning, + }; + + Severity severity; + /// Absolute path of the configuration file. + std::string file; + std::string message; + /// 1-based position in the file; 0 when unknown. + std::uint32_t line = 0; + std::uint32_t column = 0; +}; + /// Configuration for the clice LSP server, loadable from clice.toml /// or passed via LSP initializationOptions. struct Config { @@ -64,8 +83,17 @@ struct Config { std::vector& append, std::vector& remove) const; - /// Try to load configuration from a TOML file. - static std::optional load(llvm::StringRef path, llvm::StringRef workspace_root); + /// Try to load configuration from a TOML file. Parse/validation problems + /// are appended to `issues` when provided: decode failures as Error (the + /// caller falls back to defaults), unknown keys as Warning (the rest of + /// the file still applies). Set `with_defaults` to false when further + /// config sources will be overlaid before apply_defaults() runs — derived + /// fields (index_dir, logging_dir, ...) must be computed only once, from + /// the final merged values. + static std::optional load(llvm::StringRef path, + llvm::StringRef workspace_root, + std::vector* issues = nullptr, + bool with_defaults = true); /// Try to load configuration from a JSON string (e.g. initializationOptions). static std::optional load_from_json(llvm::StringRef json, @@ -73,7 +101,13 @@ struct Config { /// Load config from the workspace, trying standard locations. /// Returns a default config (with apply_defaults) if no file is found. - static Config load_from_workspace(llvm::StringRef workspace_root); + /// `loaded_path`, when provided, receives the path of the config file + /// that was found (even if it failed to parse), or stays empty. + /// `with_defaults` as in load(). + static Config load_from_workspace(llvm::StringRef workspace_root, + std::vector* issues = nullptr, + std::string* loaded_path = nullptr, + bool with_defaults = true); }; } // namespace clice diff --git a/tests/unit/server/config_tests.cpp b/tests/unit/server/config_tests.cpp index 8fd9adca..75bc2212 100644 --- a/tests/unit/server/config_tests.cpp +++ b/tests/unit/server/config_tests.cpp @@ -370,6 +370,42 @@ TEST_CASE(TomlErrorLocated) { EXPECT_FALSE(result.has_value()); } +TEST_CASE(SyntaxIssueHasLocation) { + TempDir tmp; + tmp.touch("clice.toml", "[project\nclang_tidy = true\n"); + std::vector issues; + auto result = Config::load(tmp.path("clice.toml"), tmp.root.str(), &issues); + EXPECT_FALSE(result.has_value()); + ASSERT_EQ(issues.size(), 1u); + EXPECT_EQ(issues[0].severity, ConfigIssue::Severity::Error); + EXPECT_EQ(issues[0].line, 1u); +} + +TEST_CASE(TypeIssueHasLocation) { + TempDir tmp; + tmp.touch("clice.toml", "[project]\nclang_tidy = \"yes\"\n"); + std::vector issues; + auto result = Config::load(tmp.path("clice.toml"), tmp.root.str(), &issues); + EXPECT_FALSE(result.has_value()); + ASSERT_EQ(issues.size(), 1u); + EXPECT_EQ(issues[0].severity, ConfigIssue::Severity::Error); + EXPECT_EQ(issues[0].line, 2u); + EXPECT_NE(issues[0].message.find("clang_tidy"), std::string::npos); +} + +TEST_CASE(UnknownKeyIssueWarns) { + TempDir tmp; + tmp.touch("clice.toml", "[project]\nclang_tdy = true\n"); + std::vector issues; + auto result = Config::load(tmp.path("clice.toml"), tmp.root.str(), &issues); + // Unknown keys do not reject the config — it still loads. + EXPECT_TRUE(result.has_value()); + ASSERT_EQ(issues.size(), 1u); + EXPECT_EQ(issues[0].severity, ConfigIssue::Severity::Warning); + EXPECT_EQ(issues[0].line, 2u); + EXPECT_NE(issues[0].message.find("clang_tdy"), std::string::npos); +} + TEST_CASE(WorkspaceMalformedFallback) { // load_from_workspace must fall back to defaults when clice.toml is malformed, // not propagate the failure. From 073d91a13a78d79904c30ff4b72caa3fe311c6d1 Mon Sep 17 00:00:00 2001 From: ykiko Date: Fri, 12 Jun 2026 11:16:31 +0800 Subject: [PATCH 4/6] test(integration): anomaly capture, error-feedback e2e, timing constants - CliceClient records window/logMessage; assert_no_anomaly() runs in every client/agentic fixture teardown, scanning both notifications and master/worker log files (opt-out: @pytest.mark.allow_anomaly). - replay.py fails a trace when an [anomaly:] message is pushed. - E2E: fallback-command guidance diagnostic lifecycle (appears without CDB, gone after providing one), clice.toml type/unknown-key diagnostics with line/column and clearing after a fix, worker-crash anomaly reporting (Linux), error responses for requests on closed or never-opened documents. - Named timing constants (MTIME_GRANULARITY/SETTLE_TIME/IDLE_TIMEOUT) replace hardcoded sleeps; workspace fixture cleans up after tests; log messages are dumped when a test fails. - Tester::to_local_range() replaces per-file duplicates. --- tests/conftest.py | 50 +++++++++++-- .../compilation/test_persistent_cache.py | 7 +- .../integration/compilation/test_staleness.py | 48 +++++++----- .../features/test_guidance_diagnostics.py | 71 ++++++++++++++++++ tests/integration/features/test_server.py | 19 ++--- tests/integration/lifecycle/test_anomaly.py | 75 +++++++++++++++++++ tests/integration/lifecycle/test_config.py | 60 +++++++++++++++ .../lifecycle/test_file_operation.py | 15 ++-- tests/integration/modules/test_modules.py | 3 +- tests/integration/stress/test_rapid_edit.py | 3 +- tests/integration/utils/assertions.py | 53 ++++++++++++- tests/integration/utils/client.py | 9 +++ tests/integration/utils/wait.py | 5 ++ tests/pytest.ini | 1 + tests/replay.py | 13 ++++ tests/unit/feature/document_link_tests.cpp | 5 -- tests/unit/feature/folding_range_tests.cpp | 17 +---- tests/unit/test/tester.h | 10 +++ 18 files changed, 400 insertions(+), 64 deletions(-) create mode 100644 tests/integration/features/test_guidance_diagnostics.py create mode 100644 tests/integration/lifecycle/test_anomaly.py diff --git a/tests/conftest.py b/tests/conftest.py index 79da8a7f..549f760a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,6 +9,15 @@ import pytest from tests.integration.utils.client import CliceClient +from tests.integration.utils.assertions import assert_no_anomaly + + +@pytest.hookimpl(tryfirst=True, hookwrapper=True) +def pytest_runtest_makereport(item, call): + """Store test outcome so fixtures can detect failures during teardown.""" + outcome = yield + rep = outcome.get_result() + setattr(item, f"rep_{rep.when}", rep) def pytest_addoption(parser: pytest.Parser) -> None: @@ -76,7 +85,8 @@ def workspace(request: pytest.FixtureRequest, test_data_dir: Path) -> Path | Non """ marker = request.node.get_closest_marker("workspace") if marker is None: - return None + yield None + return if not marker.args or not isinstance(marker.args[0], str): raise pytest.UsageError( "@pytest.mark.workspace requires a string argument, e.g. " @@ -89,7 +99,11 @@ def workspace(request: pytest.FixtureRequest, test_data_dir: Path) -> Path | Non clice_dir = path / ".clice" if clice_dir.exists(): shutil.rmtree(clice_dir) - return path + yield path + # Post-test cleanup: drop the cache generated during the test so static + # test-data directories don't accumulate state. + if clice_dir.exists(): + shutil.rmtree(clice_dir, ignore_errors=True) @pytest.fixture @@ -119,7 +133,23 @@ async def client( yield c - await _shutdown_client(c) + test_failed = ( + getattr(request.node, "rep_call", None) is not None + and request.node.rep_call.failed + ) + await _shutdown_client(c, verbose=test_failed) + _check_no_anomaly(request, c) + + +def _check_no_anomaly(request: pytest.FixtureRequest, c: CliceClient) -> None: + """Teardown gate: anomalies are internal clice bugs — every test session + must end without one. Tests that intentionally trigger anomalies opt out + with @pytest.mark.allow_anomaly and assert on them explicitly.""" + if request.node.get_closest_marker("allow_anomaly") is not None: + return + # c.workspace also covers tests that initialize tmp_path workspaces + # themselves (without the @pytest.mark.workspace marker). + assert_no_anomaly(c, c.workspace) def _find_free_port() -> int: @@ -153,6 +183,7 @@ async def agentic( yield executable, host, port await _shutdown_client(c) + _check_no_anomaly(request, c) def generate_cdb(workspace: Path) -> None: @@ -249,8 +280,12 @@ async def assert_server_exited_cleanly(server, timeout: float = 10.0) -> None: pytest.fail("\n".join(failures)) -async def _shutdown_client(c: CliceClient) -> None: - """Gracefully shut down a client, force-kill if needed.""" +async def _shutdown_client(c: CliceClient, *, verbose: bool = False) -> None: + """Gracefully shut down a client, force-kill if needed. + + When verbose=True (typically on test failure), dump the collected + window/logMessage notifications to help diagnose the failure. + """ server = getattr(c, "_server", None) try: @@ -263,6 +298,11 @@ async def _shutdown_client(c: CliceClient) -> None: except Exception: pass + if verbose and c.log_messages: + for msg in c.log_messages: + level = {1: "ERROR", 2: "WARN", 3: "INFO", 4: "LOG"}.get(msg.type, "?") + print(f"[logMessage/{level}] {msg.message}", flush=True) + try: await assert_server_exited_cleanly(server) finally: diff --git a/tests/integration/compilation/test_persistent_cache.py b/tests/integration/compilation/test_persistent_cache.py index fb7be2f7..a7b3b6a6 100644 --- a/tests/integration/compilation/test_persistent_cache.py +++ b/tests/integration/compilation/test_persistent_cache.py @@ -16,6 +16,7 @@ from tests.conftest import make_client, shutdown_client from tests.integration.utils import write_cdb, doc +from tests.integration.utils.wait import MTIME_GRANULARITY, SETTLE_TIME from tests.integration.utils.cache import ( list_pch_files, list_pcm_files, @@ -100,7 +101,7 @@ async def test_pch_reused_on_close_reopen(client, tmp_path): # Close. client.text_document_did_close(DidCloseTextDocumentParams(text_document=doc(uri))) - await asyncio.sleep(0.5) + await asyncio.sleep(SETTLE_TIME) # Clear diagnostics so we can wait for fresh ones. client.diagnostics.pop(uri, None) @@ -227,7 +228,7 @@ async def test_pch_rebuilt_on_header_change(client, tmp_path): assert len(pch_before) >= 1 # Modify header — changes preamble content hash. - await asyncio.sleep(1.1) + await asyncio.sleep(MTIME_GRANULARITY) (tmp_path / "header.h").write_text("#pragma once\nstruct V2 { int b; };\n") # Also update main.cpp to use V2 so it compiles cleanly. (tmp_path / "main.cpp").write_text( @@ -236,7 +237,7 @@ async def test_pch_rebuilt_on_header_change(client, tmp_path): # Close and reopen to get fresh preamble. client.text_document_did_close(DidCloseTextDocumentParams(text_document=doc(uri))) - await asyncio.sleep(0.5) + await asyncio.sleep(SETTLE_TIME) client.diagnostics.pop(uri, None) uri2, _ = await client.open_and_wait(tmp_path / "main.cpp") diff --git a/tests/integration/compilation/test_staleness.py b/tests/integration/compilation/test_staleness.py index 88df093b..1b332018 100644 --- a/tests/integration/compilation/test_staleness.py +++ b/tests/integration/compilation/test_staleness.py @@ -21,7 +21,11 @@ ) from tests.integration.utils import write_cdb, doc -from tests.integration.utils.wait import wait_for_recompile +from tests.integration.utils.wait import ( + MTIME_GRANULARITY, + SETTLE_TIME, + wait_for_recompile, +) from tests.integration.utils.assertions import assert_clean_compile, assert_has_errors @@ -42,7 +46,7 @@ async def test_header_change_invalidates_ast(client, tmp_path): # Modify header on disk — introduce an error. # Ensure mtime advances past filesystem granularity (1s on some FSes). - await asyncio.sleep(1.1) + await asyncio.sleep(MTIME_GRANULARITY) (tmp_path / "header.h").write_text( "inline int value() { return }\n" ) # syntax error @@ -71,7 +75,7 @@ async def test_header_change_invalidates_pch(client, tmp_path): # Modify header — rename struct field. # Ensure mtime advances past filesystem granularity (1s on some FSes). - await asyncio.sleep(1.1) + await asyncio.sleep(MTIME_GRANULARITY) (tmp_path / "header.h").write_text( "#pragma once\nstruct Foo { int y; };\n" # x -> y ) @@ -115,16 +119,22 @@ async def test_touch_without_content_change_skips_recompile(client, tmp_path): assert_clean_compile(client, uri) # Touch the header — mtime changes but content stays the same. - await asyncio.sleep(1.1) + await asyncio.sleep(MTIME_GRANULARITY) original_content = (tmp_path / "header.h").read_text() (tmp_path / "header.h").write_text(original_content) # Hover triggers ensure_compiled which runs deps_changed. # Layer 2 hash confirms nothing actually changed → cached AST reused. - # Hover on "main" (line 1, col 4) which should be hoverable. - hover = await client.text_document_hover_async( - HoverParams(text_document=doc(uri), position=Position(line=1, character=4)) - ) + # The first hover may see ast_dirty=true (mtime changed, hash check in + # progress), so retry to let the hash check complete. + hover = None + for _ in range(3): + hover = await client.text_document_hover_async( + HoverParams(text_document=doc(uri), position=Position(line=1, character=4)) + ) + if hover is not None: + break + await asyncio.sleep(SETTLE_TIME) assert hover is not None # No new diagnostics should appear — the file is still clean. @@ -145,7 +155,7 @@ async def test_header_replaced_with_different_content(client, tmp_path): assert_clean_compile(client, uri) # Replace header — delete and recreate with a breaking change. - await asyncio.sleep(1.1) + await asyncio.sleep(MTIME_GRANULARITY) (tmp_path / "header.h").unlink() (tmp_path / "header.h").write_text("inline int renamed_value() { return 1; }\n") @@ -170,7 +180,7 @@ async def test_fix_error_clears_diagnostics(client, tmp_path): assert_has_errors(client, uri, "Expected diagnostics from broken header") # Fix the header. - await asyncio.sleep(1.1) + await asyncio.sleep(MTIME_GRANULARITY) (tmp_path / "header.h").write_text("inline int value() { return 1; }\n") # Hover triggers recompilation — diagnostics should clear. @@ -198,7 +208,7 @@ async def test_multiple_files_share_header(client, tmp_path): assert_clean_compile(client, uri_b) # Break the shared header. - await asyncio.sleep(1.1) + await asyncio.sleep(MTIME_GRANULARITY) (tmp_path / "shared.h").write_text("inline int shared() { return }\n") # Both files should get diagnostics after hover. @@ -223,7 +233,7 @@ async def test_transitive_header_change(client, tmp_path): assert_clean_compile(client, uri) # Modify the transitive dep (base.h). - await asyncio.sleep(1.1) + await asyncio.sleep(MTIME_GRANULARITY) (tmp_path / "base.h").write_text("inline int base() { return }\n") # broken await wait_for_recompile(client, uri) @@ -310,7 +320,7 @@ async def test_didclose_then_reopen(client, tmp_path): client.text_document_did_close(DidCloseTextDocumentParams(text_document=doc(uri))) # Modify on disk while closed. - await asyncio.sleep(1.1) + await asyncio.sleep(MTIME_GRANULARITY) (tmp_path / "main.cpp").write_text("int main() { return }\n") # broken # Reopen — should compile the new (broken) content from disk. @@ -321,7 +331,7 @@ async def test_didclose_then_reopen(client, tmp_path): async def test_didclose_clears_hover(client, tmp_path): - """After didClose, hover on the closed file should return None.""" + """After didClose, hover on the closed file should return an error.""" (tmp_path / "main.cpp").write_text("int main() { return 0; }\n") write_cdb(tmp_path, ["main.cpp"]) await client.initialize(tmp_path) @@ -330,10 +340,10 @@ async def test_didclose_clears_hover(client, tmp_path): client.text_document_did_close(DidCloseTextDocumentParams(text_document=doc(uri))) - hover = await client.text_document_hover_async( - HoverParams(text_document=doc(uri), position=Position(line=0, character=4)) - ) - assert hover is None, "Hover on closed file should return None" + with pytest.raises(Exception, match="Document not open"): + await client.text_document_hover_async( + HoverParams(text_document=doc(uri), position=Position(line=0, character=4)) + ) async def test_didsave_triggers_recompile_for_dependents(client, tmp_path): @@ -349,7 +359,7 @@ async def test_didsave_triggers_recompile_for_dependents(client, tmp_path): assert_clean_compile(client, uri) # Modify header on disk and send didSave. - await asyncio.sleep(1.1) + await asyncio.sleep(MTIME_GRANULARITY) (tmp_path / "header.h").write_text("inline int value() { return }\n") # broken client.text_document_did_save( DidSaveTextDocumentParams( diff --git a/tests/integration/features/test_guidance_diagnostics.py b/tests/integration/features/test_guidance_diagnostics.py new file mode 100644 index 00000000..e3284f5d --- /dev/null +++ b/tests/integration/features/test_guidance_diagnostics.py @@ -0,0 +1,71 @@ +"""Guidance diagnostics for files compiled with guessed compile commands. + +When a file has no compilation database entry, clice compiles it with a +synthesized fallback command. If that produces file-not-found errors, a +file-top warning explains the situation; an exact CDB match never gets it. +""" + +from lsprotocol.types import DiagnosticSeverity + +from tests.conftest import make_client, shutdown_client +from tests.integration.utils import write_cdb +from tests.integration.utils.assertions import assert_no_anomaly, guidance_messages + +GUIDANCE_CODE = "inferred-compile-command" + +BROKEN_INCLUDE = '#include "no_such_header.h"\nint main() { return 0; }\n' + + +def guidance_diags(client, uri): + return [d for d in client.diagnostics.get(uri, []) if d.code == GUIDANCE_CODE] + + +def file_not_found_diags(client, uri): + return [ + d for d in client.diagnostics.get(uri, []) if d.code == "err_pp_file_not_found" + ] + + +async def test_fallback_guidance_lifecycle(executable, tmp_path): + (tmp_path / "main.cpp").write_text(BROKEN_INCLUDE) + + # Phase 1: no CDB — fallback command, broken include → guidance at the top. + client = await make_client(executable, tmp_path) + try: + uri, _ = await client.open_and_wait(tmp_path / "main.cpp") + assert file_not_found_diags(client, uri), "broken include should surface" + guidance = guidance_diags(client, uri) + assert len(guidance) == 1, f"expected one guidance diagnostic: {guidance}" + assert guidance[0].severity == DiagnosticSeverity.Warning + assert guidance[0].range.start.line == 0 + assert guidance[0].source == "clice" + # The missing CDB is also announced via window/logMessage guidance. + assert any("compile_commands.json" in m for m in guidance_messages(client)) + assert_no_anomaly(client, tmp_path) + finally: + await shutdown_client(client) + + # Phase 2: provide a CDB and restart — the include error remains, the + # guidance diagnostic must disappear (exact CDB match never gets it). + write_cdb(tmp_path, ["main.cpp"]) + client = await make_client(executable, tmp_path) + try: + uri, _ = await client.open_and_wait(tmp_path / "main.cpp") + assert file_not_found_diags(client, uri), "include is still broken" + assert not guidance_diags(client, uri), ( + "CDB-matched files must not get the inferred-command guidance" + ) + assert_no_anomaly(client, tmp_path) + finally: + await shutdown_client(client) + + +async def test_fallback_clean_no_guidance(executable, tmp_path): + # A guessed command that works produces no guidance noise. + (tmp_path / "main.cpp").write_text("int main() { return 0; }\n") + client = await make_client(executable, tmp_path) + try: + uri, _ = await client.open_and_wait(tmp_path / "main.cpp") + assert not guidance_diags(client, uri) + finally: + await shutdown_client(client) diff --git a/tests/integration/features/test_server.py b/tests/integration/features/test_server.py index b573f75e..6f305a70 100644 --- a/tests/integration/features/test_server.py +++ b/tests/integration/features/test_server.py @@ -10,6 +10,7 @@ ) from tests.integration.utils import doc +from tests.integration.utils.wait import SETTLE_TIME from tests.integration.utils.workspace import did_change @@ -72,7 +73,7 @@ async def test_semantic_token_modifier_legend(client, workspace): @pytest.mark.workspace("hello_world") async def test_did_open_close_cycle(client, workspace): uri, _ = client.open(workspace / "main.cpp") - await asyncio.sleep(0.5) + await asyncio.sleep(SETTLE_TIME) client.close(uri) @@ -85,8 +86,8 @@ async def test_shutdown_exit(client, workspace): async def test_feature_requests_after_close(client, workspace): uri, _ = client.open(workspace / "main.cpp") client.close(uri) - result = await client.hover_at(uri, 0, 0) - assert result is None + with pytest.raises(Exception, match="Document not open"): + await client.hover_at(uri, 0, 0) @pytest.mark.workspace("hello_world") @@ -96,7 +97,7 @@ async def test_incremental_change(client, workspace): content += f"\n// change {i}" did_change(client, uri, i + 1, content) await asyncio.sleep(0.05) - await asyncio.sleep(1) + await asyncio.sleep(SETTLE_TIME * 2) client.close(uri) @@ -193,23 +194,23 @@ async def test_rapid_changes_stress(client, workspace): for i in range(20): content += f"\n// stress change {i}\n" did_change(client, uri, i + 1, content) - await asyncio.sleep(2) + await asyncio.sleep(SETTLE_TIME * 4) client.close(uri) @pytest.mark.workspace("hello_world") async def test_save_notification(client, workspace): uri, _ = client.open(workspace / "main.cpp") - await asyncio.sleep(0.5) + await asyncio.sleep(SETTLE_TIME) client.text_document_did_save(DidSaveTextDocumentParams(text_document=doc(uri))) - await asyncio.sleep(0.5) + await asyncio.sleep(SETTLE_TIME) client.close(uri) @pytest.mark.workspace("hello_world") async def test_hover_on_unknown_file(client, workspace): - result = await client.hover_at("file:///nonexistent/fake.cpp", 0, 0) - assert result is None + with pytest.raises(Exception, match="Document not open"): + await client.hover_at("file:///nonexistent/fake.cpp", 0, 0) @pytest.mark.workspace("hello_world") diff --git a/tests/integration/lifecycle/test_anomaly.py b/tests/integration/lifecycle/test_anomaly.py new file mode 100644 index 00000000..fa1af9d7 --- /dev/null +++ b/tests/integration/lifecycle/test_anomaly.py @@ -0,0 +1,75 @@ +"""End-to-end check of the anomaly reporting machinery. + +Kills a worker process and verifies the master reports `[anomaly:worker_crash]` +both via window/logMessage and in its log file — the same channels +assert_no_anomaly() watches in every other test's teardown. +""" + +import asyncio +import os +import signal +import sys +from pathlib import Path + +import pytest + +from tests.conftest import make_client, shutdown_client +from tests.integration.utils import write_cdb +from tests.integration.utils.assertions import ( + anomalies_in_log_files, + anomalies_in_log_messages, +) + + +def child_pids(parent_pid: int) -> list[int]: + pids = [] + for entry in Path("/proc").iterdir(): + if not entry.name.isdigit(): + continue + try: + stat = (entry / "stat").read_text() + except OSError: + continue + # /proc//stat: pid (comm) state ppid ... + ppid = int(stat.rsplit(")", 1)[1].split()[1]) + if ppid == parent_pid: + pids.append(int(entry.name)) + return pids + + +@pytest.mark.skipif(sys.platform != "linux", reason="worker discovery uses /proc") +@pytest.mark.allow_anomaly +async def test_worker_crash_reported(executable, tmp_path): + (tmp_path / "main.cpp").write_text("int main() { return 0; }\n") + write_cdb(tmp_path, ["main.cpp"]) + + # Debug builds abort on anomalies by design; disable the trap so this + # test can observe the report-and-continue (Release) behavior everywhere. + os.environ["CLICE_ANOMALY_NO_TRAP"] = "1" + try: + client = await make_client(executable, tmp_path) + finally: + os.environ.pop("CLICE_ANOMALY_NO_TRAP", None) + + try: + await client.open_and_wait(tmp_path / "main.cpp") + + server_pid = client._server.pid + workers = child_pids(server_pid) + assert workers, "server should have spawned worker processes" + os.kill(workers[0], signal.SIGKILL) + + for _ in range(50): + if "worker_crash" in anomalies_in_log_messages(client): + break + await asyncio.sleep(0.2) + assert "worker_crash" in anomalies_in_log_messages(client), ( + f"expected worker_crash anomaly, got messages: " + f"{[m.message for m in client.log_messages]}" + ) + finally: + await shutdown_client(client) + + # The same marker must be greppable in the log files (this is what + # assert_no_anomaly relies on for worker-side anomalies). + assert any("worker_crash" in entry for entry in anomalies_in_log_files(tmp_path)) diff --git a/tests/integration/lifecycle/test_config.py b/tests/integration/lifecycle/test_config.py index a6abaf73..051f7a49 100644 --- a/tests/integration/lifecycle/test_config.py +++ b/tests/integration/lifecycle/test_config.py @@ -6,7 +6,9 @@ """ import pytest +from lsprotocol.types import DiagnosticSeverity +from tests.conftest import make_client, shutdown_client from tests.integration.utils.assertions import ( assert_clean_compile, assert_has_errors, @@ -66,3 +68,61 @@ async def test_init_options_replaces_toml_rules(client, workspace): async def test_rules_pattern_mismatch(client, workspace): uri, _ = await client.open_and_wait(workspace / "main.cpp") assert_has_errors(client, uri, "Rule pattern should not have matched main.cpp") + + +async def test_config_type_error_diagnostic(executable, tmp_path): + # Wrong value type → Error diagnostic on the clice.toml URI with the + # offending line/column; the config falls back to defaults. + (tmp_path / "clice.toml").write_text('[project]\nclang_tidy = "yes"\n') + (tmp_path / "main.cpp").write_text("int main() { return 0; }\n") + client = await make_client(executable, tmp_path) + try: + toml_uri = client.path_to_uri(tmp_path / "clice.toml") + await client.wait_diagnostics(toml_uri, timeout=10) + diags = client.diagnostics[toml_uri] + assert len(diags) == 1, f"expected one config diagnostic: {diags}" + assert diags[0].severity == DiagnosticSeverity.Error + assert diags[0].range.start.line == 1 # clang_tidy is on line 2 (0-based 1) + assert "clang_tidy" in diags[0].message + finally: + await shutdown_client(client) + + +async def test_config_unknown_key_diagnostic(executable, tmp_path): + # Typo'd key → Warning diagnostic; the rest of the config still applies. + (tmp_path / "clice.toml").write_text("[project]\nclang_tdy = true\n") + (tmp_path / "main.cpp").write_text("int main() { return 0; }\n") + client = await make_client(executable, tmp_path) + try: + toml_uri = client.path_to_uri(tmp_path / "clice.toml") + await client.wait_diagnostics(toml_uri, timeout=10) + diags = client.diagnostics[toml_uri] + assert len(diags) == 1, f"expected one config diagnostic: {diags}" + assert diags[0].severity == DiagnosticSeverity.Warning + assert diags[0].range.start.line == 1 + assert "clang_tdy" in diags[0].message + finally: + await shutdown_client(client) + + +async def test_config_diagnostic_clears_after_fix(executable, tmp_path): + (tmp_path / "clice.toml").write_text('[project]\nclang_tidy = "yes"\n') + (tmp_path / "main.cpp").write_text("int main() { return 0; }\n") + client = await make_client(executable, tmp_path) + try: + toml_uri = client.path_to_uri(tmp_path / "clice.toml") + await client.wait_diagnostics(toml_uri, timeout=10) + assert client.diagnostics[toml_uri], "broken config should be diagnosed" + finally: + await shutdown_client(client) + + # Fix the config and restart — the new session publishes an empty list + # for the config URI so stale markers clear. + (tmp_path / "clice.toml").write_text("[project]\nclang_tidy = true\n") + client = await make_client(executable, tmp_path) + try: + toml_uri = client.path_to_uri(tmp_path / "clice.toml") + await client.wait_diagnostics(toml_uri, timeout=10) + assert client.diagnostics[toml_uri] == [], "fixed config must clear diagnostics" + finally: + await shutdown_client(client) diff --git a/tests/integration/lifecycle/test_file_operation.py b/tests/integration/lifecycle/test_file_operation.py index f324ac73..c74406ff 100644 --- a/tests/integration/lifecycle/test_file_operation.py +++ b/tests/integration/lifecycle/test_file_operation.py @@ -13,13 +13,14 @@ ) from tests.integration.utils import doc +from tests.integration.utils.wait import IDLE_TIMEOUT from tests.integration.utils.workspace import did_change @pytest.mark.workspace("hello_world") async def test_did_open(client, workspace): client.open(workspace / "main.cpp") - await asyncio.sleep(5) + await asyncio.sleep(IDLE_TIMEOUT) @pytest.mark.workspace("hello_world") @@ -29,13 +30,13 @@ async def test_did_change(client, workspace): content += "\n" await asyncio.sleep(0.2) did_change(client, uri, i + 1, content) - await asyncio.sleep(5) + await asyncio.sleep(IDLE_TIMEOUT) @pytest.mark.workspace("clang_tidy") async def test_clang_tidy(client, workspace): client.open(workspace / "main.cpp") - await asyncio.sleep(5) + await asyncio.sleep(IDLE_TIMEOUT) @pytest.mark.workspace("hello_world") @@ -56,7 +57,7 @@ async def test_hover_save_close(client, workspace): ) ) client.text_document_did_close(DidCloseTextDocumentParams(text_document=doc(uri))) - closed_hover = await client.text_document_hover_async( - HoverParams(text_document=doc(uri), position=Position(line=0, character=0)) - ) - assert closed_hover is None + with pytest.raises(Exception, match="Document not open"): + await client.text_document_hover_async( + HoverParams(text_document=doc(uri), position=Position(line=0, character=0)) + ) diff --git a/tests/integration/modules/test_modules.py b/tests/integration/modules/test_modules.py index b5690c51..edb37d9b 100644 --- a/tests/integration/modules/test_modules.py +++ b/tests/integration/modules/test_modules.py @@ -14,6 +14,7 @@ ) from tests.integration.utils.assertions import assert_clean_compile, assert_has_errors +from tests.integration.utils.wait import IDLE_TIMEOUT @pytest.mark.workspace("modules/single_module_no_deps") @@ -267,7 +268,7 @@ async def test_circular_module_dependency(client, workspace): the server remains responsive by opening a non-cyclic file afterwards. """ client.open(workspace / "cycle_a.cppm") - await asyncio.sleep(5.0) + await asyncio.sleep(IDLE_TIMEOUT) uri_ok, _ = await client.open_and_wait(workspace / "ok.cppm") diags = client.diagnostics.get(uri_ok, []) diff --git a/tests/integration/stress/test_rapid_edit.py b/tests/integration/stress/test_rapid_edit.py index 70ac4762..7c32a08f 100644 --- a/tests/integration/stress/test_rapid_edit.py +++ b/tests/integration/stress/test_rapid_edit.py @@ -10,6 +10,7 @@ ) from tests.integration.utils import doc +from tests.integration.utils.wait import SETTLE_TIME from tests.integration.utils.workspace import did_change @@ -53,7 +54,7 @@ async def test_rapid_edits_with_hover(client, workspace): await asyncio.sleep(0.02) # ~20ms between edits # Wait a moment for in-flight requests to settle. - await asyncio.sleep(1.0) + await asyncio.sleep(SETTLE_TIME * 2) # Final hover must succeed and return correct result. final_hover = await asyncio.wait_for( diff --git a/tests/integration/utils/assertions.py b/tests/integration/utils/assertions.py index a2ed7b3c..e2803272 100644 --- a/tests/integration/utils/assertions.py +++ b/tests/integration/utils/assertions.py @@ -1,7 +1,58 @@ -"""Diagnostic assertion helpers for integration tests.""" +"""Diagnostic and anomaly assertion helpers for integration tests.""" + +import re +from pathlib import Path from lsprotocol.types import Diagnostic, DiagnosticSeverity +ANOMALY_PATTERN = re.compile(r"\[anomaly:([a-z_]+)\]") + + +def anomalies_in_log_messages(client) -> list[str]: + """Anomaly IDs from window/logMessage notifications (master process).""" + found = [] + for msg in client.log_messages: + match = ANOMALY_PATTERN.search(msg.message) + if match: + found.append(match.group(1)) + return found + + +def anomalies_in_log_files(workspace: Path | None) -> list[str]: + """Anomaly IDs from master/worker log files under /.clice/logs. + + Workers don't forward logMessage in v1.0, so their anomalies are only + visible in their log files. + """ + if workspace is None: + return [] + logs_dir = Path(workspace) / ".clice" / "logs" + if not logs_dir.exists(): + return [] + found = [] + for log_file in sorted(logs_dir.rglob("*.log")): + for line in log_file.read_text(errors="replace").splitlines(): + match = ANOMALY_PATTERN.search(line) + if match: + found.append(f"{match.group(1)} ({log_file.name}: {line.strip()})") + return found + + +def assert_no_anomaly(client, workspace: Path | None = None) -> None: + """Assert the session produced zero anomalies (client messages + logs). + + Runs in every integration test teardown: anomalies are internal clice + bugs and must never occur on regular paths. + """ + found = anomalies_in_log_messages(client) + found += anomalies_in_log_files(workspace) + assert not found, f"clice reported internal anomalies: {found}" + + +def guidance_messages(client) -> list[str]: + """Guidance texts from window/logMessage notifications.""" + return [msg.message for msg in client.log_messages if "[guidance]" in msg.message] + def get_errors(diagnostics: list[Diagnostic]) -> list[Diagnostic]: """Filter diagnostics to errors only (severity == 1).""" diff --git a/tests/integration/utils/client.py b/tests/integration/utils/client.py index 8a6882b9..49133c77 100644 --- a/tests/integration/utils/client.py +++ b/tests/integration/utils/client.py @@ -7,6 +7,7 @@ from lsprotocol.types import ( PROGRESS, TEXT_DOCUMENT_PUBLISH_DIAGNOSTICS, + WINDOW_LOG_MESSAGE, WINDOW_WORK_DONE_PROGRESS_CREATE, ClientCapabilities, CodeActionContext, @@ -27,6 +28,7 @@ InitializeParams, InitializeResult, InitializedParams, + LogMessageParams, Position, ProgressParams, PublishDiagnosticsParams, @@ -51,9 +53,11 @@ def __init__(self) -> None: super().__init__("clice-test-client", "0.1.0") self.diagnostics: dict[str, list[Diagnostic]] = {} self.diagnostics_events: dict[str, asyncio.Event] = {} + self.log_messages: list[LogMessageParams] = [] self.progress_tokens: list[str] = [] self.progress_events: list[dict] = [] self.init_result: InitializeResult | None = None + self.workspace: Path | None = None @self.feature(TEXT_DOCUMENT_PUBLISH_DIAGNOSTICS) def on_diagnostics(params: PublishDiagnosticsParams) -> None: @@ -67,6 +71,10 @@ def on_diagnostics(params: PublishDiagnosticsParams) -> None: if key in self.diagnostics_events: self.diagnostics_events[key].set() + @self.feature(WINDOW_LOG_MESSAGE) + def on_log_message(params: LogMessageParams) -> None: + self.log_messages.append(params) + @self.feature(WINDOW_WORK_DONE_PROGRESS_CREATE) def on_create_progress(params: WorkDoneProgressCreateParams) -> None: token = str(params.token) if isinstance(params.token, int) else params.token @@ -110,6 +118,7 @@ async def initialize( result = await self.initialize_async(params) self.initialized(InitializedParams()) self.init_result = result + self.workspace = workspace return result # ── Document operations ────────────────────────────────────────── diff --git a/tests/integration/utils/wait.py b/tests/integration/utils/wait.py index 91d4a26b..06a07779 100644 --- a/tests/integration/utils/wait.py +++ b/tests/integration/utils/wait.py @@ -9,6 +9,11 @@ WorkspaceSymbolParams, ) +# Standard timing constants — use these instead of hardcoded sleep values. +MTIME_GRANULARITY = 1.1 # Filesystem mtime precision (1s on some FSes, +0.1 margin) +SETTLE_TIME = 0.5 # Time for the server to stabilize after an operation +IDLE_TIMEOUT = 5.0 # Idle soak time in lifecycle tests + async def wait_for_recompile(client, uri: str, *, timeout: float = 60.0) -> None: """Trigger recompilation via hover and wait for fresh diagnostics. diff --git a/tests/pytest.ini b/tests/pytest.ini index ebf65ab1..02b425ab 100644 --- a/tests/pytest.ini +++ b/tests/pytest.ini @@ -5,3 +5,4 @@ filterwarnings = markers = workspace init_options + allow_anomaly diff --git a/tests/replay.py b/tests/replay.py index 0de00c27..f81cdf12 100644 --- a/tests/replay.py +++ b/tests/replay.py @@ -147,6 +147,8 @@ async def replay_one( pending: dict[int | str, asyncio.Future] = {} + anomalies: list[str] = [] + async def reader_loop(): try: while True: @@ -154,6 +156,11 @@ async def reader_loop(): if msg is None: break msg_id, method = msg.get("id"), msg.get("method") + if method == "window/logMessage": + text = msg.get("params", {}).get("message", "") + if "[anomaly:" in text: + anomalies.append(text) + print(f" ANOMALY: {text}") if msg_id is not None and method is not None: resp = SERVER_REQUEST_DEFAULTS.get(method) await write_lsp_message( @@ -312,6 +319,12 @@ def print_stderr(): print_stderr() return False + if anomalies: + # Anomalies are internal clice bugs; a replay session must be clean. + print(f" result: FAIL ({len(anomalies)} anomaly report(s), {elapsed:.1f}s)") + print_stderr() + return False + print(f" result: PASS ({elapsed:.1f}s)") return True diff --git a/tests/unit/feature/document_link_tests.cpp b/tests/unit/feature/document_link_tests.cpp index 9da70053..84ce533a 100644 --- a/tests/unit/feature/document_link_tests.cpp +++ b/tests/unit/feature/document_link_tests.cpp @@ -21,11 +21,6 @@ void run(llvm::StringRef source, llvm::StringRef standard = "-std=c++17") { links = feature::document_links(*unit, feature::PositionEncoding::UTF8); } -auto to_local_range(const protocol::Range& range) -> LocalSourceRange { - feature::PositionMapper converter(unit->interested_content(), feature::PositionEncoding::UTF8); - return LocalSourceRange(*converter.to_offset(range.start), *converter.to_offset(range.end)); -} - void EXPECT_LINK(std::size_t index, llvm::StringRef name, llvm::StringRef path) { auto& link = links[index]; auto expected = range(name, "main.cpp"); diff --git a/tests/unit/feature/folding_range_tests.cpp b/tests/unit/feature/folding_range_tests.cpp index b7a75a55..93447d6f 100644 --- a/tests/unit/feature/folding_range_tests.cpp +++ b/tests/unit/feature/folding_range_tests.cpp @@ -37,19 +37,10 @@ void run(llvm::StringRef code) { } auto to_local_range(const protocol::FoldingRange& range) -> LocalSourceRange { - feature::PositionMapper converter(unit->interested_content(), feature::PositionEncoding::UTF8); - - auto start = protocol::Position{ - .line = range.start_line, - .character = range.start_character.value_or(0), - }; - - auto end = protocol::Position{ - .line = range.end_line, - .character = range.end_character.value_or(0), - }; - - return LocalSourceRange(*converter.to_offset(start), *converter.to_offset(end)); + return Tester::to_local_range(protocol::Range{ + .start = {.line = range.start_line, .character = range.start_character.value_or(0)}, + .end = {.line = range.end_line, .character = range.end_character.value_or(0) }, + }); } void EXPECT_FOLDING(std::uint32_t index, diff --git a/tests/unit/test/tester.h b/tests/unit/test/tester.h index 247144f2..ad074b9a 100644 --- a/tests/unit/test/tester.h +++ b/tests/unit/test/tester.h @@ -10,6 +10,7 @@ #include "command/command.h" #include "command/toolchain.h" #include "compile/compilation.h" +#include "feature/feature.h" #include "support/logging.h" namespace clice::testing { @@ -93,6 +94,15 @@ struct Tester { LocalSourceRange range(llvm::StringRef name = "", llvm::StringRef file = ""); + /// Convert a protocol range back to byte offsets in the compiled unit's + /// interested file. Shared by feature tests that compare protocol results + /// against annotation ranges. + LocalSourceRange to_local_range(const kota::ipc::protocol::Range& range) { + feature::PositionMapper converter(unit->interested_content(), + feature::PositionEncoding::UTF8); + return LocalSourceRange(*converter.to_offset(range.start), *converter.to_offset(range.end)); + } + void clear(); }; From cd1bdd7847402b4dc449962d64c4781fead26aab Mon Sep 17 00:00:00 2001 From: ykiko Date: Fri, 12 Jun 2026 11:59:00 +0800 Subject: [PATCH 5/6] fix(server): correct config diagnostic URIs and command tier fallthrough Pre-PR review findings: - publish_config_diagnostics groups issues by their own file URI; a malformed clice.toml no longer renders its error on the next config candidate that loaded. - A failed switchContext header context now falls through to the real CDB entry before reaching the synthesized fallback. - Header-context host selection filters hosts through has_entry(); lookup() never returns empty (it synthesizes), so the old emptiness checks were dead and hosts without real entries were mislabeled as IncludeGraph. - Style: string_view message parameters, llvm::join, doc comments. --- src/server/compiler/compiler.cpp | 47 +++++++++++++++---------------- src/server/compiler/compiler.h | 1 + src/server/protocol/worker.h | 2 +- src/server/service/lsp_client.cpp | 35 ++++++++++++++--------- src/support/anomaly.cpp | 8 +++--- src/support/anomaly.h | 4 +-- 6 files changed, 52 insertions(+), 45 deletions(-) diff --git a/src/server/compiler/compiler.cpp b/src/server/compiler/compiler.cpp index 6805de95..b23c291d 100644 --- a/src/server/compiler/compiler.cpp +++ b/src/server/compiler/compiler.cpp @@ -18,6 +18,7 @@ #include "kota/codec/json/json.h" #include "kota/ipc/lsp/position.h" #include "kota/ipc/lsp/uri.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Path.h" @@ -174,14 +175,8 @@ static void log_command_decision(llvm::StringRef path, llvm::ArrayRef tried, CommandSource source, llvm::ArrayRef arguments) { - if(clice::logging::options.level > clice::logging::Level::info) + if(logging::options.level > logging::Level::info) return; - std::string tried_text; - for(auto tier: tried) { - if(!tried_text.empty()) - tried_text += ","; - tried_text += tier; - } std::string joined; for(auto& arg: arguments) { joined += arg; @@ -189,7 +184,7 @@ static void log_command_decision(llvm::StringRef path, } LOG_INFO("compile_args: file={} tried=[{}] source={} args_hash={:016x}", path, - tried_text, + llvm::join(tried, ","), command_source_name(source), llvm::xxh3_64bits(llvm::StringRef(joined))); } @@ -209,10 +204,13 @@ CommandSource Compiler::fill_compile_args(llvm::StringRef path, log_command_decision(path, tried, CommandSource::IncludeGraph, arguments); return CommandSource::IncludeGraph; } - } else if(workspace.cdb.has_entry(path)) { - // 2. Real CDB entry for the file itself. - // Apply rules from config (append/remove flags based on file patterns). - tried.push_back("cdb"); + } + + // 2. Real CDB entry for the file itself (lookup() synthesizes a command + // for unknown files, so a non-empty result alone proves nothing). + // Apply rules from config (append/remove flags based on file patterns). + tried.push_back("cdb"); + if(workspace.cdb.has_entry(path)) { std::vector rule_append, rule_remove; workspace.config.match_rules(path, rule_append, rule_remove); auto results = workspace.cdb.lookup(path, {.remove = rule_remove, .append = rule_append}); @@ -222,9 +220,10 @@ CommandSource Compiler::fill_compile_args(llvm::StringRef path, arguments = cmd.to_string_argv(); log_command_decision(path, tried, CommandSource::CdbExact, arguments); return CommandSource::CdbExact; - } else { - // 3. No CDB entry — try automatic header context resolution. - tried.push_back("cdb"); + } + + // 3. No CDB entry — try automatic header context resolution. + if(!(session && session->active_context.has_value())) { tried.push_back("include_graph"); if(fill_header_context_args(path, path_id, directory, arguments, session)) { log_command_decision(path, tried, CommandSource::IncludeGraph, arguments); @@ -284,16 +283,17 @@ bool Compiler::fill_header_context_args(llvm::StringRef path, } auto host_path = workspace.path_pool.resolve(ctx_ptr->host_path_id); + if(!workspace.cdb.has_entry(host_path)) { + LOG_WARN("fill_header_context_args: host {} has no CDB entry", host_path); + return false; + } + // Apply rules matching the HEADER path (what the user is editing) on top of // the host's command — rules are expected to apply uniformly to every file. std::vector rule_append, rule_remove; workspace.config.match_rules(path, rule_append, rule_remove); auto host_results = workspace.cdb.lookup(host_path, {.remove = rule_remove, .append = rule_append}); - if(host_results.empty()) { - LOG_WARN("fill_header_context_args: host {} has no CDB entry", host_path); - return false; - } workspace.toolchain.resolve_or_warn(host_results.front()); @@ -334,8 +334,7 @@ std::optional Compiler::resolve_header_context(std::uint32_t if(session && session->active_context.has_value()) { auto preferred = *session->active_context; auto preferred_path = workspace.path_pool.resolve(preferred); - auto results = workspace.cdb.lookup(preferred_path); - if(!results.empty()) { + if(workspace.cdb.has_entry(preferred_path)) { auto c = workspace.dep_graph.find_include_chain(preferred, header_path_id); if(!c.empty()) { host_path_id = preferred; @@ -344,12 +343,12 @@ std::optional Compiler::resolve_header_context(std::uint32_t } } - // Fall back to the first available host that has a CDB entry. + // Fall back to the first available host that has a real CDB entry — + // a host with a synthesized command would just be a fallback in disguise. if(chain.empty()) { for(auto candidate: hosts) { auto candidate_path = workspace.path_pool.resolve(candidate); - auto results = workspace.cdb.lookup(candidate_path); - if(results.empty()) + if(!workspace.cdb.has_entry(candidate_path)) continue; auto c = workspace.dep_graph.find_include_chain(candidate, header_path_id); if(c.empty()) diff --git a/src/server/compiler/compiler.h b/src/server/compiler/compiler.h index 8dccf06f..6fbf0199 100644 --- a/src/server/compiler/compiler.h +++ b/src/server/compiler/compiler.h @@ -47,6 +47,7 @@ enum class CommandSource : std::uint8_t { Fallback, }; +/// Stable snake_case name of a CommandSource, used in the decision log. llvm::StringRef command_source_name(CommandSource source); /// Compilation service — drives worker processes to build ASTs, PCHs, and PCMs. diff --git a/src/server/protocol/worker.h b/src/server/protocol/worker.h index 60914568..329630de 100644 --- a/src/server/protocol/worker.h +++ b/src/server/protocol/worker.h @@ -102,7 +102,7 @@ struct BuildResult { bool success = true; std::string error; /// On failure: whether `error` carries user-code compile errors. A failure - /// withOUT user errors indicates clice infrastructure breakage (anomaly). + /// without user errors indicates clice infrastructure breakage (anomaly). bool has_user_errors = false; std::string output_path; ///< PCH or PCM path std::vector deps; diff --git a/src/server/service/lsp_client.cpp b/src/server/service/lsp_client.cpp index 2705792a..9cfdb274 100644 --- a/src/server/service/lsp_client.cpp +++ b/src/server/service/lsp_client.cpp @@ -720,22 +720,18 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s }); } -/// Publish clice.toml load problems as diagnostics on the config file's URI. -/// The file is usually not open in the editor — publishing to a closed file -/// is fine, the client shows it in the problems panel. A clean load publishes -/// an empty list so diagnostics from a previous (broken) state clear. +/// Publish clice.toml load problems as diagnostics, each on its own file's +/// URI (multiple files can contribute issues when the first config candidate +/// is malformed and the next one loads). The files are usually not open in +/// the editor — publishing to a closed file is fine, the client shows it in +/// the problems panel. The loaded file always gets a publish, so a clean +/// load clears diagnostics from a previous (broken) state. void LSPClient::publish_config_diagnostics() { if(server.config_path.empty()) return; - auto uri = lsp::URI::from_file_path(server.config_path); - if(!uri) { - LOG_WARN("Cannot build URI for config file {}", server.config_path); - return; - } - - protocol::PublishDiagnosticsParams params; - params.uri = uri->str(); + llvm::StringMap> by_file; + by_file[server.config_path]; for(auto& issue: server.config_issues) { // rich_error positions are 1-based; LSP wants 0-based. An unknown // position (0) maps to the file top. The range spans a single @@ -753,11 +749,22 @@ void LSPClient::publish_config_diagnostics() { : protocol::DiagnosticSeverity::Warning; diagnostic.source = "clice"; diagnostic.message = issue.message; - params.diagnostics.push_back(std::move(diagnostic)); + by_file[issue.file].push_back(std::move(diagnostic)); LOG_GUIDANCE("Configuration problem in {}: {}", issue.file, issue.message); } - peer.send_notification(params); + + for(auto& [file, diagnostics]: by_file) { + auto uri = lsp::URI::from_file_path(file.str()); + if(!uri) { + LOG_WARN("Cannot build URI for config file {}", file.str()); + continue; + } + protocol::PublishDiagnosticsParams params; + params.uri = uri->str(); + params.diagnostics = std::move(diagnostics); + peer.send_notification(params); + } } LSPClient::~LSPClient() { diff --git a/src/support/anomaly.cpp b/src/support/anomaly.cpp index 488cc5e6..ff1668ee 100644 --- a/src/support/anomaly.cpp +++ b/src/support/anomaly.cpp @@ -72,7 +72,7 @@ bool anomaly_should_report(AnomalyId id) { return false; } -void report_anomaly(AnomalyId id, std::string message, std::source_location location) { +void report_anomaly(AnomalyId id, std::string_view message, std::source_location location) { auto text = std::format("[anomaly:{}] {}", anomaly_name(id), message); logging::log(spdlog::level::err, location, "{}", text); if(notify_hook) @@ -84,7 +84,7 @@ bool guidance_should_report() { return options.level <= Level::warn; } -void report_guidance(std::string message, std::source_location location) { +void report_guidance(std::string_view message, std::source_location location) { auto text = std::format("[guidance] {}", message); logging::log(spdlog::level::warn, location, "{}", text); if(notify_hook) @@ -95,8 +95,8 @@ void set_notify_hook(std::function hook) { notify_hook = std::move(hook); } -void set_anomaly_trap_for_testing(std::function trap) { - testing_trap = std::move(trap); +void set_anomaly_trap_for_testing(std::function hook) { + testing_trap = std::move(hook); } void reset_anomaly_for_testing() { diff --git a/src/support/anomaly.h b/src/support/anomaly.h index 540790a9..adb93856 100644 --- a/src/support/anomaly.h +++ b/src/support/anomaly.h @@ -56,7 +56,7 @@ bool anomaly_should_report(AnomalyId id); /// trap is overridden for tests or CLICE_ANOMALY_NO_TRAP is set in the /// environment; Release builds continue. void report_anomaly(AnomalyId id, - std::string message, + std::string_view message, std::source_location location = std::source_location::current()); /// Gate evaluated BEFORE the format arguments of LOG_GUIDANCE. @@ -65,7 +65,7 @@ bool guidance_should_report(); /// Log "[guidance] " at warn level and forward it to the notify hook. /// For user-actionable situations that are not clice bugs (missing CDB, /// invalid configuration, ...). -void report_guidance(std::string message, +void report_guidance(std::string_view message, std::source_location location = std::source_location::current()); /// Severity forwarded to the notify hook; values mirror LSP MessageType. From 340f9dd53d86eee2fd3f5d46fae7fb01f35a8bd1 Mon Sep 17 00:00:00 2001 From: ykiko Date: Fri, 12 Jun 2026 11:59:00 +0800 Subject: [PATCH 6/6] test: close review gaps in error-feedback coverage - All seven AnomalyIds fire through the macro once (marker names are wire-stable for the integration greps). - InvalidParams for an out-of-range hover position; error response for an unresolvable call hierarchy item. - Column assertions for config issue locations. - assert_no_anomaly in the agentic local fixtures and make_client-based tests that bypass the client fixture teardown. --- tests/conftest.py | 2 +- tests/integration/agentic/test_agentic.py | 3 +- tests/integration/agentic/test_cli.py | 3 +- .../compilation/test_persistent_cache.py | 4 ++- .../features/test_guidance_diagnostics.py | 1 + tests/integration/features/test_index.py | 21 +++++++++++++ tests/integration/features/test_server.py | 7 +++++ tests/integration/lifecycle/test_config.py | 6 ++++ tests/unit/server/config_tests.cpp | 2 ++ tests/unit/support/anomaly_tests.cpp | 30 ++++++++++++++++++- 10 files changed, 74 insertions(+), 5 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 549f760a..892836e5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -77,7 +77,7 @@ def test_data_dir() -> Path: @pytest.fixture -def workspace(request: pytest.FixtureRequest, test_data_dir: Path) -> Path | None: +def workspace(request: pytest.FixtureRequest, test_data_dir: Path): """Resolve workspace path from @pytest.mark.workspace("subdir") marker. If the workspace contains a CMakeLists.txt, automatically runs cmake diff --git a/tests/integration/agentic/test_agentic.py b/tests/integration/agentic/test_agentic.py index 5e47487f..e10dbc49 100644 --- a/tests/integration/agentic/test_agentic.py +++ b/tests/integration/agentic/test_agentic.py @@ -143,7 +143,7 @@ def do_request(_): async def indexed_agentic(request, executable, workspace): """Start server with LSP+agentic, compile a file, wait for indexing.""" from tests.integration.utils.client import CliceClient - from tests.conftest import _shutdown_client, _find_free_port + from tests.conftest import _check_no_anomaly, _shutdown_client, _find_free_port host = "127.0.0.1" port = _find_free_port() @@ -173,6 +173,7 @@ async def indexed_agentic(request, executable, workspace): rpc.close() c.close(uri) await _shutdown_client(c) + _check_no_anomaly(request, c) @pytest.mark.workspace("index_features") diff --git a/tests/integration/agentic/test_cli.py b/tests/integration/agentic/test_cli.py index 2989fb74..7c989ae3 100644 --- a/tests/integration/agentic/test_cli.py +++ b/tests/integration/agentic/test_cli.py @@ -31,7 +31,7 @@ async def indexed_server(request, executable, workspace): """Start server with LSP+agentic, compile a file, wait for indexing.""" import asyncio from tests.integration.utils.client import CliceClient - from tests.conftest import _shutdown_client, _find_free_port + from tests.conftest import _check_no_anomaly, _shutdown_client, _find_free_port host = "127.0.0.1" port = _find_free_port() @@ -60,6 +60,7 @@ async def indexed_server(request, executable, workspace): c.close(uri) await _shutdown_client(c) + _check_no_anomaly(request, c) @pytest.mark.workspace("index_features") diff --git a/tests/integration/compilation/test_persistent_cache.py b/tests/integration/compilation/test_persistent_cache.py index a7b3b6a6..f27bddc2 100644 --- a/tests/integration/compilation/test_persistent_cache.py +++ b/tests/integration/compilation/test_persistent_cache.py @@ -22,7 +22,7 @@ list_pcm_files, read_cache_json, ) -from tests.integration.utils.assertions import assert_clean_compile +from tests.integration.utils.assertions import assert_clean_compile, assert_no_anomaly def _pin_cache_to_workspace(tmp_path): @@ -138,6 +138,7 @@ async def test_pch_survives_server_restart(executable, tmp_path): cache_s1 = read_cache_json(tmp_path) assert cache_s1 is not None, "cache.json should exist after session 1" + assert_no_anomaly(c1, tmp_path) await shutdown_client(c1) # Session 2: restart server, reopen file. @@ -156,6 +157,7 @@ async def test_pch_survives_server_restart(executable, tmp_path): "PCH file should not be rebuilt (mtime should be unchanged)" ) + assert_no_anomaly(c2, tmp_path) await shutdown_client(c2) diff --git a/tests/integration/features/test_guidance_diagnostics.py b/tests/integration/features/test_guidance_diagnostics.py index e3284f5d..a4a060ec 100644 --- a/tests/integration/features/test_guidance_diagnostics.py +++ b/tests/integration/features/test_guidance_diagnostics.py @@ -67,5 +67,6 @@ async def test_fallback_clean_no_guidance(executable, tmp_path): try: uri, _ = await client.open_and_wait(tmp_path / "main.cpp") assert not guidance_diags(client, uri) + assert_no_anomaly(client, tmp_path) finally: await shutdown_client(client) diff --git a/tests/integration/features/test_index.py b/tests/integration/features/test_index.py index 7686dfc7..4cbab493 100644 --- a/tests/integration/features/test_index.py +++ b/tests/integration/features/test_index.py @@ -4,9 +4,12 @@ import pytest from lsprotocol.types import ( CallHierarchyIncomingCallsParams, + CallHierarchyItem, CallHierarchyOutgoingCallsParams, CallHierarchyPrepareParams, Position, + Range, + SymbolKind, TypeHierarchyPrepareParams, TypeHierarchySubtypesParams, TypeHierarchySupertypesParams, @@ -75,6 +78,24 @@ async def test_call_hierarchy_prepare(client, workspace): client.close(uri) +@pytest.mark.workspace("index_features") +async def test_call_hierarchy_bogus_item(client, workspace): + """incomingCalls with an unresolvable item returns an error, not null.""" + uri, _ = await client.open_and_wait(workspace / "main.cpp") + bogus = CallHierarchyItem( + name="ghost", + kind=SymbolKind.Function, + uri="file:///nonexistent/ghost.cpp", + range=Range(start=Position(0, 0), end=Position(0, 5)), + selection_range=Range(start=Position(0, 0), end=Position(0, 5)), + ) + with pytest.raises(Exception, match="Failed to resolve call hierarchy item"): + await client.call_hierarchy_incoming_calls_async( + CallHierarchyIncomingCallsParams(item=bogus) + ) + client.close(uri) + + @pytest.mark.workspace("index_features") async def test_call_hierarchy_incoming(client, workspace): """Test incomingCalls shows compute() calls add().""" diff --git a/tests/integration/features/test_server.py b/tests/integration/features/test_server.py index 6f305a70..2a935fb5 100644 --- a/tests/integration/features/test_server.py +++ b/tests/integration/features/test_server.py @@ -213,6 +213,13 @@ async def test_hover_on_unknown_file(client, workspace): await client.hover_at("file:///nonexistent/fake.cpp", 0, 0) +@pytest.mark.workspace("hello_world") +async def test_hover_out_of_range_position(client, workspace): + uri, _ = await client.open_and_wait(workspace / "main.cpp") + with pytest.raises(Exception, match="Invalid position"): + await client.hover_at(uri, 99999, 0) + + @pytest.mark.workspace("hello_world") async def test_all_features_after_compile_wait(client, workspace): """Exercise all feature requests after compilation completes.""" diff --git a/tests/integration/lifecycle/test_config.py b/tests/integration/lifecycle/test_config.py index 051f7a49..2a147ea4 100644 --- a/tests/integration/lifecycle/test_config.py +++ b/tests/integration/lifecycle/test_config.py @@ -12,6 +12,7 @@ from tests.integration.utils.assertions import ( assert_clean_compile, assert_has_errors, + assert_no_anomaly, get_errors, ) @@ -83,7 +84,9 @@ async def test_config_type_error_diagnostic(executable, tmp_path): assert len(diags) == 1, f"expected one config diagnostic: {diags}" assert diags[0].severity == DiagnosticSeverity.Error assert diags[0].range.start.line == 1 # clang_tidy is on line 2 (0-based 1) + assert diags[0].range.start.character > 0 assert "clang_tidy" in diags[0].message + assert_no_anomaly(client, tmp_path) finally: await shutdown_client(client) @@ -100,7 +103,9 @@ async def test_config_unknown_key_diagnostic(executable, tmp_path): assert len(diags) == 1, f"expected one config diagnostic: {diags}" assert diags[0].severity == DiagnosticSeverity.Warning assert diags[0].range.start.line == 1 + assert diags[0].range.start.character > 0 assert "clang_tdy" in diags[0].message + assert_no_anomaly(client, tmp_path) finally: await shutdown_client(client) @@ -124,5 +129,6 @@ async def test_config_diagnostic_clears_after_fix(executable, tmp_path): toml_uri = client.path_to_uri(tmp_path / "clice.toml") await client.wait_diagnostics(toml_uri, timeout=10) assert client.diagnostics[toml_uri] == [], "fixed config must clear diagnostics" + assert_no_anomaly(client, tmp_path) finally: await shutdown_client(client) diff --git a/tests/unit/server/config_tests.cpp b/tests/unit/server/config_tests.cpp index 75bc2212..1392d953 100644 --- a/tests/unit/server/config_tests.cpp +++ b/tests/unit/server/config_tests.cpp @@ -390,6 +390,7 @@ TEST_CASE(TypeIssueHasLocation) { ASSERT_EQ(issues.size(), 1u); EXPECT_EQ(issues[0].severity, ConfigIssue::Severity::Error); EXPECT_EQ(issues[0].line, 2u); + EXPECT_EQ(issues[0].column, 14u); // the "yes" value node EXPECT_NE(issues[0].message.find("clang_tidy"), std::string::npos); } @@ -403,6 +404,7 @@ TEST_CASE(UnknownKeyIssueWarns) { ASSERT_EQ(issues.size(), 1u); EXPECT_EQ(issues[0].severity, ConfigIssue::Severity::Warning); EXPECT_EQ(issues[0].line, 2u); + EXPECT_EQ(issues[0].column, 13u); // the value node of the unknown key EXPECT_NE(issues[0].message.find("clang_tdy"), std::string::npos); } diff --git a/tests/unit/support/anomaly_tests.cpp b/tests/unit/support/anomaly_tests.cpp index bce237c0..0d9fe7b7 100644 --- a/tests/unit/support/anomaly_tests.cpp +++ b/tests/unit/support/anomaly_tests.cpp @@ -69,7 +69,7 @@ TEST_CASE(RateLimitSuppresses) { EXPECT_EQ(capture.trapped.size(), logging::anomaly_report_limit); } -TEST_CASE(RateLimitIsPerId) { +TEST_CASE(RateLimitPerId) { AnomalyCapture capture; for(std::uint32_t i = 0; i < logging::anomaly_report_limit + 5; ++i) { @@ -141,6 +141,34 @@ TEST_CASE(GuidanceLazyAtLevel) { EXPECT_EQ(capture.notified.size(), 0u); } +TEST_CASE(MarkerNamesStable) { + /// Every id fires through the macro once and produces its wire marker. + /// Integration tests grep these exact strings — keep them stable. + AnomalyCapture capture; + + LOG_ANOMALY(PchBuildFail, "x"); + LOG_ANOMALY(PcmBuildFail, "x"); + LOG_ANOMALY(CompileFail, "x"); + LOG_ANOMALY(WorkerRequestFail, "x"); + LOG_ANOMALY(WorkerCrash, "x"); + LOG_ANOMALY(WorkerSpawnFail, "x"); + LOG_ANOMALY(PositionMapFail, "x"); + + ASSERT_EQ(capture.notified.size(), logging::anomaly_id_count); + const char* expected[] = { + "pch_build_fail", + "pcm_build_fail", + "compile_fail", + "worker_request_fail", + "worker_crash", + "worker_spawn_fail", + "position_map_fail", + }; + for(std::size_t i = 0; i < logging::anomaly_id_count; ++i) { + EXPECT_EQ(capture.notified[i].second, std::format("[anomaly:{}] x", expected[i])); + } +} + }; // TEST_SUITE(Anomaly) } // namespace