fix: address all code review issues (HIGH/MEDIUM/LOW)

- H1: fix integer overflow in assertImage (int→size_t for pixel counts)
- H2: fix localtime_s portability (add localtime_r for POSIX)
- H3: remove misleading std::once_flag (single-threaded server)
- M1: reject tools/list and tools/call before initialization (-32002)
- M2: throw on duplicate tool name registration
- M3: reject unknown parameters in validateArgs
- M4: remove dead null checks after controller() (already throws)
- M5: replace hand-crafted JSON in snapshot.cpp with nlohmann::json
- M6: nest AssertResult details under "details" key
- M7: isolate stb_image IMPLEMENTATION into dedicated TU
- L1: check std::cout.fail() to detect broken output pipe
- L2: fix #include order in main.cpp
- L3: tighten findNewestCapture filename matching
- L4: make test stub controller() throw like real implementation
- L5: add batch edge case tests (non-object elements, all-notifications)
- L6: add ShaderEditState::clear() method

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: JiaboLi-GitHub

CMakeLists.txt

@@ -44,6 +44,7 @@ if(RENDERDOC_DIR)
         src/core/snapshot.cpp
         src/core/usage.cpp
         src/core/assertions.cpp
+        src/core/stb_impl.cpp
         src/core/diff_session.cpp
         src/core/diff_alignment.cpp
         src/core/diff_structure.cpp

src/core/assertions.cpp

@@ -11,10 +11,7 @@
 #include <memory>
 #include <sstream>
 
-#define STB_IMAGE_IMPLEMENTATION
 #include "stb_image.h"
-
-#define STB_IMAGE_WRITE_IMPLEMENTATION
 #include "stb_image_write.h"
 
 #include <renderdoc_replay.h>
@@ -108,13 +105,13 @@ ImageCompareResult assertImage(const std::string& expectedPath,
                         "Image size mismatch: " + std::to_string(ew) + "x" + std::to_string(eh) +
                         " vs " + std::to_string(aw) + "x" + std::to_string(ah));
 
-    int totalPixels = ew * eh;
-    int diffPixels = 0;
+    size_t totalPixels = static_cast<size_t>(ew) * static_cast<size_t>(eh);
+    size_t diffPixels = 0;
     std::vector<unsigned char> diffImage;
     if (!diffOutputPath.empty()) diffImage.resize(totalPixels * 4);
 
