From 4e04d245a2c1d70ff055eaa89207594548040162 Mon Sep 17 00:00:00 2001 From: Taylor Date: Fri, 2 Oct 2026 20:38:01 -0700 Subject: [PATCH 1/4] fix(cli): exit 1 instead of aborting on uncaught filesystem errors `cgraph seam fuse --out ` (and the one-shot build) aborted with exit 134: create_directories threw an uncaught std::filesystem::filesystem_error, and main only caught FileWriteError. - atomic_write: add create_output_directories, which throws FileWriteError ("failed to write : cannot create directory ()") so output directory failures read like every other output write. Used by the CLI output paths, write_exports, and the semantic stat index / cache writers (whose daemon callers already log FileWriteError as non-fatal). - cli/main: one std::exception handler prints `cgraph: ` and exits 1. - graphd, cgraph-client, cgraph-mcp mains: same last-resort handler, so e.g. `graphd --idle-timeout abc` or a deleted cwd exits 1, not 134. - mcp_server: a throwing forwarder fails only that request (-32603) instead of escaping and killing the stdio loop. Graph output unchanged; kIndexVersionKey not bumped. Co-Authored-By: Claude Opus 5.5 --- src/cli/main.cpp | 14 ++++++---- src/client/main.cpp | 18 ++++++++++--- src/daemon/main.cpp | 19 ++++++++++--- src/engine/atomic_write.cpp | 11 ++++++++ src/engine/include/cgraph/atomic_write.hpp | 7 +++++ src/engine/pipeline.cpp | 4 +-- src/engine/semantic_cache.cpp | 2 +- src/engine/semantic_chunk_plan.cpp | 2 +- src/mcp/main.cpp | 19 ++++++++++++- src/mcp/mcp_server.cpp | 9 ++++++- tests/smoke/cli_main_test.cpp | 31 ++++++++++++++++++++++ tests/smoke/mcp_server_test.cpp | 20 ++++++++++++++ 12 files changed, 139 insertions(+), 17 deletions(-) diff --git a/src/cli/main.cpp b/src/cli/main.cpp index 25f210c1..5c1b783d 100644 --- a/src/cli/main.cpp +++ b/src/cli/main.cpp @@ -162,7 +162,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'; @@ -445,7 +445,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() @@ -647,7 +647,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, " @@ -766,7 +766,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. @@ -1250,9 +1250,13 @@ 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. + // One handler reports every failure the same way: cgraph::FileWriteError + // (names the file and the OS reason), std::filesystem::filesystem_error + // (what() names the path(s)), and any other std::exception. An escaped + // exception would instead abort with a core dump (exit 134). try { return run(argc, argv); - } catch (const cgraph::FileWriteError& error) { + } catch (const std::exception& error) { std::cerr << "cgraph: " << error.what() << '\n'; return 1; } diff --git a/src/client/main.cpp b/src/client/main.cpp index 0eb5975f..417c5f79 100644 --- a/src/client/main.cpp +++ b/src/client/main.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -15,9 +16,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(), @@ -88,3 +87,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: " << error.what() << '\n'; + return 1; + } +} diff --git a/src/daemon/main.cpp b/src/daemon/main.cpp index f0e4064a..c1048c3e 100644 --- a/src/daemon/main.cpp +++ b/src/daemon/main.cpp @@ -6,6 +6,7 @@ #include "cgraph/seam.hpp" #include +#include #include #include @@ -16,9 +17,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; @@ -100,3 +99,17 @@ int main(int argc, char** argv) { } return cgraph::run_daemon_server(root, options); } + +} // namespace + +int main(int argc, char** argv) { + // A failure the server does not handle itself (e.g. a filesystem_error + // creating the socket directory, or an unreadable --idle-timeout) is logged + // and exits non-zero; an escaped exception would abort with a core dump. + try { + return run(argc, argv); + } catch (const std::exception& error) { + std::cerr << "graphd: " << error.what() << '\n'; + return 1; + } +} diff --git a/src/engine/atomic_write.cpp b/src/engine/atomic_write.cpp index 45ba110a..ac837cd8 100644 --- a/src/engine/atomic_write.cpp +++ b/src/engine/atomic_write.cpp @@ -168,4 +168,15 @@ 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) { + throw FileWriteError(failure(directory, "cannot create directory", error.value())); + } +} + } // namespace cgraph 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/pipeline.cpp b/src/engine/pipeline.cpp index 7a70f16a..8e8607f9 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); } @@ -121,7 +121,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 07f6d96c..f554b87d 100644 --- a/src/engine/semantic_cache.cpp +++ b/src/engine/semantic_cache.cpp @@ -393,7 +393,7 @@ ReconciliationResult reconcile_semantic_cache( } void 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 f453e00b..e0aaf68a 100644 --- a/src/mcp/main.cpp +++ b/src/mcp/main.cpp @@ -2,11 +2,14 @@ #include "cgraph/mcp_server.hpp" #include +#include #include #include #include -int main(int argc, char** argv) { +namespace { + +int run(int argc, char** argv) { cgraph::ClientRequest base; // Claude Code (and other hosts) set CLAUDE_PROJECT_DIR to the project root and // do not guarantee the server's working directory, so prefer it over cwd. @@ -56,3 +59,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: " << error.what() << '\n'; + return 1; + } +} diff --git a/src/mcp/mcp_server.cpp b/src/mcp/mcp_server.cpp index 4de4a508..ecaa5d35 100644 --- a/src/mcp/mcp_server.cpp +++ b/src/mcp/mcp_server.cpp @@ -342,7 +342,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)) { diff --git a/tests/smoke/cli_main_test.cpp b/tests/smoke/cli_main_test.cpp index 0b239406..764efd34 100644 --- a/tests/smoke/cli_main_test.cpp +++ b/tests/smoke/cli_main_test.cpp @@ -173,6 +173,37 @@ 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"; + } + fs::remove_all(root); if (ok) { std::cout << "cli_main_test: ok\n"; diff --git a/tests/smoke/mcp_server_test.cpp b/tests/smoke/mcp_server_test.cpp index ea6d276a..e1c1fef5 100644 --- a/tests/smoke/mcp_server_test.cpp +++ b/tests/smoke/mcp_server_test.cpp @@ -3,7 +3,9 @@ #include #include +#include #include +#include int main() { nlohmann::json forwarded; @@ -255,5 +257,23 @@ 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; + } + return 0; } From 12a71836ddfb98deb1a56fc1f60e9b13d22f709d Mon Sep 17 00:00:00 2001 From: Taylor Date: Fri, 2 Oct 2026 20:54:58 -0700 Subject: [PATCH 2/4] fix: review round 1 -- daemon/MCP exception safety, numeric flag usage errors - graphd: exceptions in the serve loop, the initial-build thread, or the enrichment worker are logged with their type and stop the daemon with exit 1. A scope guard joins both workers and releases the endpoint on any exit, so nothing unwinds past a joinable std::thread (std::terminate). The exit-time incremental persist is skipped after a failure. - cgraph-mcp: handle_mcp_request answers a parseable line of the wrong shape (`5`, `[1]`, a non-string method) with -32600, id null, and the stdio loop keeps serving. - New cgraph/cli_support: integer_flag/number_flag/parse_integer replace std::stoi/stoll/stod/stol at every CLI and graphd flag site, so a bad value exits 2 with ": --depth expects an integer, got 'abc'"; describe_exception puts the demangled type in last-resort handlers. - Tests: daemon_server_test (fatal serve-loop and build-thread failures), mcp_main_test (stdio loop), mcp_server_test, cli_support_test, cli_main_test (--depth abc), atomic_write_test (create_output_directories). Co-Authored-By: Claude Opus 5.5 --- src/cli/main.cpp | 51 +-- src/client/main.cpp | 3 +- src/daemon/main.cpp | 16 +- src/engine/CMakeLists.txt | 1 + src/engine/cli_support.cpp | 34 ++ src/engine/daemon_server.cpp | 376 ++++++++++++---------- src/engine/include/cgraph/cli_support.hpp | 52 +++ src/mcp/main.cpp | 3 +- src/mcp/mcp_server.cpp | 21 +- tests/smoke/CMakeLists.txt | 26 ++ tests/smoke/atomic_write_test.cpp | 25 ++ tests/smoke/cli_main_test.cpp | 8 + tests/smoke/cli_support_test.cpp | 78 +++++ tests/smoke/daemon_server_test.cpp | 44 +++ tests/smoke/mcp_main_test.cpp | 120 +++++++ tests/smoke/mcp_server_test.cpp | 11 + 16 files changed, 671 insertions(+), 198 deletions(-) create mode 100644 src/engine/cli_support.cpp create mode 100644 src/engine/include/cgraph/cli_support.hpp create mode 100644 tests/smoke/cli_support_test.cpp create mode 100644 tests/smoke/mcp_main_test.cpp diff --git a/src/cli/main.cpp b/src/cli/main.cpp index 5c1b783d..18adb934 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" @@ -142,14 +143,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; @@ -210,13 +211,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); } @@ -330,17 +330,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) { @@ -910,7 +910,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; @@ -1111,9 +1111,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) { @@ -1250,14 +1250,19 @@ 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. - // One handler reports every failure the same way: cgraph::FileWriteError - // (names the file and the OS reason), std::filesystem::filesystem_error - // (what() names the path(s)), and any other std::exception. An escaped - // exception would instead abort with a core dump (exit 134). + // 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 std::exception& error) { + } 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 417c5f79..41af3fb3 100644 --- a/src/client/main.cpp +++ b/src/client/main.cpp @@ -1,3 +1,4 @@ +#include "cgraph/cli_support.hpp" #include "cgraph/client_runtime.hpp" #include "cgraph/engine.hpp" @@ -96,7 +97,7 @@ int main(int argc, char** argv) { try { return run(argc, argv); } catch (const std::exception& error) { - std::cerr << "cgraph-client: " << error.what() << '\n'; + std::cerr << "cgraph-client: " << cgraph::describe_exception(error) << '\n'; return 1; } } diff --git a/src/daemon/main.cpp b/src/daemon/main.cpp index c1048c3e..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" @@ -41,7 +42,7 @@ int run(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") { @@ -103,13 +104,18 @@ int run(int argc, char** argv) { } // namespace int main(int argc, char** argv) { - // A failure the server does not handle itself (e.g. a filesystem_error - // creating the socket directory, or an unreadable --idle-timeout) is logged - // and exits non-zero; an escaped exception would abort with a core dump. + // 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 std::exception& error) { + } 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 15429ec3..7ccaee1f 100644 --- a/src/engine/CMakeLists.txt +++ b/src/engine/CMakeLists.txt @@ -2,6 +2,7 @@ add_library(cgraph_engine STATIC analysis.cpp atomic_write.cpp change_context.cpp + cli_support.cpp daemon_endpoint.cpp daemon_server.cpp configured_extractors.cpp diff --git a/src/engine/cli_support.cpp b/src/engine/cli_support.cpp new file mode 100644 index 00000000..f118593a --- /dev/null +++ b/src/engine/cli_support.cpp @@ -0,0 +1,34 @@ +#include "cgraph/cli_support.hpp" + +#include +#include +#include +#include +#include + +#include + +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) { + 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(); +} + +} // namespace cgraph diff --git a/src/engine/daemon_server.cpp b/src/engine/daemon_server.cpp index c1cb5560..ab77330c 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" @@ -165,6 +166,19 @@ std::optional request_over_unix_socket(const std::filesystem::pa namespace { +// 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_; +}; + // Sentinel returned by open_listen_socket when a healthy daemon already serves // this root: the caller should exit cleanly (0) rather than treat it as an error. constexpr int kEndpointAlreadyServed = -2; @@ -804,23 +818,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 }); } @@ -836,178 +864,204 @@ 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(); + ::close(listen_fd); + (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) { - fd_set read_set; - FD_ZERO(&read_set); - FD_SET(listen_fd, &read_set); - timeval timeout{}; - timeout.tv_usec = static_cast( - std::chrono::duration_cast(options.drop_poll_interval).count() % 1'000'000); - timeout.tv_sec = static_cast( - std::chrono::duration_cast(options.drop_poll_interval).count()); - - const int ready = ::select(listen_fd + 1, &read_set, nullptr, nullptr, &timeout); - if (ready < 0) { - if (errno == EINTR) { - continue; + try { + while (!state.shutdown_requested && !fatal_error.load()) { + fd_set read_set; + FD_ZERO(&read_set); + FD_SET(listen_fd, &read_set); + timeval timeout{}; + timeout.tv_usec = static_cast( + std::chrono::duration_cast(options.drop_poll_interval).count() % 1'000'000); + timeout.tv_sec = static_cast( + std::chrono::duration_cast(options.drop_poll_interval).count()); + + const int ready = ::select(listen_fd + 1, &read_set, nullptr, nullptr, &timeout); + if (ready < 0) { + if (errno == EINTR) { + continue; + } + std::cerr << "graphd: select() failed: " << std::strerror(errno) << '\n'; + break; } - std::cerr << "graphd: select() failed: " << std::strerror(errno) << '\n'; - 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(); - } - // 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. - 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. + 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 == 0) { + // 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 == 0) { - // 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"; + const int conn = ::accept(listen_fd, nullptr, nullptr); + if (conn < 0) { + if (errno == EINTR) { + continue; + } break; } - continue; // no connection this tick; keep polling drops - } - - const int conn = ::accept(listen_fd, nullptr, nullptr); - if (conn < 0) { - if (errno == EINTR) { - continue; + last_activity = FileWatcherClock::now(); + apply_connection_timeout(conn); // a stalled peer must not wedge the serve loop + // The thin client uses one connection per request: read it, answer it, close. + const ScopeExit close_conn([conn] { ::close(conn); }); + if (const auto request = read_frame(conn)) { + const auto response = handle_daemon_request(state, *request); + (void)write_frame(conn, response); } - break; } - last_activity = FileWatcherClock::now(); - apply_connection_timeout(conn); // a stalled peer must not wedge the serve loop - // The thin client uses one connection per request: read it, answer it, close. - if (const auto request = read_frame(conn)) { - const auto response = handle_daemon_request(state, *request); - (void)write_frame(conn, response); - } - ::close(conn); + } catch (const std::exception& error) { + record_fatal("serve loop", error); } if (build_thread.joinable()) { @@ -1015,7 +1069,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"; @@ -1034,15 +1090,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 - ::close(listen_fd); - (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/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/mcp/main.cpp b/src/mcp/main.cpp index e0aaf68a..089071a4 100644 --- a/src/mcp/main.cpp +++ b/src/mcp/main.cpp @@ -1,3 +1,4 @@ +#include "cgraph/cli_support.hpp" #include "cgraph/client_runtime.hpp" #include "cgraph/mcp_server.hpp" @@ -69,7 +70,7 @@ int main(int argc, char** argv) { try { return run(argc, argv); } catch (const std::exception& error) { - std::cerr << "cgraph-mcp: " << error.what() << '\n'; + 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 ecaa5d35..2202e9bd 100644 --- a/src/mcp/mcp_server.cpp +++ b/src/mcp/mcp_server.cpp @@ -295,10 +295,8 @@ namespace { return {}; } -} // namespace - -nlohmann::json handle_mcp_request(const nlohmann::json& request, const McpForwarder& forwarder, - const McpChangeContext& change_context_runner) { +[[nodiscard]] nlohmann::json respond(const nlohmann::json& request, const McpForwarder& forwarder, + const McpChangeContext& change_context_runner) { 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()) { @@ -361,4 +359,19 @@ nlohmann::json handle_mcp_request(const nlohmann::json& request, const McpForwar return response(id, text_content(daemon_response.value("result", nlohmann::json::object()))); } +} // namespace + +nlohmann::json handle_mcp_request(const nlohmann::json& request, const McpForwarder& forwarder, + const McpChangeContext& change_context_runner) { + // 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 (JSON-RPC 2.0: id is null when it + // could not be determined); the stdio loop keeps serving. + try { + return respond(request, forwarder, change_context_runner); + } catch (const nlohmann::json::exception& error) { + return error_response(nullptr, -32600, std::string{"invalid JSON-RPC request: "} + error.what()); + } +} + } // namespace cgraph diff --git a/tests/smoke/CMakeLists.txt b/tests/smoke/CMakeLists.txt index af7e6762..a1f5a16c 100644 --- a/tests/smoke/CMakeLists.txt +++ b/tests/smoke/CMakeLists.txt @@ -829,6 +829,32 @@ add_test(NAME cgraph_cli_main_test $ "${CMAKE_CURRENT_SOURCE_DIR}/../fixtures/one_shot") +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 + $) + +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 aa99fe55..b7676e67 100644 --- a/tests/smoke/atomic_write_test.cpp +++ b/tests/smoke/atomic_write_test.cpp @@ -203,6 +203,31 @@ int main() { } 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"); + 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); + } + 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 764efd34..6d565c24 100644 --- a/tests/smoke/cli_main_test.cpp +++ b/tests/smoke/cli_main_test.cpp @@ -204,6 +204,14 @@ int main(int argc, char** argv) { 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..f0e1357d --- /dev/null +++ b/tests/smoke/cli_support_test.cpp @@ -0,0 +1,78 @@ +#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); + expect(ok, text.starts_with("std::filesystem::") && 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 dd7bf676..9bcc3ad1 100644 --- a/tests/smoke/daemon_server_test.cpp +++ b/tests/smoke/daemon_server_test.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -1155,6 +1156,49 @@ 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. + 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); + } + fs::remove_all(root); return ok ? 0 : 1; } diff --git a/tests/smoke/mcp_main_test.cpp b/tests/smoke/mcp_main_test.cpp new file mode 100644 index 00000000..dd53bc4a --- /dev/null +++ b/tests/smoke/mcp_main_test.cpp @@ -0,0 +1,120 @@ +// 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, id 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 + +#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); + for (std::size_t index = 0; index < 3 && index < responses.size(); ++index) { + const auto& response = responses[index]; + expect(ok, + response.is_object() && response.value("jsonrpc", std::string{}) == "2.0" && response["id"].is_null() && + response["error"]["code"] == -32600, + "malformed line " + std::to_string(index + 1) + " answered with Invalid Request, id null: " + + 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; +} diff --git a/tests/smoke/mcp_server_test.cpp b/tests/smoke/mcp_server_test.cpp index e1c1fef5..4a57232a 100644 --- a/tests/smoke/mcp_server_test.cpp +++ b/tests/smoke/mcp_server_test.cpp @@ -275,5 +275,16 @@ int main() { return 1; } + // Parseable requests of the wrong shape are Invalid Request (id null), not a + // nlohmann type_error escaping to the stdio loop. + for (const auto& malformed : {nlohmann::json(5), nlohmann::json::array({1}), + nlohmann::json{{"jsonrpc", "2.0"}, {"id", 16}, {"method", 7}}, + nlohmann::json{{"jsonrpc", "2.0"}, {"id", 17}, {"method", "tools/call"}, {"params", 5}}}) { + const auto invalid = cgraph::handle_mcp_request(malformed, forwarder); + if (!invalid["id"].is_null() || invalid["error"]["code"] != -32600) { + return 1; + } + } + return 0; } From 36f57868e00f0de995625908d6df91464ba4f4c6 Mon Sep 17 00:00:00 2001 From: Taylor Date: Fri, 2 Oct 2026 21:17:47 -0700 Subject: [PATCH 3/4] fix: review round 2 -- portable demangle test, echo readable MCP ids - cli_support_test: assert only "filesystem_error: " and the path; libc++ demangles the type as std::__1::__fs::filesystem::filesystem_error. - mcp_server: an Invalid Request echoes the request's id when it is an object member that is a string, number or null; otherwise null. - cli_support: guard for MSVC, whose type_info::name() is already readable. Co-Authored-By: Claude Opus 5.5 --- src/engine/cli_support.cpp | 7 +++++++ src/mcp/mcp_server.cpp | 15 ++++++++++++--- tests/smoke/cli_support_test.cpp | 4 ++-- tests/smoke/mcp_main_test.cpp | 14 ++++++++------ tests/smoke/mcp_server_test.cpp | 20 ++++++++++++++------ 5 files changed, 43 insertions(+), 17 deletions(-) diff --git a/src/engine/cli_support.cpp b/src/engine/cli_support.cpp index f118593a..1f0ce986 100644 --- a/src/engine/cli_support.cpp +++ b/src/engine/cli_support.cpp @@ -6,7 +6,9 @@ #include #include +#if !defined(_MSC_VER) #include +#endif namespace cgraph { @@ -24,11 +26,16 @@ double number_flag(std::string_view flag, std::string_view value) { } std::string describe_exception(const std::exception& error) { +#if defined(_MSC_VER) + // MSVC's type_info::name() is already human-readable ("class std::out_of_range"). + return std::string(typeid(error).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/mcp/mcp_server.cpp b/src/mcp/mcp_server.cpp index 2202e9bd..5099c31d 100644 --- a/src/mcp/mcp_server.cpp +++ b/src/mcp/mcp_server.cpp @@ -365,12 +365,21 @@ nlohmann::json handle_mcp_request(const nlohmann::json& request, const McpForwar const McpChangeContext& change_context_runner) { // 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 (JSON-RPC 2.0: id is null when it - // could not be determined); the stdio loop keeps serving. + // 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); } catch (const nlohmann::json::exception& error) { - return error_response(nullptr, -32600, std::string{"invalid JSON-RPC request: "} + error.what()); + 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()); } } diff --git a/tests/smoke/cli_support_test.cpp b/tests/smoke/cli_support_test.cpp index f0e1357d..a4c6012a 100644 --- a/tests/smoke/cli_support_test.cpp +++ b/tests/smoke/cli_support_test.cpp @@ -66,8 +66,8 @@ int main() { std::make_error_code(std::errc::permission_denied)); } catch (const std::exception& error) { const auto text = cgraph::describe_exception(error); - expect(ok, text.starts_with("std::filesystem::") && text.find("filesystem_error: ") != std::string::npos && - text.find("[/x]") != std::string::npos, + // The namespace is library-specific (libc++: std::__1::__fs::filesystem::). + 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); } diff --git a/tests/smoke/mcp_main_test.cpp b/tests/smoke/mcp_main_test.cpp index dd53bc4a..64068b17 100644 --- a/tests/smoke/mcp_main_test.cpp +++ b/tests/smoke/mcp_main_test.cpp @@ -1,6 +1,6 @@ // 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, id null) and the loop keeps serving the next line. Before, nlohmann +// (-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 @@ -99,13 +99,15 @@ int main(int argc, char** argv) { responses.push_back(nlohmann::json::parse(line, nullptr, false)); } expect(ok, responses.size() == 4, "one response per request line: " + run.stdout_text); - for (std::size_t index = 0; index < 3 && index < responses.size(); ++index) { + // `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"].is_null() && - response["error"]["code"] == -32600, - "malformed line " + std::to_string(index + 1) + " answered with Invalid Request, id null: " + - response.dump()); + 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]; diff --git a/tests/smoke/mcp_server_test.cpp b/tests/smoke/mcp_server_test.cpp index 4a57232a..ee25e63c 100644 --- a/tests/smoke/mcp_server_test.cpp +++ b/tests/smoke/mcp_server_test.cpp @@ -5,6 +5,8 @@ #include #include #include +#include +#include #include int main() { @@ -275,13 +277,19 @@ int main() { return 1; } - // Parseable requests of the wrong shape are Invalid Request (id null), not a - // nlohmann type_error escaping to the stdio loop. - for (const auto& malformed : {nlohmann::json(5), nlohmann::json::array({1}), - nlohmann::json{{"jsonrpc", "2.0"}, {"id", 16}, {"method", 7}}, - nlohmann::json{{"jsonrpc", "2.0"}, {"id", 17}, {"method", "tools/call"}, {"params", 5}}}) { + // 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"].is_null() || invalid["error"]["code"] != -32600) { + if (invalid["id"] != expected_id || invalid["error"]["code"] != -32600) { return 1; } } From 918dc77ff41bedbba1750390c7b77bf8488b8eef Mon Sep 17 00:00:00 2001 From: Taylor Date: Mon, 19 Oct 2026 00:04:22 -0700 Subject: [PATCH 4/4] fix(atomic-write): name the OS reason from the error_code on Windows create_output_directories passed error.value() to failure(), which reads it as an errno. On Windows std::filesystem reports Win32 codes in system_category, so ERROR_ACCESS_DENIED (5) printed as EIO. Use error.message() instead; the text is unchanged on POSIX. Co-Authored-By: Claude Opus 5.5 --- src/engine/atomic_write.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/engine/atomic_write.cpp b/src/engine/atomic_write.cpp index e58a8a94..7285458b 100644 --- a/src/engine/atomic_write.cpp +++ b/src/engine/atomic_write.cpp @@ -260,7 +260,9 @@ void create_output_directories(const std::filesystem::path& directory) { std::error_code error; std::filesystem::create_directories(directory, error); if (error) { - throw FileWriteError(failure(directory, "cannot create directory", error.value())); + // 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)); } }