feat: diagnostics + drift-manifest modules, bulk hook API, and dev tooling#101
Conversation
…tizer workflow - add root .clang-format, .clang-tidy, .editorconfig matching the existing style - add a mingw-debug-coverage preset and a non-blocking quality.yml (ASan/UBSan probe + advisory clang-format)
- new diagnostics module exposing consumer-queryable counters for intentional loader-lock leak/detach events - instrument the loader-lock teardown paths in logger, async_logger, config_watcher, input, memory, worker, and bootstrap
- serialize/parse the self-heal drift report (DriftEntry) to a durable, diffable manifest - owning parsed records, file I/O, and fail-closed parsing
… usings - add enable_hooks/disable_hooks/enable_all_hooks/disable_all_hooks (batch toggle under one lock) - record the hook_manager loader-lock leak path via the diagnostics counters - narrow the redundant sub-namespace using directives in config, scanner, and hook_manager - update README and AGENTS for the new modules and APIs
|
Warning Review limit reached
More reviews will be available in 42 minutes and 57 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces process-wide diagnostics instrumentation, a durable drift manifest serialization format for offline RTTI analysis, and batch hook management APIs. It includes code quality infrastructure (linting, formatting, CI workflow), comprehensive test coverage, and integration of diagnostics across shutdown paths to track intentional thread detaches under loader-lock conditions. ChangesDiagnostics and drift manifest features with batch hook operations
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/quality.yml (1)
1-69:⚠️ Potential issue | 🟠 MajorHarden workflow security posture for
quality.yml(least-privilege, disable checkout credentials, pin checkout action)
- Add
permissions: contents: readto avoid default broadGITHUB_TOKENpermissions.- Set
with: persist-credentials: falseon bothactions/checkout@v4steps to prevent credential leakage via artifacts.- Pin
actions/checkout@v4to a commit SHA to prevent mutable-tag supply-chain risk (34e114876b0b11c390a56381ad16ebd13914f8d5).🔒 Proposed hardening
name: Quality Probes on: pull_request: branches: [main] workflow_dispatch: +# Restrict to minimal read-only permissions +permissions: + contents: read + # Both jobs are advisory and must never block a PR: they surface formatting # drift and prove the sanitizer preset still links and runs, but clang-format # version and MinGW ASan availability both vary by runner. The blocking gate # (build, tests, coverage) lives in pr-check.yml and is untouched here. jobs: format-check: name: clang-format (advisory) runs-on: windows-latest continue-on-error: true steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 + with: + persist-credentials: false - name: Install clang-format run: pip install "clang-format==18.*" shell: bash - name: Check formatting (dry run, project sources only) run: | # git ls-files lists only this repo's tracked files, so submodule # sources under external/ are never formatted. git ls-files '*.cpp' '*.hpp' | xargs clang-format --dry-run --Werror shell: bash sanitizers: name: ASan + UBSan probe (advisory) runs-on: windows-latest continue-on-error: true env: MINGW_BIN: C:\mingw64\bin steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 with: + persist-credentials: false submodules: "recursive"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/quality.yml around lines 1 - 69, The workflow grants excessive default token rights and uses mutable checkout tags; harden it by adding a top-level permissions: contents: read entry to restrict GITHUB_TOKEN to repository read-only, replace both uses of actions/checkout@v4 with the pinned commit SHA (34e114876b0b11c390a56381ad16ebd13914f8d5) and set with: persist-credentials: false on the Checkout steps (both the Checkout code step in the format-check job and the Checkout code step in the sanitizers job that currently includes submodules) to prevent credential persistence/leakage.
🧹 Nitpick comments (1)
include/DetourModKit/hook_manager.hpp (1)
830-854: ⚡ Quick winAdd explicit
@paramtags for consistency.Both
enable_hooksanddisable_hooksdescribe thehook_idsparameter within the@detailssection, but the file's established pattern (seecreate_inline_hookat line 500 and other methods) uses explicit@paramtags for every parameter. Add@param hook_idsto each doc block for consistency.📝 Suggested addition
For
enable_hooks(after line 839):* SafetyHook backend installs via a vectored exception handler and does not * suspend threads, so there is no process-wide suspension to amortize. + * `@param` hook_ids The names of the hooks to enable. * `@return` The number of hooks now active.For
disable_hooks(after line 850):* and skips unknown ids, and counts an already-disabled hook as a * success (disable is idempotent). Ergonomics only (see * `@ref` enable_hooks). + * `@param` hook_ids The names of the hooks to disable. * `@return` The number of hooks now disabled.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@include/DetourModKit/hook_manager.hpp` around lines 830 - 854, Add explicit `@param` hook_ids tags to the Doxygen blocks for enable_hooks and disable_hooks: locate the comment above the enable_hooks declaration (function name enable_hooks) and insert a "`@param` hook_ids The names of the hooks to enable." line; do the same for the disable_hooks comment (function name disable_hooks) with "`@param` hook_ids The names of the hooks to disable." to match the project's existing doc style.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.clang-format:
- Line 6: The .clang-format file sets "Standard: c++20" while the project uses
C++23; update the Standard field in .clang-format from "c++20" to "c++23" so the
formatter recognizes C++23 syntax (locate the "Standard:" entry in the
.clang-format file and change its value to c++23 to match CMAKE_CXX_STANDARD and
project guidelines).
---
Outside diff comments:
In @.github/workflows/quality.yml:
- Around line 1-69: The workflow grants excessive default token rights and uses
mutable checkout tags; harden it by adding a top-level permissions: contents:
read entry to restrict GITHUB_TOKEN to repository read-only, replace both uses
of actions/checkout@v4 with the pinned commit SHA
(34e114876b0b11c390a56381ad16ebd13914f8d5) and set with: persist-credentials:
false on the Checkout steps (both the Checkout code step in the format-check job
and the Checkout code step in the sanitizers job that currently includes
submodules) to prevent credential persistence/leakage.
---
Nitpick comments:
In `@include/DetourModKit/hook_manager.hpp`:
- Around line 830-854: Add explicit `@param` hook_ids tags to the Doxygen blocks
for enable_hooks and disable_hooks: locate the comment above the enable_hooks
declaration (function name enable_hooks) and insert a "`@param` hook_ids The names
of the hooks to enable." line; do the same for the disable_hooks comment
(function name disable_hooks) with "`@param` hook_ids The names of the hooks to
disable." to match the project's existing doc style.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea27e4b3-af18-4b78-ac5e-898805684da6
📒 Files selected for processing (26)
.clang-format.clang-tidy.editorconfig.github/workflows/quality.ymlAGENTS.mdCMakePresets.jsonREADME.mddocs/misc/rtti-self-heal.mdinclude/DetourModKit/diagnostics.hppinclude/DetourModKit/drift_manifest.hppinclude/DetourModKit/hook_manager.hppsrc/async_logger.cppsrc/bootstrap.cppsrc/config.cppsrc/config_watcher.cppsrc/diagnostics.cppsrc/drift_manifest.cppsrc/hook_manager.cppsrc/input.cppsrc/logger.cppsrc/memory.cppsrc/scanner.cppsrc/worker.cpptests/test_diagnostics.cpptests/test_drift_manifest.cpptests/test_hook_manager.cpp
💤 Files with no reviewable changes (1)
- src/scanner.cpp
- pin actions/checkout to a commit SHA, add contents: read, set persist-credentials: false - run the ASan/UBSan probe via setup-msys2 (mingw-w64-x86_64-gcc ships the runtimes the default runner MinGW lacks) - .clang-format: use Standard: Latest (clang-format has no c++23 enum) so C++23 parses
Summary
Follow-up batch from the independent audit: two new modules, a hook-manager convenience API, and developer tooling, on top of the prior hardening PR. Full MinGW suite passes (1389/1389).
Changes
enable_hooks/disable_hooksandenable_all_hooks/disable_all_hooksthat toggle many hooks under one lock (ergonomics, not a perf change)..clang-format/.clang-tidy/.editorconfig, amingw-debug-coveragepreset, and a non-blockingquality.yml(ASan/UBSan probe plus advisory clang-format).usingdirectives in config, scanner, and hook_manager.Notes
The advisory
quality.ymljobs are non-blocking and do not gate PRs. No public API was removed.Summary by CodeRabbit
New Features
Documentation
Chores