Skip to content

feat(cli): QtMesh Cloud remote rules + automatic scan upload - #287

Merged
fernandotonon merged 6 commits into
masterfrom
feature/qtmesh-cloud-rules-upload
Apr 15, 2026
Merged

feat(cli): QtMesh Cloud remote rules + automatic scan upload#287
fernandotonon merged 6 commits into
masterfrom
feature/qtmesh-cloud-rules-upload

Conversation

@fernandotonon

@fernandotonon fernandotonon commented Apr 14, 2026

Copy link
Copy Markdown
Owner

Summary

Implements QtMesh Cloud integration for qtmesh scan: remote rules when no local config exists, and automatic POST of the same JSON as --json after each run when an ingest token is set.

Config precedence

  1. --config <path> — load file; never fetch remote rules. If a token is set, scan JSON is still uploaded (unless --no-upload). A short note is printed only when a token is present.
  2. Local qtmesh.yml / qtmesh.yaml / qtmesh.json in cwd — load it; do not fetch remote rules (stderr note).
  3. Else if QTMESH_TOKEN, QTMESH_CLOUD_TOKEN, or --token is set — GET /v1/ingest/rules; on failure, fallback to built-in defaults with a warning.
  4. Else built-in defaults.

Upload / exit codes

  • POST /v1/ingest/scan with compact JSON (same schema as --json) whenever a token is set, even without --json on the command line.
  • Upload failure: stderr with HTTP status and body snippet; exit code unchanged by default (scan fail_on only).
  • --strict-upload — exit 1 if upload fails (for CI).\n+ - --no-upload — skip upload when a token is set.