-    for (int i = 0; i < totalPixels; ++i) {
-        int offset = i * 4;
+    for (size_t i = 0; i < totalPixels; ++i) {
+        size_t offset = i * 4;
         bool same = (expectedData[offset] == actualData[offset] &&
                      expectedData[offset+1] == actualData[offset+1] &&
                      expectedData[offset+2] == actualData[offset+2] &&

src/core/capture.cpp

@@ -35,7 +35,11 @@ std::string generateOutputPath(const std::string& exePath) {
     auto now = std::chrono::system_clock::now();
     auto timeT = std::chrono::system_clock::to_time_t(now);
     std::tm tm{};
+#ifdef _WIN32
     localtime_s(&tm, &timeT);
+#else
+    localtime_r(&timeT, &tm);
+#endif
 
     char buf[32];
     std::strftime(buf, sizeof(buf), "%Y%m%d_%H%M%S", &tm);
@@ -71,7 +75,11 @@ std::string findNewestCapture(const std::string& exeName,
         for (const auto& entry : fs::directory_iterator(dir)) {
             if (entry.path().extension() != ".rdc") continue;
             auto name = entry.path().filename().string();
-            if (name.find(exeName) == std::string::npos) continue;
+            // Match filenames that start with exeName or contain exeName preceded by
+            // a separator, to avoid overly broad substring matches.
+            auto pos = name.find(exeName);
+            if (pos == std::string::npos) continue;
+            if (pos != 0 && name[pos - 1] != '_' && name[pos - 1] != '-') continue;
             auto ftime = entry.last_write_time();
             if (found.empty() || ftime > newestTime) {
                 found = entry.path().string();

src/core/export.cpp

@@ -40,9 +40,6 @@ ExportResult exportRenderTarget(const Session& session,
                                 const std::string& outputDir)
 {
     auto* ctrl = session.controller();
-    if (!ctrl)
-        throw CoreError(CoreError::Code::NoCaptureOpen,
-                        "No capture is open. Call open_capture first.");
 
     // Walk the action tree to find the current event and retrieve its output RT.
     const uint32_t eventId = session.currentEventId();
@@ -123,9 +120,6 @@ ExportResult exportTexture(const Session& session,
                            uint32_t layer)
 {
     auto* ctrl = session.controller();
-    if (!ctrl)
-        throw CoreError(CoreError::Code::NoCaptureOpen,
-                        "No capture is open. Call open_capture first.");
 
     ::ResourceId rdcId = fromResourceId(id);
 
@@ -164,9 +158,6 @@ ExportResult exportBuffer(const Session& session,
                           uint64_t size)
 {
     auto* ctrl = session.controller();
-    if (!ctrl)
-        throw CoreError(CoreError::Code::NoCaptureOpen,
-                        "No capture is open. Call open_capture first.");
 
     ::ResourceId rdcId = fromResourceId(id);
     rdcarray<byte> data = ctrl->GetBufferData(rdcId, offset, size);

src/core/session.cpp

@@ -17,13 +17,13 @@ Session::~Session() {
 }
 
 void Session::ensureReplayInitialized() {
-    std::call_once(m_replayInitFlag, [this]() {
-        GlobalEnvironment env;
-        memset(&env, 0, sizeof(env));
-        rdcarray<rdcstr> args;
-        RENDERDOC_InitialiseReplay(env, args);
-        m_replayInitialized = true;
-    });
+    if (m_replayInitialized)
+        return;
+    GlobalEnvironment env;
+    memset(&env, 0, sizeof(env));
+    rdcarray<rdcstr> args;
+    RENDERDOC_InitialiseReplay(env, args);
+    m_replayInitialized = true;
 }
 
 void Session::closeCurrent() {
@@ -40,7 +40,7 @@ void Session::closeCurrent() {
     m_capturePath.clear();
     m_totalEvents = 0;
     m_api = GraphicsApi::Unknown;
-    m_shaderEditState = ShaderEditState{};
+    m_shaderEditState.clear();
 }
 
 void Session::close() {

src/core/session.h

@@ -2,7 +2,6 @@
 
 #include "core/types.h"
 #include "core/shader_edit.h"
-#include <mutex>
 #include <string>
 
 // Forward declarations from RenderDoc
@@ -46,7 +45,6 @@ class Session {
     IReplayController* m_controller = nullptr;
     uint32_t m_currentEventId = 0;
     std::string m_capturePath;
-    std::once_flag m_replayInitFlag;
     bool m_replayInitialized = false;
     uint32_t m_totalEvents = 0;
     GraphicsApi m_api = GraphicsApi::Unknown;

src/core/shader_edit.h

@@ -13,6 +13,11 @@ class Session;
 struct ShaderEditState {
     std::map<uint64_t, uint64_t> builtShaders;
     std::map<uint64_t, uint64_t> shaderReplacements;
+
+    void clear() {
+        builtShaders.clear();
+        shaderReplacements.clear();
+    }
 };
 
 std::vector<std::string> getShaderEncodings(const Session& session);

src/core/snapshot.cpp

@@ -12,6 +12,7 @@
 #include <chrono>
 #include <filesystem>
 #include <fstream>
+#include <nlohmann/json.hpp>
 #include <sstream>
 
 namespace fs = std::filesystem;
@@ -20,31 +21,6 @@ namespace renderdoc::core {
 
 namespace {
 
-// Escape a string for JSON output (handles quotes, backslashes, control chars).
-std::string jsonEscape(const std::string& s) {
-    std::string out;
-    out.reserve(s.size() + 8);
-    for (char c : s) {
-        switch (c) {
-            case '"':  out += "\\\""; break;
-            case '\\': out += "\\\\"; break;
-            case '\n': out += "\\n";  break;
-            case '\r': out += "\\r";  break;
-            case '\t': out += "\\t";  break;
-            default:
-                if (static_cast<unsigned char>(c) < 0x20) {
-                    char buf[8];
-                    snprintf(buf, sizeof(buf), "\\u%04x", static_cast<unsigned>(c));
-                    out += buf;
-                } else {
-                    out += c;
-                }
-                break;
-        }
-    }
-    return out;
-}
-
 // Map ShaderStage to a short string for filenames.
 std::string stageToFileSuffix(ShaderStage stage) {
     switch (stage) {
@@ -64,9 +40,6 @@ SnapshotResult exportSnapshot(Session& session, uint32_t eventId,
                               const std::string& outputDir,
                               std::function<std::string(const PipelineState&)> pipelineSerializer) {
     auto* ctrl = session.controller();
-    if (!ctrl)
-        throw CoreError(CoreError::Code::NoCaptureOpen,
-                        "No capture is open. Call open_capture first.");
 
     ctrl->SetFrameEvent(eventId, true);
 
@@ -180,42 +153,28 @@ SnapshotResult exportSnapshot(Session& session, uint32_t eventId,
         }
     }
 
-    // 5. Write manifest.json (hand-crafted, no nlohmann::json dependency).
+    // 5. Write manifest.json.
     {
         auto now = std::chrono::system_clock::now();
         auto epoch = std::chrono::duration_cast<std::chrono::seconds>(
             now.time_since_epoch()).count();
 
-        std::ostringstream manifest;
-        manifest << "{\n";
-        manifest << "  \"eventId\": " << eventId << ",\n";
-        manifest << "  \"timestamp\": " << epoch << ",\n";
-        manifest << "  \"files\": [";
-
-        for (size_t i = 0; i < result.files.size(); i++) {
-            if (i > 0) manifest << ",";
-            // Use just the filename, not the full path.
-            std::string fname = fs::path(result.files[i]).filename().string();
-            manifest << "\n    \"" << jsonEscape(fname) << "\"";
-        }
-
-        manifest << "\n  ]";
+        nlohmann::json manifest;
+        manifest["eventId"] = eventId;
+        manifest["timestamp"] = epoch;
 
-        if (!result.errors.empty()) {
-            manifest << ",\n  \"errors\": [";
-            for (size_t i = 0; i < result.errors.size(); i++) {
-                if (i > 0) manifest << ",";
-                manifest << "\n    \"" << jsonEscape(result.errors[i]) << "\"";
-            }
-            manifest << "\n  ]";
-        }
+        auto filesArr = nlohmann::json::array();
+        for (const auto& f : result.files)
+            filesArr.push_back(fs::path(f).filename().string());
+        manifest["files"] = filesArr;
 
-        manifest << "\n}\n";
+        if (!result.errors.empty())
+            manifest["errors"] = result.errors;
 
         std::string manifestPath = (fs::path(outputDir) / "manifest.json").string();
         std::ofstream ofs(manifestPath);
         if (ofs) {
-            ofs << manifest.str();
+            ofs << manifest.dump(2);
             ofs.close();
             result.manifestPath = manifestPath;
             result.files.push_back(manifestPath);

src/core/stb_impl.cpp

@@ -0,0 +1,8 @@
+// Dedicated translation unit for stb_image implementations.
+// These macros must appear in exactly one .cpp file.
+
+#define STB_IMAGE_IMPLEMENTATION
+#include "stb_image.h"
+
+#define STB_IMAGE_WRITE_IMPLEMENTATION
+#include "stb_image_write.h"

src/core/types.h

@@ -403,8 +403,8 @@ struct PixelAssertResult {
 
 struct ImageCompareResult {
     bool pass = false;
-    int diffPixels = 0;
-    int totalPixels = 0;
+    size_t diffPixels = 0;
+    size_t totalPixels = 0;
     double diffRatio = 0.0;
     std::string diffOutputPath;
     std::string message;