diff --git a/src/cli/main.cpp b/src/cli/main.cpp index a2a2d46e..7a86520a 100644 --- a/src/cli/main.cpp +++ b/src/cli/main.cpp @@ -1,4 +1,5 @@ #include "cgraph/atomic_write.hpp" +#include "cgraph/cli_support.hpp" #include "cgraph/change_context.hpp" #include "cgraph/client_runtime.hpp" #include "cgraph/daemon_endpoint.hpp" @@ -138,14 +139,14 @@ int run_change_context(int argc, char** argv) { else if (arg == "--expected-base-content-root") params["expected_base_content_root"] = value; else if (arg == "--expected-target-content-root") params["expected_target_content_root"] = value; else if (arg == "--budget" || arg == "--max-depth") { - std::size_t consumed = 0; - const auto number = std::stoll(value, &consumed); - if (consumed != value.size()) throw std::invalid_argument("invalid numeric argument"); - params[arg == "--budget" ? "budget" : "max_depth"] = number; + params[arg == "--budget" ? "budget" : "max_depth"] = cgraph::integer_flag(arg, value); } else throw std::invalid_argument("unknown argument: " + arg); } std::cout << cgraph::change_context_across_workspace(params, cgraph::ClientRequest{}).dump() << '\n'; return 0; + } catch (const cgraph::UsageError& error) { + std::cout << nlohmann::json{{"error", error.what()}}.dump() << '\n'; + return 2; } catch (const std::exception& error) { std::cout << nlohmann::json{{"error", error.what()}}.dump() << '\n'; return 1; @@ -158,7 +159,7 @@ int run_build(const Args& args) { // Sidecar stats.json (durable, diffable) deliberately kept out of graph.json // so the Graphify node-link parity golden stays byte-identical. - std::filesystem::create_directories(args.output); + cgraph::create_output_directories(args.output); cgraph::write_file_atomically(args.output / "stats.json", cgraph::build_stats_json(result.stats).dump(2)); std::cerr << "processed " << result.file_count << " files, wrote exports to " << args.output << '\n'; @@ -209,13 +210,12 @@ int run_enrich_ingest(const Args& args) { return cgraph::parse_iso8601_utc(midnight); } if (spec.size() > 1 && (spec.back() == 'h' || spec.back() == 'd')) { - try { - const long n = std::stol(spec.substr(0, spec.size() - 1)); - const auto hours = std::chrono::hours(spec.back() == 'd' ? n * 24 : n); - return cgraph::WallClock::now() - hours; - } catch (...) { + const auto n = cgraph::parse_integer(std::string_view(spec).substr(0, spec.size() - 1)); + if (!n) { return std::nullopt; } + const auto hours = std::chrono::hours(spec.back() == 'd' ? *n * 24 : *n); + return cgraph::WallClock::now() - hours; } return cgraph::parse_iso8601_utc(spec); } @@ -329,17 +329,17 @@ int run_report(int argc, char** argv) { } else if (arg == "--group-by" && has_value) { request.params["group_by"] = argv[++index]; } else if (arg == "--depth" && has_value) { - request.params["depth"] = std::stoi(argv[++index]); + request.params["depth"] = cgraph::integer_flag(arg, argv[++index]); } else if (arg == "--budget" && has_value) { - request.params["budget"] = std::stoll(argv[++index]); + request.params["budget"] = cgraph::integer_flag(arg, argv[++index]); } else if (arg == "--threshold" && has_value) { - request.params["threshold"] = std::stod(argv[++index]); + request.params["threshold"] = cgraph::number_flag(arg, argv[++index]); } else if (arg == "--min-members" && has_value) { - request.params["min_members"] = std::stoi(argv[++index]); + request.params["min_members"] = cgraph::integer_flag(arg, argv[++index]); } else if (arg == "--min-tokens" && has_value) { - request.params["min_tokens"] = std::stoi(argv[++index]); + request.params["min_tokens"] = cgraph::integer_flag(arg, argv[++index]); } else if (arg == "--hops" && has_value) { - request.params["hops"] = std::stoi(argv[++index]); + request.params["hops"] = cgraph::integer_flag(arg, argv[++index]); } else if (arg == "--include-tests") { request.params["include_tests"] = true; } else if (arg == "--daemon" && has_value) { @@ -444,7 +444,7 @@ int run_seam_gen(int argc, char** argv) { std::cerr << " " << line << '\n'; } - std::filesystem::create_directories(out_dir); + cgraph::create_output_directories(out_dir); const auto out_file = out_dir / "chunk_00.json"; cgraph::write_file_atomically(out_file, cgraph::to_json(result.fragment).dump(2) + '\n'); std::cerr << "seam gen: wrote " << out_file << " (" << result.fragment.nodes.size() @@ -646,7 +646,7 @@ int run_seam_discover(int argc, char** argv) { for (const auto& line : result.resolution_log) { std::cerr << " " << line << '\n'; } - std::filesystem::create_directories(out_dir); + cgraph::create_output_directories(out_dir); const auto out_file = out_dir / "chunk_00.json"; cgraph::write_file_atomically(out_file, cgraph::to_json(result.fragment).dump(2) + '\n'); std::cerr << "seam discover: wrote " << out_file << " (" << result.fragment.nodes.size() << " nodes, " @@ -765,7 +765,7 @@ int run_seam_fuse(int argc, char** argv) { return 1; } - std::filesystem::create_directories(out_dir); + cgraph::create_output_directories(out_dir); cgraph::write_file_atomically(out_dir / "graph.json", cgraph::to_node_link_json(fused.graph).dump(2) + '\n'); cgraph::write_file_atomically(out_dir / "graph.html", cgraph::export_graph_html(fused.graph)); // Marker: tells graphd to serve this dir as a static read-only seam graph. @@ -897,7 +897,7 @@ std::filesystem::path supervisor_config_path() { } else if (arg == "--cgraph" && index + 1 < argc) { config.cgraph_binary = argv[++index]; } else if (arg == "--interval" && index + 1 < argc) { - config.reconcile_interval_seconds = std::stoi(argv[++index]); + config.reconcile_interval_seconds = cgraph::integer_flag(arg, argv[++index]); } else { std::cerr << "cgraph daemon: unknown argument: " << arg << '\n'; return false; @@ -1103,9 +1103,9 @@ int run_drain_command(int argc, char** argv) { for (int index = 3; index < argc; ++index) { const std::string arg = argv[index]; if (arg == "--interval" && index + 1 < argc) { - config.interval_seconds = std::stoi(argv[++index]); + config.interval_seconds = cgraph::integer_flag(arg, argv[++index]); } else if (arg == "--chunk-cap" && index + 1 < argc) { - config.chunk_cap = std::stoi(argv[++index]); + config.chunk_cap = cgraph::integer_flag(arg, argv[++index]); } else if (arg == "--script" && index + 1 < argc) { config.script_path = argv[++index]; } else if (arg == "--launch-agents-dir" && index + 1 < argc) { @@ -1242,11 +1242,20 @@ int run(int argc, char** argv) { int main(int argc, char** argv) { // Every output file goes through write_file_atomically: a full disk or any // other failed write leaves no truncated file behind and must not exit 0. + // A malformed flag value is a usage error (exit 2). Any other exception is + // reported with its type (a filesystem_error's what() also names the + // path(s)) and exits 1; escaping main would abort with a core dump (134). try { return run(argc, argv); + } catch (const cgraph::UsageError& error) { + std::cerr << "cgraph: " << error.what() << '\n'; + return 2; } catch (const cgraph::FileWriteError& error) { std::cerr << "cgraph: " << error.what() << '\n'; return 1; + } catch (const std::exception& error) { + std::cerr << "cgraph: " << cgraph::describe_exception(error) << '\n'; + return 1; } } diff --git a/src/client/main.cpp b/src/client/main.cpp index d7d7d317..92832a40 100644 --- a/src/client/main.cpp +++ b/src/client/main.cpp @@ -1,9 +1,11 @@ +#include "cgraph/cli_support.hpp" #include "cgraph/client_runtime.hpp" #include "cgraph/engine.hpp" #include #include #include +#include #include #include @@ -15,9 +17,7 @@ void print_usage() { " the other services on the far side of each endpoint FILE serves or calls\n"; } -} // namespace - -int main(int argc, char** argv) { +int run(int argc, char** argv) { const auto info = cgraph::build_info(); cgraph::ClientRequest request{ .project_root = std::filesystem::current_path(), @@ -93,3 +93,16 @@ int main(int argc, char** argv) { std::cout << result.response->dump(2) << '\n'; return 0; } + +} // namespace + +int main(int argc, char** argv) { + // Report any unhandled failure (e.g. a filesystem_error resolving the + // project root) and exit non-zero instead of aborting with a core dump. + try { + return run(argc, argv); + } catch (const std::exception& error) { + std::cerr << "cgraph-client: " << cgraph::describe_exception(error) << '\n'; + return 1; + } +} diff --git a/src/daemon/main.cpp b/src/daemon/main.cpp index f0e4064a..f294fb54 100644 --- a/src/daemon/main.cpp +++ b/src/daemon/main.cpp @@ -1,3 +1,4 @@ +#include "cgraph/cli_support.hpp" #include "cgraph/daemon_lifecycle.hpp" #include "cgraph/daemon_ops.hpp" #include "cgraph/daemon_server.hpp" @@ -6,6 +7,7 @@ #include "cgraph/seam.hpp" #include +#include #include #include @@ -16,9 +18,7 @@ void print_usage() { " [--benchmark-query --graph PATH --query TEXT]\n"; } -} // namespace - -int main(int argc, char** argv) { +int run(int argc, char** argv) { const auto info = cgraph::build_info(); std::filesystem::path graph_path; std::filesystem::path root; @@ -42,7 +42,7 @@ int main(int argc, char** argv) { continue; } if (arg == "--idle-timeout" && index + 1 < argc) { - idle_timeout = std::chrono::seconds(std::stoll(argv[++index])); + idle_timeout = std::chrono::seconds(cgraph::integer_flag(arg, argv[++index])); continue; } if (arg == "--no-watch") { @@ -100,3 +100,22 @@ int main(int argc, char** argv) { } return cgraph::run_daemon_server(root, options); } + +} // namespace + +int main(int argc, char** argv) { + // A malformed flag value is a usage error (exit 2). Setup failures outside + // the serve loop (e.g. a filesystem_error creating the socket directory) are + // logged with their type and exit 1; an escaped exception would abort with a + // core dump. Failures inside the serve loop and its worker threads are + // handled by run_daemon_server itself, which shuts down cleanly and returns 1. + try { + return run(argc, argv); + } catch (const cgraph::UsageError& error) { + std::cerr << "graphd: " << error.what() << '\n'; + return 2; + } catch (const std::exception& error) { + std::cerr << "graphd: " << cgraph::describe_exception(error) << '\n'; + return 1; + } +} diff --git a/src/engine/CMakeLists.txt b/src/engine/CMakeLists.txt index 030a007c..af441629 100644 --- a/src/engine/CMakeLists.txt +++ b/src/engine/CMakeLists.txt @@ -3,6 +3,7 @@ add_library(cgraph_engine STATIC atomic_write.cpp body_index.cpp change_context.cpp + cli_support.cpp daemon_endpoint.cpp daemon_server.cpp configured_extractors.cpp diff --git a/src/engine/atomic_write.cpp b/src/engine/atomic_write.cpp index c8e53c06..7285458b 100644 --- a/src/engine/atomic_write.cpp +++ b/src/engine/atomic_write.cpp @@ -253,4 +253,17 @@ void write_file_atomically(const std::filesystem::path& path, std::string_view c } } +void create_output_directories(const std::filesystem::path& directory) { + if (directory.empty()) { + return; + } + std::error_code error; + std::filesystem::create_directories(directory, error); + if (error) { + // error.message(), not failure()'s errno lookup: on Windows the code is a + // Win32 error in system_category, which generic_category would misname. + throw FileWriteError(failure(directory, "cannot create directory (" + error.message() + ")", 0)); + } +} + } // namespace cgraph diff --git a/src/engine/cli_support.cpp b/src/engine/cli_support.cpp new file mode 100644 index 00000000..3856515e --- /dev/null +++ b/src/engine/cli_support.cpp @@ -0,0 +1,49 @@ +#include "cgraph/cli_support.hpp" + +#include +#include +#include +#include +#include + +#if !defined(_MSC_VER) +#include +#endif + +namespace cgraph { + +double number_flag(std::string_view flag, std::string_view value) { + // strtod, not from_chars: libc++ on macOS lacks floating-point from_chars. + const std::string text(value); + char* end = nullptr; + errno = 0; + const double parsed = std::strtod(text.c_str(), &end); + if (text.empty() || std::isspace(static_cast(text.front())) != 0 || + end != text.c_str() + text.size() || errno == ERANGE) { + throw UsageError(std::string(flag) + " expects a number, got '" + text + "'"); + } + return parsed; +} + +std::string describe_exception(const std::exception& error) { +#if defined(_MSC_VER) + // MSVC's type_info::name() is already demangled but keyed ("class std::out_of_range"); + // drop the key so every platform names the type the same way. + std::string_view name = typeid(error).name(); + for (const std::string_view key : {std::string_view{"class "}, std::string_view{"struct "}}) { + if (name.substr(0, key.size()) == key) { + name.remove_prefix(key.size()); + break; + } + } + return std::string(name) + ": " + error.what(); +#else + const char* mangled = typeid(error).name(); + int status = 0; + const std::unique_ptr demangled( + abi::__cxa_demangle(mangled, nullptr, nullptr, &status), &std::free); + return std::string(status == 0 && demangled ? demangled.get() : mangled) + ": " + error.what(); +#endif +} + +} // namespace cgraph diff --git a/src/engine/daemon_server.cpp b/src/engine/daemon_server.cpp index 43cc36bb..89cf711e 100644 --- a/src/engine/daemon_server.cpp +++ b/src/engine/daemon_server.cpp @@ -1,6 +1,7 @@ #include "cgraph/daemon_server.hpp" #include "cgraph/atomic_write.hpp" +#include "cgraph/cli_support.hpp" #include "cgraph/daemon_endpoint.hpp" #include "cgraph/daemon_identity.hpp" #include "cgraph/daemon_lifecycle.hpp" @@ -47,6 +48,19 @@ namespace { // the thin client makes; a slower peer is dropped and the serve loop continues. constexpr std::chrono::seconds kConnectionIoTimeout{5}; +// Runs `cleanup` when the scope exits, by return or by exception. +template +class ScopeExit { + public: + explicit ScopeExit(Cleanup cleanup) : cleanup_(std::move(cleanup)) {} + ~ScopeExit() { cleanup_(); } + ScopeExit(const ScopeExit&) = delete; + ScopeExit& operator=(const ScopeExit&) = delete; + + private: + Cleanup cleanup_; +}; + // Reads one length-prefixed frame (4-byte little-endian length + body) and // decodes it, reusing the shared protocol codec. [[nodiscard]] std::optional read_frame(const LocalSocket& socket, const SocketDeadline& deadline = {}) { @@ -736,23 +750,37 @@ int run_daemon_server(const std::filesystem::path& root, DaemonServerOptions opt std::atomic initial_build_done{false}; state.watching = watch_code && options.build_graph_on_start; + // Set when the serve loop or a worker thread fails with an exception: the + // failure is logged, the loop stops, and the daemon exits 1 after the same + // cleanup as a normal shutdown. An exception must never leave a worker + // thread (std::terminate) or unwind past a joinable one (std::terminate). + std::atomic fatal_error{false}; + const auto record_fatal = [&](std::string_view where, const std::exception& error) { + std::cerr << "graphd: " << where << " failed: " << describe_exception(error) << '\n'; + fatal_error.store(true); + }; + // Build on a worker thread so the accept loop below starts serving at once. // rescan() locks graph_mutex; status/query answer from the empty snapshot until // the build publishes the real one. std::thread build_thread; if (options.build_graph_on_start) { build_thread = std::thread([&] { - if (watch_code) { - (void)code_watcher.poll(FileWatcherClock::now()); // prime: baseline only, no events - } - if (!try_load_persisted()) { - (void)rescan(); + try { + if (watch_code) { + (void)code_watcher.poll(FileWatcherClock::now()); // prime: baseline only, no events + } + if (!try_load_persisted()) { + (void)rescan(); + } + // Plan enrichment after the initial build/load to populate current health. + // Later doc/media changes, drop ingests, and code-dependency reconciliation + // request another plan from the persisted stat/hash index. + request_refresh(); + initial_build_done.store(true); // hands the watcher to the serve loop + } catch (const std::exception& error) { + record_fatal("initial build", error); } - // Plan enrichment after the initial build/load to populate current health. - // Later doc/media changes, drop ingests, and code-dependency reconciliation - // request another plan from the persisted stat/hash index. - request_refresh(); - initial_build_done.store(true); // hands the watcher to the serve loop }); } @@ -768,158 +796,184 @@ int run_daemon_server(const std::filesystem::path& root, DaemonServerOptions opt } refresh_requested = false; } - run_enrichment_refresh(); + try { + run_enrichment_refresh(); + } catch (const std::exception& error) { + record_fatal("enrichment refresh", error); + return; + } } }); + // However this scope exits -- a normal shutdown, or an exception from the + // post-loop flush -- join both workers (they capture locals by reference) + // and release the endpoint. + const ScopeExit stop_workers([&] { + if (build_thread.joinable()) { + build_thread.join(); + } + { + const std::scoped_lock lock(refresh_mutex); + refresh_stop = true; + } + refresh_cv.notify_one(); + enrichment_worker.join(); + listener = LocalSocket{}; // close before removing the socket file + (void)cleanup_daemon_endpoint(socket_path); + }); + (void)drop_watcher.poll(FileWatcherClock::now()); // prime: existing drops are already overlaid std::cerr << "graphd listening on " << socket_path << " for root " << identity.project_root << '\n'; auto last_activity = FileWatcherClock::now(); auto last_code_poll = FileWatcherClock::now(); - while (!state.shutdown_requested) { - const auto ready = wait_for_client(listener, options.drop_poll_interval); - if (ready == SocketWait::Failed) { - break; - } - - // Ingest any debounced fragment drops discovered since the last tick. Not - // polled until the initial build publishes: the build thread holds - // graph_mutex across semantic replay (which can block on a source read), so - // taking it here would wedge the accept loop -- the exact hang the persist - // step's try_to_lock avoids. Drops that land mid-build stay pending in the - // watcher and are picked up on the first post-build poll; the build's own - // rebuild_final_overlay already discovers on-disk drops directly. - if (const auto events = (!options.build_graph_on_start || initial_build_done.load()) - ? drop_watcher.poll(FileWatcherClock::now()) - : std::vector{}; - !events.empty()) { - const std::scoped_lock lock(graph_mutex); // serialize with the build/rescan thread - // Mark the batch in-flight so a concurrent `status` reports enrichment as - // running; the scope clears the running count on exit and request_refresh - // recomputes the steady state below. - const EnrichmentRunningScope running(state, events.size()); - rebuild_final_overlay(&events); - request_refresh(); - last_activity = FileWatcherClock::now(); - } + try { + while (!state.shutdown_requested && !fatal_error.load()) { + const auto ready = wait_for_client(listener, options.drop_poll_interval); + if (ready == SocketWait::Failed) { + break; + } - // Fold live code changes into the graph. Each watcher poll walks the project - // tree, so it runs on its own (slower) cadence than the drop poll, and only - // once the initial build has published a baseline. - if (watch_code && initial_build_done.load() && - FileWatcherClock::now() - last_code_poll >= options.code_poll_interval) { - last_code_poll = FileWatcherClock::now(); - const auto events = code_watcher.poll(last_code_poll); - bool code_changed = false; - std::vector semantic_source_events; - for (const auto& event : events) { - if (event.change == FileWatchChange::Overflow || event.kind == WatchedFileKind::Code) { - code_changed = true; - } else { - semantic_source_events.push_back(event); - } + // Ingest any debounced fragment drops discovered since the last tick. Not + // polled until the initial build publishes: the build thread holds + // graph_mutex across semantic replay (which can block on a source read), so + // taking it here would wedge the accept loop -- the exact hang the persist + // step's try_to_lock avoids. Drops that land mid-build stay pending in the + // watcher and are picked up on the first post-build poll; the build's own + // rebuild_final_overlay already discovers on-disk drops directly. + if (const auto events = (!options.build_graph_on_start || initial_build_done.load()) + ? drop_watcher.poll(FileWatcherClock::now()) + : std::vector{}; + !events.empty()) { + const std::scoped_lock lock(graph_mutex); // serialize with the build/rescan thread + // Mark the batch in-flight so a concurrent `status` reports enrichment as + // running; the scope clears the running count on exit and request_refresh + // recomputes the steady state below. + const EnrichmentRunningScope running(state, events.size()); + rebuild_final_overlay(&events); + request_refresh(); + last_activity = FileWatcherClock::now(); } - if (code_changed) { - // Neighborhood dedup keeps each incremental update fast, but it skips - // the fuzzy-duplicate merges a full pass makes elsewhere in the graph, - // so the node set drifts above the canonical build's. Reconciling with - // a full dedup every Nth update bounds that drift; an explicit `update` - // op or a restart rescan also reconverges it. - constexpr std::size_t kFullDedupReconcileEvery = 5; - const std::scoped_lock lock(graph_mutex); - IncrementalUpdateResult result; - DaemonState hydration_state; - const bool hydration = !index_hydrated.load(); - if (!hydration) { - result = apply_incremental_code_updates( - state, index, events, IncrementalDedupPolicy{.full_reconcile_every = kFullDedupReconcileEvery}); - } else { - // First edit after a fast-load restart: hydrate the index with one - // full rescan (a per-file rebuild here would wipe the graph), staged - // like every rescan so readers never see an intermediate; subsequent - // events go incremental. - hydration_state.graph_snapshot = read_graph_snapshot(state); // body-index reuse, as in rescan - result = full_stat_index_rescan(hydration_state, index, identity.project_root); - } - for (const auto& warning : result.warnings) { - std::cerr << "graphd: " << warning << '\n'; + + // Fold live code changes into the graph. Each watcher poll walks the project + // tree, so it runs on its own (slower) cadence than the drop poll, and only + // once the initial build has published a baseline. + if (watch_code && initial_build_done.load() && + FileWatcherClock::now() - last_code_poll >= options.code_poll_interval) { + last_code_poll = FileWatcherClock::now(); + const auto events = code_watcher.poll(last_code_poll); + bool code_changed = false; + std::vector semantic_source_events; + for (const auto& event : events) { + if (event.change == FileWatchChange::Overflow || event.kind == WatchedFileKind::Code) { + code_changed = true; + } else { + semantic_source_events.push_back(event); + } } - if (result.applied) { - if (hydration) { - { - const std::scoped_lock enrichment_lock(state.enrichment_mutex); - state.unextracted = hydration_state.unextracted; - state.route_resolution = hydration_state.route_resolution; - state.last_files_cache_hit = hydration_state.last_files_cache_hit; - state.last_extract_mean_ms = hydration_state.last_extract_mean_ms; - } - index_hydrated.store(true); - deterministic_graph = *read_graph_snapshot(hydration_state); + if (code_changed) { + // Neighborhood dedup keeps each incremental update fast, but it skips + // the fuzzy-duplicate merges a full pass makes elsewhere in the graph, + // so the node set drifts above the canonical build's. Reconciling with + // a full dedup every Nth update bounds that drift; an explicit `update` + // op or a restart rescan also reconverges it. + constexpr std::size_t kFullDedupReconcileEvery = 5; + const std::scoped_lock lock(graph_mutex); + IncrementalUpdateResult result; + DaemonState hydration_state; + const bool hydration = !index_hydrated.load(); + if (!hydration) { + result = apply_incremental_code_updates( + state, index, events, IncrementalDedupPolicy{.full_reconcile_every = kFullDedupReconcileEvery}); } else { - deterministic_graph = *read_graph_snapshot(state); + // First edit after a fast-load restart: hydrate the index with one + // full rescan (a per-file rebuild here would wipe the graph), staged + // like every rescan so readers never see an intermediate; subsequent + // events go incremental. + hydration_state.graph_snapshot = read_graph_snapshot(state); // body-index reuse, as in rescan + result = full_stat_index_rescan(hydration_state, index, identity.project_root); + } + for (const auto& warning : result.warnings) { + std::cerr << "graphd: " << warning << '\n'; } + if (result.applied) { + if (hydration) { + { + const std::scoped_lock enrichment_lock(state.enrichment_mutex); + state.unextracted = hydration_state.unextracted; + state.route_resolution = hydration_state.route_resolution; + state.last_files_cache_hit = hydration_state.last_files_cache_hit; + state.last_extract_mean_ms = hydration_state.last_extract_mean_ms; + } + index_hydrated.store(true); + deterministic_graph = *read_graph_snapshot(hydration_state); + } else { + deterministic_graph = *read_graph_snapshot(state); + } + rebuild_final_overlay(); + request_refresh(); // code dependencies may have requeued semantic sources + ++state.incremental_updates; + mark_graph_dirty(lifecycle, DaemonClock::now()); + std::cerr << "graphd: incremental update (" << result.files_reextracted << " re-extracted, " + << result.files_removed << " removed" << (result.full_rescan ? ", full rescan" : "") << ")\n"; + } + last_activity = FileWatcherClock::now(); // active editing keeps the daemon alive + } + if (!semantic_source_events.empty()) { + const std::scoped_lock lock(graph_mutex); + for (const auto& event : semantic_source_events) { + stat_index.erase(normalize_semantic_source_path(event.path)); + } + ++stat_index_revision; + persist_stat_index(stat_index, stat_index_path); rebuild_final_overlay(); - request_refresh(); // code dependencies may have requeued semantic sources - ++state.incremental_updates; - mark_graph_dirty(lifecycle, DaemonClock::now()); - std::cerr << "graphd: incremental update (" << result.files_reextracted << " re-extracted, " - << result.files_removed << " removed" << (result.full_rescan ? ", full rescan" : "") << ")\n"; + request_refresh(); + last_activity = FileWatcherClock::now(); } - last_activity = FileWatcherClock::now(); // active editing keeps the daemon alive } - if (!semantic_source_events.empty()) { - const std::scoped_lock lock(graph_mutex); - for (const auto& event : semantic_source_events) { - stat_index.erase(normalize_semantic_source_path(event.path)); + + // Re-persist graph + manifest once incremental changes have aged past the + // persist interval, so a crash loses at most that window. try_to_lock: the + // initial build (or a long overlay replay) holds graph_mutex, and the serve + // loop must keep answering from the current snapshot while it runs -- a + // skipped persist is retried on the next loop pass. + bool persisted_incremental = false; + { + const std::unique_lock lock(graph_mutex, std::try_to_lock); + if (lock.owns_lock()) { + persisted_incremental = + persist_if_due(deterministic_graph, lifecycle, lifecycle_config, DaemonClock::now()); + if (persisted_incremental) { + persist_manifest(); + } } - ++stat_index_revision; - persist_stat_index(stat_index, stat_index_path); - rebuild_final_overlay(); - request_refresh(); - last_activity = FileWatcherClock::now(); } - } + if (persisted_incremental) { + std::cerr << "graphd: persisted incremental graph state\n"; + } - // Re-persist graph + manifest once incremental changes have aged past the - // persist interval, so a crash loses at most that window. try_to_lock: the - // initial build (or a long overlay replay) holds graph_mutex, and the serve - // loop must keep answering from the current snapshot while it runs -- a - // skipped persist is retried on the next loop pass. - bool persisted_incremental = false; - { - const std::unique_lock lock(graph_mutex, std::try_to_lock); - if (lock.owns_lock()) { - persisted_incremental = - persist_if_due(deterministic_graph, lifecycle, lifecycle_config, DaemonClock::now()); - if (persisted_incremental) { - persist_manifest(); + if (ready == SocketWait::TimedOut) { + // A non-positive idle timeout disables idle shutdown entirely: the daemon + // stays resident (and watching) until an explicit shutdown op or signal. + if (options.idle_timeout > std::chrono::seconds::zero() && + FileWatcherClock::now() - last_activity >= options.idle_timeout) { + std::cerr << "graphd: idle timeout, shutting down\n"; + break; } + continue; // no connection this tick; keep polling drops } - } - if (persisted_incremental) { - std::cerr << "graphd: persisted incremental graph state\n"; - } - if (ready == SocketWait::TimedOut) { - // A non-positive idle timeout disables idle shutdown entirely: the daemon - // stays resident (and watching) until an explicit shutdown op or signal. - if (options.idle_timeout > std::chrono::seconds::zero() && - FileWatcherClock::now() - last_activity >= options.idle_timeout) { - std::cerr << "graphd: idle timeout, shutting down\n"; + // The I/O timeout keeps a stalled peer from wedging the serve loop. + const auto conn = accept_local_connection(listener, kConnectionIoTimeout); + if (!conn.valid()) { break; } - continue; // no connection this tick; keep polling drops - } - - // The I/O timeout keeps a stalled peer from wedging the serve loop. - const auto conn = accept_local_connection(listener, kConnectionIoTimeout); - if (!conn.valid()) { - break; + last_activity = FileWatcherClock::now(); + answer_connection(state, conn); } - last_activity = FileWatcherClock::now(); - answer_connection(state, conn); + } catch (const std::exception& error) { + record_fatal("serve loop", error); } if (build_thread.joinable()) { @@ -927,7 +981,9 @@ int run_daemon_server(const std::filesystem::path& root, DaemonServerOptions opt } // Flush any deterministic incremental state whose persist interval had not // elapsed. Semantic cache and memory sidecars are written on their own paths. - if (lifecycle.graph_dirty) { + // Skipped after a failure: the index may be half-updated, and a manifest + // persisted from it could let the next start fast-load a mismatched graph. + if (lifecycle.graph_dirty && !fatal_error.load()) { const std::scoped_lock lock(graph_mutex); persist_graph_and_manifest(); std::cerr << "graphd: persisted incremental graph state on exit\n"; @@ -946,15 +1002,7 @@ int run_daemon_server(const std::filesystem::path& root, DaemonServerOptions opt std::cerr << "graphd: op-stats ledger flush failed (non-fatal)\n"; } } - { - const std::scoped_lock lock(refresh_mutex); - refresh_stop = true; - } - refresh_cv.notify_one(); - enrichment_worker.join(); // also captures locals by reference; join before scope exit - listener = LocalSocket{}; // close before removing the socket file - (void)cleanup_daemon_endpoint(socket_path); - return 0; + return fatal_error.load() ? 1 : 0; // stop_workers joins the workers and releases the endpoint } std::optional request_over_unix_socket( diff --git a/src/engine/include/cgraph/atomic_write.hpp b/src/engine/include/cgraph/atomic_write.hpp index 00aa3217..519d4789 100644 --- a/src/engine/include/cgraph/atomic_write.hpp +++ b/src/engine/include/cgraph/atomic_write.hpp @@ -48,4 +48,11 @@ enum class WriteDurability { void write_file_atomically(const std::filesystem::path& path, std::string_view contents, WriteDurability durability = WriteDurability::Atomic); +// Creates `directory` and any missing parents so output files can be written +// into it. Throws FileWriteError naming `directory` and the OS reason (e.g. +// "Permission denied") on failure, so an unwritable output location fails +// like any other output write instead of escaping as std::filesystem_error. +// An empty `directory` is the current directory and needs no creating. +void create_output_directories(const std::filesystem::path& directory); + } // namespace cgraph diff --git a/src/engine/include/cgraph/cli_support.hpp b/src/engine/include/cgraph/cli_support.hpp new file mode 100644 index 00000000..faf9b395 --- /dev/null +++ b/src/engine/include/cgraph/cli_support.hpp @@ -0,0 +1,52 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace cgraph { + +// A malformed command line. what() is the message without a program prefix; +// each executable's main prints `: ` and exits 2. +class UsageError : public std::invalid_argument { + public: + using std::invalid_argument::invalid_argument; +}; + +// The whole of `text` as a base-10 T, or nullopt when it has stray characters +// ("5x"), is empty, or does not fit T. +template +[[nodiscard]] std::optional parse_integer(std::string_view text) { + T value{}; + const auto* end = text.data() + text.size(); + const auto [ptr, error] = std::from_chars(text.data(), end, value); + if (text.empty() || error != std::errc{} || ptr != end) { + return std::nullopt; + } + return value; +} + +// The value of integer flag `flag`, or UsageError +// " expects an integer, got ''". +template +[[nodiscard]] T integer_flag(std::string_view flag, std::string_view value) { + if (const auto parsed = parse_integer(value)) { + return *parsed; + } + throw UsageError(std::string(flag) + " expects an integer, got '" + std::string(value) + "'"); +} + +// The value of numeric flag `flag`, or UsageError +// " expects a number, got ''". +[[nodiscard]] double number_flag(std::string_view flag, std::string_view value); + +// ": ", e.g. "std::out_of_range: map::at", for +// last-resort handlers where the type is the only clue to where it came from. +[[nodiscard]] std::string describe_exception(const std::exception& error); + +} // namespace cgraph diff --git a/src/engine/pipeline.cpp b/src/engine/pipeline.cpp index ec1844cb..2919fdd4 100644 --- a/src/engine/pipeline.cpp +++ b/src/engine/pipeline.cpp @@ -26,7 +26,7 @@ namespace cgraph { namespace { void write_text(const std::filesystem::path& path, std::string_view contents) { - std::filesystem::create_directories(path.parent_path()); + create_output_directories(path.parent_path()); write_file_atomically(path, contents); } @@ -122,7 +122,7 @@ PipelineResult run_one_shot(const std::filesystem::path& root) { void write_exports(const GraphSnapshot& graph, const std::filesystem::path& output_dir, const std::filesystem::path& project_root) { - std::filesystem::create_directories(output_dir); + create_output_directories(output_dir); write_text(output_dir / "graph.json", to_node_link_json(graph).dump(2)); // A tenth sidecar, deliberately NOT part of graph.json: consumers need to // tell "deliberately ignored" from "failed to extract", and graph.json is a diff --git a/src/engine/semantic_cache.cpp b/src/engine/semantic_cache.cpp index e9690012..5ccf352c 100644 --- a/src/engine/semantic_cache.cpp +++ b/src/engine/semantic_cache.cpp @@ -433,7 +433,7 @@ namespace { std::optional write_semantic_cache( const SemanticCache& cache, const std::filesystem::path& path) { - std::filesystem::create_directories(path.parent_path()); + create_output_directories(path.parent_path()); auto records = nlohmann::json::array(); for (const auto& record : cache.records()) { records.push_back(record_to_json(record)); diff --git a/src/engine/semantic_chunk_plan.cpp b/src/engine/semantic_chunk_plan.cpp index 2aaa5d9f..72cab799 100644 --- a/src/engine/semantic_chunk_plan.cpp +++ b/src/engine/semantic_chunk_plan.cpp @@ -178,7 +178,7 @@ SemanticChunkPlan plan_semantic_chunks( } void write_semantic_stat_index(const SemanticStatIndex& index, const std::filesystem::path& path) { - std::filesystem::create_directories(path.parent_path()); + create_output_directories(path.parent_path()); // Emit in sorted key order so the file is deterministic across writes. std::vector keys; keys.reserve(index.size()); diff --git a/src/mcp/main.cpp b/src/mcp/main.cpp index 0f5af42c..9308ab3a 100644 --- a/src/mcp/main.cpp +++ b/src/mcp/main.cpp @@ -1,8 +1,10 @@ +#include "cgraph/cli_support.hpp" #include "cgraph/client_runtime.hpp" #include "cgraph/mcp_server.hpp" #include "cgraph/platform.hpp" #include +#include #include #include #include @@ -10,7 +12,9 @@ #include #include -int main(int argc, char** argv) { +namespace { + +int run(int argc, char** argv) { // JSON-RPC over stdio is newline-delimited: no "\r\n" translation on Windows. cgraph::use_binary_stdio(); cgraph::ClientRequest base; @@ -80,3 +84,17 @@ int main(int argc, char** argv) { } return 0; } + +} // namespace + +int main(int argc, char** argv) { + // Per-request failures are answered as JSON-RPC errors (handle_mcp_request); + // anything else (e.g. a filesystem_error resolving the working directory at + // startup) is reported and exits non-zero instead of aborting. + try { + return run(argc, argv); + } catch (const std::exception& error) { + std::cerr << "cgraph-mcp: " << cgraph::describe_exception(error) << '\n'; + return 1; + } +} diff --git a/src/mcp/mcp_server.cpp b/src/mcp/mcp_server.cpp index 3db5bfdf..8b3c501c 100644 --- a/src/mcp/mcp_server.cpp +++ b/src/mcp/mcp_server.cpp @@ -297,48 +297,9 @@ namespace { return {}; } -} // namespace - -std::vector mcp_tool_names() { - std::vector names; - for (const auto& tool : tools()) { - names.push_back(tool.at("name").get()); - } - return names; -} - -bool should_prewarm(const std::filesystem::path& project_root) { - std::error_code error; - return std::filesystem::exists(project_root / ".git", error) || - std::filesystem::is_directory(project_root / "cgraph-out", error); -} - -std::set parse_mcp_tool_list(const std::string& value) { - const auto known = mcp_tool_names(); - std::set names; - std::size_t start = 0; - while (start <= value.size()) { - const auto comma = std::min(value.find(',', start), value.size()); - auto name = value.substr(start, comma - start); - const auto first = name.find_first_not_of(" \t"); - name = first == std::string::npos ? std::string{} : name.substr(first, name.find_last_not_of(" \t") - first + 1); - if (name == "all") { - return {}; - } - if (!name.empty()) { - if (std::find(known.begin(), known.end(), name) == known.end()) { - throw std::invalid_argument("unknown tool '" + name + "'"); - } - names.insert(std::move(name)); - } - start = comma + 1; - } - return names; -} - -nlohmann::json handle_mcp_request(const nlohmann::json& request, const McpForwarder& forwarder, - const McpChangeContext& change_context_runner, - const std::set& listed_tools) { +[[nodiscard]] nlohmann::json respond(const nlohmann::json& request, const McpForwarder& forwarder, + const McpChangeContext& change_context_runner, + const std::set& listed_tools) { const auto id = request.value("id", nlohmann::json(nullptr)); const auto method = request.value("method", std::string{}); if (request.value("jsonrpc", std::string{}) != "2.0" || method.empty()) { @@ -390,7 +351,14 @@ nlohmann::json handle_mcp_request(const nlohmann::json& request, const McpForwar return error_response(id, -32603, "missing daemon forwarder"); } - const auto daemon_response = forwarder(daemon_request); + // A forwarder that throws (a filesystem or runtime failure reaching the + // daemon) fails this request only; the stdio loop keeps serving. + nlohmann::json daemon_response; + try { + daemon_response = forwarder(daemon_request); + } catch (const std::exception& error) { + return error_response(id, -32603, error.what()); + } if (!daemon_response.value("ok", false)) { if (name == "graph_report") { if (const auto hint = report_upgrade_hint(daemon_response)) { @@ -402,4 +370,66 @@ nlohmann::json handle_mcp_request(const nlohmann::json& request, const McpForwar return response(id, text_content(daemon_response.value("result", nlohmann::json::object()))); } +} // namespace + +std::vector mcp_tool_names() { + std::vector names; + for (const auto& tool : tools()) { + names.push_back(tool.at("name").get()); + } + return names; +} + +bool should_prewarm(const std::filesystem::path& project_root) { + std::error_code error; + return std::filesystem::exists(project_root / ".git", error) || + std::filesystem::is_directory(project_root / "cgraph-out", error); +} + +std::set parse_mcp_tool_list(const std::string& value) { + const auto known = mcp_tool_names(); + std::set names; + std::size_t start = 0; + while (start <= value.size()) { + const auto comma = std::min(value.find(',', start), value.size()); + auto name = value.substr(start, comma - start); + const auto first = name.find_first_not_of(" \t"); + name = first == std::string::npos ? std::string{} : name.substr(first, name.find_last_not_of(" \t") - first + 1); + if (name == "all") { + return {}; + } + if (!name.empty()) { + if (std::find(known.begin(), known.end(), name) == known.end()) { + throw std::invalid_argument("unknown tool '" + name + "'"); + } + names.insert(std::move(name)); + } + start = comma + 1; + } + return names; +} + +nlohmann::json handle_mcp_request(const nlohmann::json& request, const McpForwarder& forwarder, + const McpChangeContext& change_context_runner, + const std::set& listed_tools) { + // A parseable line of the wrong shape (`5`, `[1]`, a non-string "method" or + // non-object "params") makes nlohmann throw on the first typed read. That is + // an Invalid Request for this line only; the stdio loop keeps serving. The + // response echoes the request's id when it is readable (an object member + // that is a string, number or null) so the host can match it to its call; + // otherwise the id is null, as JSON-RPC 2.0 requires. + try { + return respond(request, forwarder, change_context_runner, listed_tools); + } catch (const nlohmann::json::exception& error) { + nlohmann::json id = nullptr; + if (request.is_object()) { + if (const auto it = request.find("id"); + it != request.end() && (it->is_string() || it->is_number() || it->is_null())) { + id = *it; + } + } + return error_response(id, -32600, std::string{"invalid JSON-RPC request: "} + error.what()); + } +} + } // namespace cgraph diff --git a/tests/smoke/CMakeLists.txt b/tests/smoke/CMakeLists.txt index e8f17ef0..fc7018ac 100644 --- a/tests/smoke/CMakeLists.txt +++ b/tests/smoke/CMakeLists.txt @@ -846,6 +846,34 @@ add_test(NAME cgraph_cli_main_test # Exits 77 on Windows: its full-disk cases need RLIMIT_FSIZE (see the test). set_tests_properties(cgraph_cli_main_test PROPERTIES SKIP_RETURN_CODE 77) +add_executable(cgraph_mcp_main_test + mcp_main_test.cpp) + +target_link_libraries(cgraph_mcp_main_test + PRIVATE + nlohmann_json::nlohmann_json) + +cgraph_set_warnings(cgraph_mcp_main_test) +cgraph_enable_sanitizers(cgraph_mcp_main_test) + +add_test(NAME cgraph_mcp_main_test + COMMAND cgraph_mcp_main_test + $) +# Exits 77 on Windows: its stdio harness needs fork and pipe (see the test). +set_tests_properties(cgraph_mcp_main_test PROPERTIES SKIP_RETURN_CODE 77) + +add_executable(cgraph_cli_support_test + cli_support_test.cpp) + +target_link_libraries(cgraph_cli_support_test + PRIVATE + cgraph::engine) + +cgraph_set_warnings(cgraph_cli_support_test) +cgraph_enable_sanitizers(cgraph_cli_support_test) + +add_test(NAME cgraph_cli_support_test COMMAND cgraph_cli_support_test) + add_executable(cgraph_atomic_write_test atomic_write_test.cpp) diff --git a/tests/smoke/atomic_write_test.cpp b/tests/smoke/atomic_write_test.cpp index 703bd962..407f7b6a 100644 --- a/tests/smoke/atomic_write_test.cpp +++ b/tests/smoke/atomic_write_test.cpp @@ -251,6 +251,33 @@ int main(int argc, char** argv) { } expect(ok, threw, "write_file_atomically throws FileWriteError naming the path"); + // create_output_directories: an empty path is the current directory (no-op), + // an existing directory is fine, nested missing ones are created, and a + // read-only parent raises FileWriteError in write_file_atomically's style -- + // never a std::filesystem::filesystem_error. + cgraph::create_output_directories({}); + cgraph::create_output_directories(root); + const auto nested = root / "a" / "b"; + cgraph::create_output_directories(nested); + expect(ok, fs::is_directory(nested), "create_output_directories creates missing parents"); +#ifndef _WIN32 // the read-only parent relies on POSIX directory permissions + if (::geteuid() != 0) { // root bypasses directory permissions + const auto read_only = root / "read-only"; + fs::create_directories(read_only); + fs::permissions(read_only, fs::perms::owner_read | fs::perms::owner_exec, fs::perm_options::replace); + const auto blocked = read_only / "out"; + std::string message; + try { + cgraph::create_output_directories(blocked); + } catch (const cgraph::FileWriteError& error) { + message = error.what(); + } + expect(ok, message == "failed to write " + blocked.string() + ": cannot create directory (Permission denied)", + ("create_output_directories under a read-only parent throws FileWriteError: " + message).c_str()); + fs::permissions(read_only, fs::perms::owner_all, fs::perm_options::replace); + } +#endif + fs::remove_all(root); if (ok) { std::cout << "atomic_write_test: ok\n"; diff --git a/tests/smoke/cli_main_test.cpp b/tests/smoke/cli_main_test.cpp index 81ea2bbe..808e11a5 100644 --- a/tests/smoke/cli_main_test.cpp +++ b/tests/smoke/cli_main_test.cpp @@ -186,6 +186,45 @@ int main(int argc, char** argv) { expect(ok, !fs::exists(limited_out / "graph.html"), "one-shot build leaves no truncated graph.html"); expect(ok, no_temp_files(limited_out), "one-shot build leaves no temp file"); + // --out under a read-only directory: creating the output directory fails. + // That must exit 1 naming the path, not escape as an uncaught + // std::filesystem::filesystem_error that aborts with a core dump (134). + // Skipped as root, which bypasses directory permissions. + if (::geteuid() != 0) { + const auto read_only = root / "read-only"; + fs::create_directories(read_only); + fs::permissions(read_only, fs::perms::owner_read | fs::perms::owner_exec, fs::perm_options::replace); + const auto blocked = read_only / "out"; + + auto blocked_fuse = fuse_inputs; + blocked_fuse.insert(blocked_fuse.end(), {"--out", blocked.string()}); + run = run_cgraph(cgraph, blocked_fuse, kUnlimited); + expect(ok, run.exit_code == 1, "seam fuse into a read-only parent exits 1 (got " + + std::to_string(run.exit_code) + "): " + run.stderr_text); + expect(ok, contains(run.stderr_text, "cgraph: failed to write " + blocked.string() + ": cannot create directory"), + "seam fuse names the directory it cannot create: " + run.stderr_text); + expect(ok, contains(run.stderr_text, "Permission denied"), "seam fuse gives the OS reason: " + run.stderr_text); + + run = run_cgraph(cgraph, {"--root", fixture.string(), "--out", blocked.string()}, kUnlimited); + expect(ok, run.exit_code == 1, "one-shot build into a read-only parent exits 1 (got " + + std::to_string(run.exit_code) + "): " + run.stderr_text); + expect(ok, contains(run.stderr_text, "cgraph: failed to write " + blocked.string() + ": cannot create directory"), + "one-shot build names the directory it cannot create: " + run.stderr_text); + expect(ok, !fs::exists(blocked), "no output directory appears under the read-only parent"); + + fs::permissions(read_only, fs::perms::owner_all, fs::perm_options::replace); + } else { + std::cout << "cli_main_test: read-only --out case skipped (running as root)\n"; + } + + // A malformed numeric flag is a usage error (exit 2) naming the flag and the + // value; std::stoi threw std::invalid_argument and aborted (134). + run = run_cgraph(cgraph, {"report", "modules", "--root", fixture.string(), "--depth", "abc"}, kUnlimited); + expect(ok, run.exit_code == 2, "report --depth abc exits 2 (got " + std::to_string(run.exit_code) + "): " + + run.stderr_text); + expect(ok, contains(run.stderr_text, "cgraph: --depth expects an integer, got 'abc'"), + "report --depth abc names the flag and value: " + run.stderr_text); + fs::remove_all(root); if (ok) { std::cout << "cli_main_test: ok\n"; diff --git a/tests/smoke/cli_support_test.cpp b/tests/smoke/cli_support_test.cpp new file mode 100644 index 00000000..b5c9b909 --- /dev/null +++ b/tests/smoke/cli_support_test.cpp @@ -0,0 +1,79 @@ +#include "cgraph/cli_support.hpp" + +#include +#include +#include +#include +#include + +namespace { + +void expect(bool& ok, bool condition, const std::string& what) { + if (!condition) { + std::cerr << "FAIL: " << what << '\n'; + ok = false; + } +} + +// The UsageError message `call` throws, or "". +template +std::string usage_error_of(Call call) { + try { + (void)call(); + } catch (const cgraph::UsageError& error) { + return error.what(); + } + return ""; +} + +} // namespace + +int main() { + bool ok = true; + + // parse_integer takes the whole text or nothing: std::stoi accepted "5x" as 5. + expect(ok, cgraph::parse_integer("42") == 42, "parse_integer reads 42"); + expect(ok, cgraph::parse_integer("-7") == -7, "parse_integer reads -7"); + expect(ok, !cgraph::parse_integer("5x"), "parse_integer rejects trailing characters"); + expect(ok, !cgraph::parse_integer(" 5"), "parse_integer rejects leading space"); + expect(ok, !cgraph::parse_integer(""), "parse_integer rejects empty text"); + expect(ok, !cgraph::parse_integer("99999999999"), "parse_integer rejects a value outside int"); + expect(ok, cgraph::parse_integer("99999999999") == 99999999999LL, "parse_integer reads a long long"); + + expect(ok, cgraph::integer_flag("--depth", "3") == 3, "integer_flag returns the value"); + const auto depth_error = usage_error_of([] { return cgraph::integer_flag("--depth", "abc"); }); + expect(ok, depth_error == "--depth expects an integer, got 'abc'", "integer_flag names flag and value: " + depth_error); + const auto range_error = usage_error_of([] { return cgraph::integer_flag("--hops", "99999999999"); }); + expect(ok, range_error == "--hops expects an integer, got '99999999999'", "integer_flag rejects overflow: " + range_error); + + expect(ok, cgraph::number_flag("--threshold", "0.85") == 0.85, "number_flag returns the value"); + const auto threshold_error = usage_error_of([] { return cgraph::number_flag("--threshold", "0.8x"); }); + expect(ok, threshold_error == "--threshold expects a number, got '0.8x'", + "number_flag rejects trailing characters: " + threshold_error); + expect(ok, usage_error_of([] { return cgraph::number_flag("--threshold", ""); }) != "", + "number_flag rejects empty text"); + + // describe_exception leads with the demangled dynamic type. + try { + const std::map empty; + (void)empty.at(1); + } catch (const std::exception& error) { + const auto text = cgraph::describe_exception(error); + expect(ok, text == std::string("std::out_of_range: ") + error.what(), "describe_exception: " + text); + } + try { + throw std::filesystem::filesystem_error("status", std::filesystem::path("/x"), + std::make_error_code(std::errc::permission_denied)); + } catch (const std::exception& error) { + const auto text = cgraph::describe_exception(error); + // The namespace and path quoting are library-specific (libc++: + // std::__1::__fs::filesystem:: and ["/x"]; libstdc++: [/x]). + expect(ok, text.find("filesystem_error: ") != std::string::npos && text.find("/x") != std::string::npos, + "describe_exception names filesystem_error and keeps the path: " + text); + } + + if (ok) { + std::cout << "cli_support_test: ok\n"; + } + return ok ? 0 : 1; +} diff --git a/tests/smoke/daemon_server_test.cpp b/tests/smoke/daemon_server_test.cpp index 1ecc5062..4bf5588d 100644 --- a/tests/smoke/daemon_server_test.cpp +++ b/tests/smoke/daemon_server_test.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include "unreadable_file.hpp" @@ -1193,6 +1194,52 @@ int main() { fs::remove_all(cov_root); } + // An exception inside the daemon -- here the project root's parent losing + // search permission, so the rescan's weakly_canonical(root) throws a + // filesystem_error -- must stop the daemon with exit 1 and release its + // endpoint. It must not escape a worker thread or unwind past a joinable + // one, either of which calls std::terminate (abort, exit 134). Skipped as + // root, which bypasses directory permissions, and on Windows, which has no + // POSIX search permission. +#ifndef _WIN32 + if (::geteuid() != 0) { + const auto fatal_parent = fs::temp_directory_path() / "cgraph_daemon_fatal_test"; + const auto fatal_root = fatal_parent / "proj"; + std::error_code restore_error; // a run that aborted mid-test left the parent at 000 + fs::permissions(fatal_parent, fs::perms::owner_all, fs::perm_options::replace, restore_error); + fs::remove_all(fatal_parent); + write_file(fatal_root / "src" / "fatal.ts", "export function fatalTarget() { return 1; }\n"); + const auto fatal_socket = cgraph::unix_socket_path(cgraph::daemon_identity_for(fatal_root)); + fs::remove(fatal_socket); + + cgraph::DaemonServerOptions fatal_options; + fatal_options.idle_timeout = std::chrono::seconds(60); + fatal_options.code_poll_interval = std::chrono::milliseconds(0); + fatal_options.drop_poll_interval = std::chrono::milliseconds(20); + + // Serve loop: an `update` whose rescan throws. + int loop_rc = -1; + std::thread loop_server([&] { loop_rc = cgraph::run_daemon_server(fatal_root, fatal_options); }); + expect(ok, wait_for_label(fatal_socket, "fatalTarget", "fatalTarget", true), + "fatal: initial build published the graph"); + expect(ok, ::chmod(fatal_parent.c_str(), 0) == 0, "fatal: removed search permission on the root's parent"); + (void)cgraph::request_over_unix_socket(fatal_socket, cgraph::make_request("update", {{"path", "."}})); + loop_server.join(); + expect(ok, loop_rc == 1, "fatal: a serve-loop exception exits run_daemon_server with 1"); + expect(ok, !fs::exists(fatal_socket), "fatal: serve-loop failure released the endpoint"); + + // Build thread: the initial build itself throws. + int build_rc = -1; + std::thread build_server([&] { build_rc = cgraph::run_daemon_server(fatal_root, fatal_options); }); + build_server.join(); + expect(ok, build_rc == 1, "fatal: an initial-build exception exits run_daemon_server with 1"); + expect(ok, !fs::exists(fatal_socket), "fatal: initial-build failure released the endpoint"); + + expect(ok, ::chmod(fatal_parent.c_str(), 0700) == 0, "fatal: restored the root's parent"); + fs::remove_all(fatal_parent); + } +#endif + // Stale-checkpoint detection through the real daemon: graph_remember anchors // each touched symbol to its own source span, and graph_recall grades each // touch valid / changed after a real edit is extracted, keeps grading it the diff --git a/tests/smoke/mcp_main_test.cpp b/tests/smoke/mcp_main_test.cpp new file mode 100644 index 00000000..52d0b0ac --- /dev/null +++ b/tests/smoke/mcp_main_test.cpp @@ -0,0 +1,135 @@ +// End-to-end coverage of the cgraph-mcp stdio loop (src/mcp/main.cpp): a +// parseable line of the wrong JSON-RPC shape is answered with Invalid Request +// (-32600; the request's id when readable, else null) and the loop keeps serving the next line. Before, nlohmann +// threw on the first typed read and the uncaught exception aborted the server. + +#include + +#include +#include +#include +#include +#include + +#ifdef _WIN32 + +// The stdin/stdout harness below is fork/pipe based. Exit code 77 is the +// test's SKIP_RETURN_CODE (tests/smoke/CMakeLists.txt), so ctest reports a skip. +int main() { + std::cout << "mcp_main_test: SKIP on Windows: the stdio harness needs fork and pipe\n"; + return 77; +} + +#else + +#include +#include + +namespace { + +void expect(bool& ok, bool condition, const std::string& what) { + if (!condition) { + std::cerr << "FAIL: " << what << '\n'; + ok = false; + } +} + +struct RunResult { + int exit_code = -1; + std::string stdout_text; +}; + +// Runs `program args...` with `input` on stdin; returns its exit code and stdout. +RunResult run_with_stdin(const std::string& program, std::vector args, const std::string& input) { + int to_child[2]; + int from_child[2]; + if (::pipe(to_child) != 0 || ::pipe(from_child) != 0) { + return {}; + } + const pid_t child = ::fork(); + if (child == 0) { + ::dup2(to_child[0], STDIN_FILENO); + ::dup2(from_child[1], STDOUT_FILENO); + ::close(to_child[0]); + ::close(to_child[1]); + ::close(from_child[0]); + ::close(from_child[1]); + std::vector argv; + std::string name = program; + argv.push_back(name.data()); + for (auto& arg : args) { + argv.push_back(arg.data()); + } + argv.push_back(nullptr); + ::execv(name.c_str(), argv.data()); + ::_exit(127); + } + ::close(to_child[0]); + ::close(from_child[1]); + // The input is far below the pipe buffer, so writing it all before reading cannot deadlock. + (void)::write(to_child[1], input.data(), input.size()); + ::close(to_child[1]); + RunResult result; + char buffer[4096]; + for (;;) { + const auto count = ::read(from_child[0], buffer, sizeof(buffer)); + if (count <= 0) { + break; + } + result.stdout_text.append(buffer, static_cast(count)); + } + ::close(from_child[0]); + int status = 0; + ::waitpid(child, &status, 0); + result.exit_code = WIFEXITED(status) ? WEXITSTATUS(status) : 128 + WTERMSIG(status); + return result; +} + +} // namespace + +int main(int argc, char** argv) { + if (argc != 2) { + std::cerr << "usage: cgraph_mcp_main_test \n"; + return 2; + } + bool ok = true; + const auto root = std::filesystem::temp_directory_path(); + + // Three malformed-but-parseable lines, then a valid request that needs no daemon. + const std::string input = + "5\n" + "[1]\n" + "{\"jsonrpc\":\"2.0\",\"id\":3,\"method\":7}\n" + "{\"jsonrpc\":\"2.0\",\"id\":9,\"method\":\"tools/list\"}\n"; + const auto run = run_with_stdin(argv[1], {"--root", root.string()}, input); + expect(ok, run.exit_code == 0, "cgraph-mcp exits 0 at end of input (got " + std::to_string(run.exit_code) + ")"); + + std::vector responses; + std::istringstream lines(run.stdout_text); + for (std::string line; std::getline(lines, line);) { + responses.push_back(nlohmann::json::parse(line, nullptr, false)); + } + expect(ok, responses.size() == 4, "one response per request line: " + run.stdout_text); + // `5` and `[1]` carry no readable id (null); the object's id 3 is echoed. + const std::vector expected_ids{nullptr, nullptr, 3}; + for (std::size_t index = 0; index < expected_ids.size() && index < responses.size(); ++index) { + const auto& response = responses[index]; + expect(ok, + response.is_object() && response.value("jsonrpc", std::string{}) == "2.0" && + response["id"] == expected_ids[index] && response["error"]["code"] == -32600, + "malformed line " + std::to_string(index + 1) + " answered with Invalid Request, id " + + expected_ids[index].dump() + ": " + response.dump()); + } + if (responses.size() == 4) { + const auto& listed = responses[3]; + expect(ok, listed.is_object() && listed["id"] == 9 && !listed["result"]["tools"].empty(), + "the valid request after the malformed lines is still answered: " + listed.dump()); + } + + if (ok) { + std::cout << "mcp_main_test: ok\n"; + } + return ok ? 0 : 1; +} + +#endif // _WIN32 diff --git a/tests/smoke/mcp_server_test.cpp b/tests/smoke/mcp_server_test.cpp index 1f57fe51..42f63570 100644 --- a/tests/smoke/mcp_server_test.cpp +++ b/tests/smoke/mcp_server_test.cpp @@ -8,6 +8,9 @@ #include #include #include +#include +#include +#include int main() { nlohmann::json forwarded; @@ -307,5 +310,40 @@ int main() { return 1; } + // A forwarder that throws (a filesystem error reaching the daemon) fails that + // one request with a JSON-RPC error naming the path; it must not escape and + // abort the stdio loop that serves every later request. + const cgraph::McpForwarder throwing_forwarder = [](const nlohmann::json&) -> nlohmann::json { + throw std::filesystem::filesystem_error("cannot create directories", std::filesystem::path("/ro/daemon-dir"), + std::make_error_code(std::errc::permission_denied)); + }; + const auto thrown = cgraph::handle_mcp_request( + nlohmann::json{{"jsonrpc", "2.0"}, + {"id", 15}, + {"method", "tools/call"}, + {"params", {{"name", "graph_query"}, {"arguments", {{"q", "alpha"}}}}}}, + throwing_forwarder); + if (thrown["id"] != 15 || thrown["error"]["code"] != -32603 || + thrown["error"]["message"].get().find("/ro/daemon-dir") == std::string::npos) { + return 1; + } + + // Parseable requests of the wrong shape are Invalid Request, not a nlohmann + // type_error escaping to the stdio loop. A readable id is echoed so the host + // can match the error to its call; otherwise the id is null. + const std::vector> malformed_cases{ + {nlohmann::json(5), nullptr}, + {nlohmann::json::array({1}), nullptr}, + {nlohmann::json{{"jsonrpc", "2.0"}, {"id", 16}, {"method", 7}}, 16}, + {nlohmann::json{{"jsonrpc", "2.0"}, {"id", "s17"}, {"method", "tools/call"}, {"params", 5}}, "s17"}, + {nlohmann::json{{"jsonrpc", "2.0"}, {"id", {{"bad", 1}}}, {"method", 7}}, nullptr}, + }; + for (const auto& [malformed, expected_id] : malformed_cases) { + const auto invalid = cgraph::handle_mcp_request(malformed, forwarder); + if (invalid["id"] != expected_id || invalid["error"]["code"] != -32600) { + return 1; + } + } + return 0; }