Other

  • QTMESH_API_BASE — override API base (default https://api.qtmesh.dev), e.g. for tests or self-hosted.
  • Minimal validation of remote config: numeric version, object scan, object rules.

Tests

  • QtMeshCloudClient validation unit tests
  • CLI: precedence (local yml vs token), strict vs warn-only upload failure, scanReportToJsonObject vs formatJson parity

Made with Cursor

Summary by CodeRabbit

  • New Features

    • Optional QtMesh Cloud: remote rule fetching and optional automatic scan-report upload; CLI flags to provide a token, disable upload, and enable strict-upload
    • Action inputs to pass cloud token and API host into CI runs
  • Behavior Changes

    • Local config now overrides remote rules
    • Reports include UTC start/end timestamps; SARIF includes invocation times
    • JSON output serialized consistently and load-error messages are redacted
  • Documentation

    • Example config comments clarifying cloud behavior
  • Tests

    • New tests for cloud fetch/upload, precedence, timestamps, and exit-code behaviors

- Add QtMeshCloudClient for GET /v1/ingest/rules and POST /v1/ingest/scan with retries
- Config precedence: --config; else local qtmesh.yml|yaml|json; else token+API; else defaults
- Fallback to defaults when API is unreachable (with warning)
- Upload scan JSON when token is set; --no-upload, --strict-upload; env QTMESH_TOKEN
- Refactor ScanEngine::scanReportToJsonObject shared by JSON output and upload
- Tests: validation, precedence, strict upload exit code, JSON parity

Made-with: Cursor
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Apr 14, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds QtMesh Cloud integration: new QtMeshCloudClient for fetching remote rules and uploading scan reports; CLI scan gains token/upload flags and new config-precedence behavior; ScanEngine centralizes JSON report construction and UTC timestamps; tests, build files, and CI/action inputs updated accordingly.

Changes

Cohort / File(s) Summary
QtMesh Cloud client
src/QtMeshCloudClient.h, src/QtMeshCloudClient.cpp
New non-instantiable client exposing apiBaseUrl(), validateCloudConfigJson(), fetchRules() (GET /v1/ingest/rules) and uploadScanReport() (POST /v1/ingest/scan). Implements retries/backoff, response-body snippets, JSON validation, and structured result types.
CLI scan integration
src/CLIPipeline.cpp
Adds --token, --no-upload, --strict-upload; resolves ingest token from flag/env; changes config-loading precedence (explicit/local override remote; fetch remote only when no local config and token present); builds unified JSON report and optionally uploads it, with configurable strict failure behavior and UTC timestamps in human-readable summary.
Scan report JSON export
src/ScanEngine.h, src/ScanEngine.cpp, src/ScanEngine_test.cpp
Introduces scanReportToJsonObject() and scanReportUtcTimes(); centralizes JSON construction/serialization; adds scanStartedUtc/scanCompletedUtc; redacts load-error finding messages and removes asset errorMessage; SARIF includes invocation times.
Tests and env helpers
src/CLIPipeline_test.cpp, src/QtMeshCloudClient_test.cpp
Adds ScopedEnvVar RAII helper; extends CLI tests for strict-upload and upload-failure behavior and local-config precedence; adds unit tests for cloud config JSON validation and updates expectations for timestamps and redaction.
Build & CI plumbing
src/CMakeLists.txt, tests/CMakeLists.txt, .github/actions/qtmesh/action.yml, action.yml
Adds QtMeshCloudClient source/header to main and test build lists; exposes optional composite action inputs qtmesh-token and qtmesh-api-base and forwards them into container env when provided.
Examples / docs
qtmesh.example.yml
Inserted documentation comments describing cloud behavior: token/env-driven remote rule fetch when no local config and automatic upload of scan JSON after runs.
Version bump
CMakeLists.txt
Project version updated from 2.25.1 to 2.26.0.

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI as CLIPipeline
    participant Local as LocalConfig
    participant CloudClient as QtMeshCloudClient
    participant API as CloudAPI
    participant Engine as ScanEngine

    User->>CLI: qtmesh scan [--token T] [--no-upload] [--strict-upload]
    CLI->>Local: check explicit --config / local qtmesh.yml|yaml|json
    alt explicit --config
        Local-->>CLI: load config (skip remote)
    else no --config
        alt local config exists
            Local-->>CLI: load local config
        else no local config
            alt ingest token present
                CLI->>CloudClient: fetchRules(token)
                CloudClient->>API: GET /v1/ingest/rules
                API-->>CloudClient: 2xx + JSON / error
                CloudClient-->>CLI: RulesResult (ok/config/source or error)
                alt fetch success
                    CLI->>Engine: use fetched config
                else
                    CLI->>Engine: use defaults
                end
            else
                CLI->>Engine: use defaults
            end
        end
    end
    CLI->>Engine: performScan(config)
    Engine-->>CLI: ScanResult
    alt ingest token present AND not --no-upload
        CLI->>Engine: scanReportToJsonObject(result)
        Engine-->>CLI: QJsonObject
        CLI->>CloudClient: uploadScanReport(token, json)
        CloudClient->>API: POST /v1/ingest/scan
        API-->>CloudClient: 2xx or error
        CloudClient-->>CLI: UploadResult (ok,httpStatus,error)
        alt --strict-upload AND upload failed
            CLI-->>User: exit 1
        else
            CLI-->>User: exit (scan threshold)
        end
    else
        CLI-->>User: exit (scan threshold)
    end
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰
I hopped with a token, nose twitching with glee,
Fetched rules from the cloud and sent reports back free,
Retries for the rain and snippets kept neat,
Timestamps in UTC for each tiny feat,
Carrots and tests — happy hops down the street!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.92% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main feature: QtMesh Cloud remote rules fetching and automatic scan upload functionality.
Description check ✅ Passed The description comprehensively covers the implementation with summary, technical details, config precedence, upload behavior, tests, and environment variable usage; it closely follows the template structure.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/qtmesh-cloud-rules-upload

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/ScanEngine.cpp (1)

710-713: ⚠️ Potential issue | 🟠 Major

Don’t ship raw importer error text in the cloud payload.

Line 120 stores Assimp’s raw GetErrorString() in asset.errorMessage, and this shared serializer now reuses it for upload. Those messages can include absolute local paths, so token-enabled scans can leak workstation/project paths even though filePath itself is omitted.

Please redact path-like substrings or exclude errorMessage from the upload payload.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/ScanEngine.cpp` around lines 710 - 713, The code currently copies
Assimp's raw importer text from asset.errorMessage into the upload payload
(ao["errorMessage"]), which can leak local absolute paths; change the serializer
in ScanEngine.cpp to avoid shipping raw messages by either omitting
ao["errorMessage"] entirely when asset.loadError is true or by sanitizing
asset.errorMessage first—detect and strip/redact path-like substrings (e.g.,
drive letters, UNC prefixes, leading "/" sequences, or sequences containing path
separators and filenames) before assigning to ao["errorMessage"]; update the
logic around asset.loadError / asset.errorMessage and ensure any tests or
consumers handle the missing or redacted field.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@qtmesh.example.yml`:
- Around line 8-9: Update the QtMesh cloud note to mention the CLI --token flag
as a valid token source that enables remote rules and upload behavior;
specifically, modify the comment line describing "QtMesh Cloud" so it lists both
environment variables (QTMESH_TOKEN / QTMESH_CLOUD_TOKEN) and the CLI --token
option as triggering remote rules and scan JSON upload, reflecting the new
precedence that treats --token the same as env vars.

In `@src/CLIPipeline_test.cpp`:
- Around line 2314-2362: The action.yml lacks inputs and environment forwarding
for cloud auth and upload flags; add inputs named token, no-upload,
strict-upload (or clearly document using the existing options input) and inputs
for QTMESH_API_BASE and QTMESH_TOKEN, then update the Docker invocation to
forward those env inputs via --env QTMESH_API_BASE=${{ inputs.QTMESH_API_BASE }}
and --env QTMESH_TOKEN=${{ inputs.QTMESH_TOKEN }} and include the
token/no-upload/strict-upload flags when building the docker run command (or
append them from inputs/options) so the CLI flags (--token, --no-upload,
--strict-upload) and environment variables are exposed to the container.

In `@src/CLIPipeline.cpp`:
- Around line 2254-2263: The upload error message in CLIPipeline.cpp doesn't
include the UploadResult.responseBodySnippet when
QtMeshCloudClient::uploadScanReport fails; update the error logging in the block
that checks up.ok (where uploadOk and const auto up are used) to append
up.responseBodySnippet to the existing stderr message (alongside up.httpStatus
and up.errorString) so failed uploads print the HTTP status, error string, and
the responseBodySnippet for debugging.

In `@src/QtMeshCloudClient.cpp`:
- Around line 44-50: The JSON validation currently allows any non-empty string
for the "version" field (variable ver), so change the check in the JSON parsing
routine to reject string types entirely: require ver to be numeric (isDouble)
and return false for ver.isString() even if non-empty; keep null/undefined
checks. Update the parsing/validation logic around ver in QtMeshCloudClient.cpp
(the block using root.value(QStringLiteral("version")) and ver) to only accept
numeric versions and fall back to defaults otherwise. Add a regression test that
submits a payload with "version": "foo" and asserts the parser rejects it (or
falls back to defaults) to prevent this from regressing.
- Around line 63-148: Add Sentry breadcrumbs around the rules fetch flow by
calling SentryReporter::addBreadcrumb("file.import", "fetchRules:start")
immediately before the network request in QtMeshCloudClient::fetchRules, then
add success and failure breadcrumbs (e.g. "file.import:success" with source info
and "file.import:failure" with out.errorString or transportErr/HTTP status) at
the same points where the function currently returns success or sets
out.errorString; apply the same pattern to the scan upload flow referenced in
the 150-218 region (use category "file.export" and analogous
start/success/failure messages), ensuring breadcrumbs include minimal context
(source, HTTP status or trimmed body) and are added before each early return.

---

Outside diff comments:
In `@src/ScanEngine.cpp`:
- Around line 710-713: The code currently copies Assimp's raw importer text from
asset.errorMessage into the upload payload (ao["errorMessage"]), which can leak
local absolute paths; change the serializer in ScanEngine.cpp to avoid shipping
raw messages by either omitting ao["errorMessage"] entirely when asset.loadError
is true or by sanitizing asset.errorMessage first—detect and strip/redact
path-like substrings (e.g., drive letters, UNC prefixes, leading "/" sequences,
or sequences containing path separators and filenames) before assigning to
ao["errorMessage"]; update the logic around asset.loadError / asset.errorMessage
and ensure any tests or consumers handle the missing or redacted field.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: c04c8fa4-53b2-458d-8a28-cdf0af912b33

📥 Commits

Reviewing files that changed from the base of the PR and between ba6502a and 7f40a83.

📒 Files selected for processing (10)
  • qtmesh.example.yml
  • src/CLIPipeline.cpp
  • src/CLIPipeline_test.cpp
  • src/CMakeLists.txt
  • src/QtMeshCloudClient.cpp
  • src/QtMeshCloudClient.h
  • src/QtMeshCloudClient_test.cpp
  • src/ScanEngine.cpp
  • src/ScanEngine.h
  • src/ScanEngine_test.cpp

Comment thread qtmesh.example.yml Outdated
Comment thread src/CLIPipeline_test.cpp
Comment on lines +2314 to +2362
TEST(CLIPipelineCmdScanCloud, StrictUploadFailsWhenApiUnreachable)
{
QTemporaryDir tmpDir;
ASSERT_TRUE(tmpDir.isValid());
ScopedCurrentDir scoped(tmpDir.path());
ScopedEnvVar api("QTMESH_API_BASE", "http://127.0.0.1:1");
ScopedEnvVar tok("QTMESH_TOKEN", "test-token");
TestArgv args({"qtmesh", "scan", "--strict-upload"});
EXPECT_EQ(CLIPipeline::cmdScan(args.argc(), args.argv()), 1);
}

TEST(CLIPipelineCmdScanCloud, UploadFailureDoesNotChangeExitCodeWithoutStrict)
{
QTemporaryDir tmpDir;
ASSERT_TRUE(tmpDir.isValid());
ScopedCurrentDir scoped(tmpDir.path());
ScopedEnvVar api("QTMESH_API_BASE", "http://127.0.0.1:1");
ScopedEnvVar tok("QTMESH_TOKEN", "test-token");
TestArgv args({"qtmesh", "scan"});
EXPECT_EQ(CLIPipeline::cmdScan(args.argc(), args.argv()), 0);
}

TEST(CLIPipelineCmdScanCloud, LocalYmlOverridesRemoteTokenForRules)
{
QTemporaryDir tmpDir;
ASSERT_TRUE(tmpDir.isValid());
const QString rootPath = QDir(tmpDir.path()).filePath("assets");
ASSERT_TRUE(QDir().mkpath(rootPath));
ASSERT_FALSE(writeMinimalObj(rootPath, "scan_mesh.obj").isEmpty());

const QString ymlPath = QDir(tmpDir.path()).filePath("qtmesh.yml");
QFile yml(ymlPath);
ASSERT_TRUE(yml.open(QIODevice::WriteOnly | QIODevice::Text));
yml.write(
"scan:\n"
" include:\n"
" - \"**/*.obj\"\n"
"rules:\n"
" max_vertex_count: 2\n");
yml.close();

ScopedCurrentDir scoped(tmpDir.path());
ScopedEnvVar api("QTMESH_API_BASE", "http://127.0.0.1:1");
ScopedEnvVar tok("QTMESH_TOKEN", "test-token");

QByteArray rootBa = rootPath.toUtf8();
TestArgv args({"qtmesh", "scan", rootBa.constData()});
EXPECT_EQ(CLIPipeline::cmdScan(args.argc(), args.argv()), 1);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ACTION_FILE="$(fd -a '^action\.yml$' | head -n1)"
test -n "$ACTION_FILE"

printf 'Inspecting %s\n\n' "$ACTION_FILE"
sed -n '1,260p' "$ACTION_FILE"

printf '\nRelevant matches:\n'
rg -n -- '--token|--no-upload|--strict-upload|QTMESH_API_BASE|QTMESH_TOKEN|QTMESH_CLOUD_TOKEN' "$ACTION_FILE" || true

Repository: fernandotonon/QtMeshEditor

Length of output: 10491


Update action.yml to expose cloud authentication flags and environment variables.

The tests add --strict-upload, --no-upload, and --token flags, but action.yml does not expose them as inputs or pass QTMESH_API_BASE and QTMESH_TOKEN environment variables to the Docker container. GitHub Action users cannot fully use this feature.

Required changes to action.yml:

  • Add action inputs for --token, --no-upload, and --strict-upload (or document that they must be passed via the generic options input).
  • Add action inputs for QTMESH_API_BASE and QTMESH_TOKEN and forward them in the docker run command via the --env flag so users can authenticate against a private API.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/CLIPipeline_test.cpp` around lines 2314 - 2362, The action.yml lacks
inputs and environment forwarding for cloud auth and upload flags; add inputs
named token, no-upload, strict-upload (or clearly document using the existing
options input) and inputs for QTMESH_API_BASE and QTMESH_TOKEN, then update the
Docker invocation to forward those env inputs via --env QTMESH_API_BASE=${{
inputs.QTMESH_API_BASE }} and --env QTMESH_TOKEN=${{ inputs.QTMESH_TOKEN }} and
include the token/no-upload/strict-upload flags when building the docker run
command (or append them from inputs/options) so the CLI flags (--token,
--no-upload, --strict-upload) and environment variables are exposed to the
container.

Comment thread src/CLIPipeline.cpp
Comment thread src/QtMeshCloudClient.cpp
Comment on lines +44 to +50
const QJsonValue ver = root.value(QStringLiteral("version"));
if (ver.isUndefined() || ver.isNull())
return false;
if (!ver.isDouble() && !ver.isString())
return false;
if (ver.isString() && ver.toString().trimmed().isEmpty())
return false;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Reject string version values here.

Line 47 currently accepts any non-empty string, so payloads like { "version": "foo", "scan": {}, "rules": {} } pass validation instead of falling back to defaults.

🔒 Suggested fix
     const QJsonValue ver = root.value(QStringLiteral("version"));
     if (ver.isUndefined() || ver.isNull())
         return false;
-    if (!ver.isDouble() && !ver.isString())
-        return false;
-    if (ver.isString() && ver.toString().trimmed().isEmpty())
+    if (!ver.isDouble())
         return false;

Please add a regression case for a non-numeric version alongside this.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const QJsonValue ver = root.value(QStringLiteral("version"));
if (ver.isUndefined() || ver.isNull())
return false;
if (!ver.isDouble() && !ver.isString())
return false;
if (ver.isString() && ver.toString().trimmed().isEmpty())
return false;
const QJsonValue ver = root.value(QStringLiteral("version"));
if (ver.isUndefined() || ver.isNull())
return false;
if (!ver.isDouble())
return false;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/QtMeshCloudClient.cpp` around lines 44 - 50, The JSON validation
currently allows any non-empty string for the "version" field (variable ver), so
change the check in the JSON parsing routine to reject string types entirely:
require ver to be numeric (isDouble) and return false for ver.isString() even if
non-empty; keep null/undefined checks. Update the parsing/validation logic
around ver in QtMeshCloudClient.cpp (the block using
root.value(QStringLiteral("version")) and ver) to only accept numeric versions
and fall back to defaults otherwise. Add a regression test that submits a
payload with "version": "foo" and asserts the parser rejects it (or falls back
to defaults) to prevent this from regressing.

Comment thread src/QtMeshCloudClient.cpp
- Remove invalid .toUtf8() on QJsonDocument::toJson() (Qt 6 returns QByteArray)
- validateCloudConfigJson: version must be JSON number; add string-version test
- Sentry breadcrumbs for fetchRules / uploadScan; stderr upload body line
- Sanitize load_error in exported JSON; omit per-asset errorMessage field
- GitHub Action: optional qtmesh-token and qtmesh-api-base forwarded to Docker

Made-with: Cursor
tests/CMakeLists.txt builds CLIPipeline.cpp for MaterialEditorQML_* targets
but omitted QtMeshCloudClient.cpp, causing undefined references to fetchRules
and uploadScanReport on Linux CI.

Made-with: Cursor

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tests/CMakeLists.txt (1)

58-58: Consider de-duplicating source inventory between src and tests CMake files.

This manual addition works, but it reinforces list drift risk since tests/CMakeLists.txt mirrors a long source list from src/CMakeLists.txt. Consider deriving TEST_SRC_FILES from a shared variable or helper include to prevent future misses.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/CMakeLists.txt` at line 58, The tests CMake file manually re-lists
QtMeshCloudClient.cpp causing duplication risk; stop adding
${CMAKE_CURRENT_SOURCE_DIR}/../src/QtMeshCloudClient.cpp directly and instead
make tests derive TEST_SRC_FILES from the shared source list used by the main
build (e.g. export the src file list variable from the src CMake logic or move
the canonical list into a small include like SourceFiles.cmake), then include or
reference that variable in tests/CMakeLists.txt (replace the explicit
QtMeshCloudClient.cpp entry with the shared variable) so TEST_SRC_FILES and the
main project use the same single source-of-truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/CMakeLists.txt`:
- Line 58: The tests CMake file manually re-lists QtMeshCloudClient.cpp causing
duplication risk; stop adding
${CMAKE_CURRENT_SOURCE_DIR}/../src/QtMeshCloudClient.cpp directly and instead
make tests derive TEST_SRC_FILES from the shared source list used by the main
build (e.g. export the src file list variable from the src CMake logic or move
the canonical list into a small include like SourceFiles.cmake), then include or
reference that variable in tests/CMakeLists.txt (replace the explicit
QtMeshCloudClient.cpp entry with the shared variable) so TEST_SRC_FILES and the
main project use the same single source-of-truth.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f6ba93e4-141f-4677-b68a-798759af0a9c

📥 Commits

Reviewing files that changed from the base of the PR and between d10fe8d and 0812954.

📒 Files selected for processing (1)
  • tests/CMakeLists.txt

Record scanStartedUtc/scanCompletedUtc in ScanEngine::run and expose
scanReportUtcTimes for consistent ISO-8601 UTC bounds in reports.
Extend CLI summary and tests accordingly.

Made-with: Cursor

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/CLIPipeline.cpp (1)

2056-2068: Breadcrumb the new cloud fetch/upload paths.

Remote rule fetches and scan uploads are new user-visible network operations, but neither path records a breadcrumb. That makes failures in the exact flows this PR adds much harder to reconstruct in Sentry.

As per coding guidelines, "Track all user-facing actions and significant operations with SentryReporter::addBreadcrumb(category, message)."

Also applies to: 2258-2271

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/CLIPipeline.cpp` around lines 2056 - 2068, When fetching remote rules in
CLIPipeline (use resolveIngestToken and QtMeshCloudClient::fetchRules logic) and
when performing the corresponding scan upload flow later (the block around the
other similar code at the region matching 2258-2271), add
SentryReporter::addBreadcrumb calls to record these user-visible network
operations: breadcrumb category like "network/cloud" and message indicating the
operation and source/token (e.g., "fetch-rules: <source or token>" on request
start and "fetch-rules-result: success" or include rules.errorString on
failure), and likewise for the upload flow record start and success/failure
messages; place these breadcrumbs immediately before/after the fetchRules call
and in the upload code paths so Sentry captures both attempts and outcomes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/CLIPipeline.cpp`:
- Around line 2263-2268: The log currently always uses the "Error:" prefix when
an upload fails (checking up.ok), which is misleading unless strict upload mode
is enabled; update the block that handles (!up.ok) to choose the prefix
dynamically (e.g., use "Error:" when the strict-upload flag/variable—such as
strictUpload—is true, otherwise use "Warning:") and write that prefix into the
err() output before the existing message (still include up.httpStatus,
up.errorString and the responseBodySnippet logic unchanged); ensure behavior for
strictUpload remains unchanged (error prefix and exit behavior) and non-strict
keeps only a warning prefix.

In `@src/ScanEngine.cpp`:
- Around line 666-671: The exported message for load errors currently misleads
local JSON-only runs; update findingMessageForExport to return a clear redaction
notice like "Failed to load asset (details redacted from exported JSON)" instead
of "see local CLI output", and also ensure cmdScan emits the full raw f.message
to stderr when running in JSON-only mode so local users still get the real load
failure details; adjust scanReportToJsonObject usage if needed to keep exported
JSON redacted while cmdScan prints the original Finding.message for visibility.

---

Nitpick comments:
In `@src/CLIPipeline.cpp`:
- Around line 2056-2068: When fetching remote rules in CLIPipeline (use
resolveIngestToken and QtMeshCloudClient::fetchRules logic) and when performing
the corresponding scan upload flow later (the block around the other similar
code at the region matching 2258-2271), add SentryReporter::addBreadcrumb calls
to record these user-visible network operations: breadcrumb category like
"network/cloud" and message indicating the operation and source/token (e.g.,
"fetch-rules: <source or token>" on request start and "fetch-rules-result:
success" or include rules.errorString on failure), and likewise for the upload
flow record start and success/failure messages; place these breadcrumbs
immediately before/after the fetchRules call and in the upload code paths so
Sentry captures both attempts and outcomes.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7b6cdddb-bbbf-4a11-a324-bb5f8d6d151e

📥 Commits

Reviewing files that changed from the base of the PR and between 61b98b5 and a562b2d.

📒 Files selected for processing (5)
  • src/CLIPipeline.cpp
  • src/CLIPipeline_test.cpp
  • src/ScanEngine.cpp
  • src/ScanEngine.h
  • src/ScanEngine_test.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/ScanEngine.h
  • src/CLIPipeline_test.cpp

Comment thread src/CLIPipeline.cpp Outdated
Comment thread src/ScanEngine.cpp
Comment on lines +666 to +671
/// Exported JSON (and cloud upload) must not embed Assimp paths or other local details.
static QString findingMessageForExport(const Finding& f)
{
if (f.rule == QLatin1String("load_error"))
return QStringLiteral("Failed to load asset (see local CLI output for details)");
return f.message;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

The redacted load_error text is misleading for local --json runs.

scanReportToJsonObject() now feeds local JSON output too, but cmdScan() suppresses per-asset text in that mode. For JSON-only runs, “see local CLI output” is false and the actual load reason is no longer recoverable from the command output. Either emit the raw detail to stderr in JSON mode or change this string to say the details were redacted.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/ScanEngine.cpp` around lines 666 - 671, The exported message for load
errors currently misleads local JSON-only runs; update findingMessageForExport
to return a clear redaction notice like "Failed to load asset (details redacted
from exported JSON)" instead of "see local CLI output", and also ensure cmdScan
emits the full raw f.message to stderr when running in JSON-only mode so local
users still get the real load failure details; adjust scanReportToJsonObject
usage if needed to keep exported JSON redacted while cmdScan prints the original
Finding.message for visibility.

- Document --token alongside env vars in qtmesh.example.yml
- GitHub Actions: qtmesh-no-upload and qtmesh-strict-upload inputs append CLI flags
- CLIPipeline: strict vs non-strict upload log prefix; merge response snippet;
  Sentry breadcrumbs for cloud rules/upload; stderr load_error details in --json
- ScanEngine: clearer redacted load_error export text; reject bool cloud version
- Tests: update JSON redaction expectation; add RejectsBoolVersion

Made-with: Cursor
@sonarqubecloud

Copy link
Copy Markdown

@fernandotonon
fernandotonon merged commit bf046ec into master Apr 15, 2026
19 checks passed
@fernandotonon
fernandotonon deleted the feature/qtmesh-cloud-rules-upload branch April 15, 2026 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant