[codex] Add QtMesh Cloud asset upload client - #695
Conversation
Add `qtmesh scan --target <id>` as a CI-friendly alias for selecting a bundled platform profile. Active profile is emitted into JSON, SARIF, and text reports. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
More reviews will be available in 46 minutes and 30 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. 📝 WalkthroughWalkthroughThe PR adds CLI profile tracking and cloud project/upload API methods. The ChangesCLI Profile Tracking
Cloud Project Upload API
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a4004acb3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| out.errorString = QStringLiteral("could not open file: %1").arg(pathLeaf(localPath)); | ||
| return out; | ||
| } | ||
| const QByteArray payload = file.readAll(); |
There was a problem hiding this comment.
When uploading large meshes or texture archives, file.readAll() loads the entire asset into a QByteArray before QNetworkAccessManager::put, so the editor/CLI must hold a full extra copy of the file in memory and can freeze or run out of memory on multi-GB assets. Since this client is intended for asset uploads, keep the QFile open and pass it as the upload device instead of materializing the whole payload.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/QtMeshCloudClient.cpp (2)
487-518: ⚖️ Poor tradeoffAvoid loading the entire asset into memory; stream from the
QFiledevice.
file.readAll()materializes the whole file into aQByteArray(andnam.put(req, payload)holds another reference). With a 120s default timeout the API clearly anticipates large assets, so this can spike memory by the full file size per upload. Since the request is driven synchronously by a localQEventLoop, theQFilestays alive for the request duration and can be passed as the upload device, usingfile.size()for the pre-flight checks.♻️ Stream the file instead of buffering it
QFile file(localPath); if (!file.open(QIODevice::ReadOnly)) { out.errorString = QStringLiteral("could not open file: %1").arg(pathLeaf(localPath)); return out; } - const QByteArray payload = file.readAll(); - file.close(); - if (target.sizeBytes > 0 && payload.size() != target.sizeBytes) { + const qint64 fileSize = file.size(); + if (target.sizeBytes > 0 && fileSize != target.sizeBytes) { out.errorString = QStringLiteral("file size changed before upload: %1").arg(pathLeaf(localPath)); return out; } @@ QNetworkRequest req(url); req.setHeader(QNetworkRequest::UserAgentHeader, QStringLiteral("qtmesheditor")); - req.setHeader(QNetworkRequest::ContentLengthHeader, payload.size()); + req.setHeader(QNetworkRequest::ContentLengthHeader, fileSize);Then upload via the device (
reply = nam.put(req, &file);) and defaultout.sizeBytestofileSize.🤖 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 `@src/QtMeshCloudClient.cpp` around lines 487 - 518, The code currently reads the whole file into memory via file.readAll() and calls nam.put(req, payload); instead stream the upload from the QFile device: open the QFile (use file.size() for the size check and for ContentLengthHeader), do not call file.readAll(), and call nam.put(req, &file) so the network layer streams from the file; ensure the QFile remains alive for the lifetime of the QNetworkReply (e.g. allocate QFile on the heap or otherwise keep it in scope and close/delete it only after reply finishes) and update references to remove payload and out.sizeBytes to reflect file.size().
519-521: ⚖️ Poor tradeoffConfirm these synchronous uploads never run on the GUI thread.
loop.exec()blocks the calling thread until the transfer completes (up to the 120stimeoutMsforuploadFileContent). If any of these methods are invoked from the GUI thread in the upcoming upload flow (issue#684), the UI will freeze for the duration. Consider documenting/enforcing that callers run these on a worker thread.🤖 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 `@src/QtMeshCloudClient.cpp` around lines 519 - 521, The synchronous wait using QEventLoop::exec() (the three lines with QEventLoop loop; QObject::connect(reply, &QNetworkReply::finished, &loop, &QEventLoop::quit); loop.exec();) can block the GUI thread for up to timeoutMs in uploadFileContent; enforce that uploadFileContent (and any callers) never run on the GUI thread by adding a runtime check at the start of uploadFileContent (compare QThread::currentThread() to qApp->thread()) and either assert/return an error or automatically offload the work to a worker thread (e.g., QtConcurrent::run or QMetaObject::invokeMethod on a worker object with Qt::QueuedConnection) so the blocking loop.exec() runs only on a non-GUI thread; alternatively convert the function to an asynchronous API using the QNetworkReply finished signal and a callback/future instead of blocking.
🤖 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.
Nitpick comments:
In `@src/QtMeshCloudClient.cpp`:
- Around line 487-518: The code currently reads the whole file into memory via
file.readAll() and calls nam.put(req, payload); instead stream the upload from
the QFile device: open the QFile (use file.size() for the size check and for
ContentLengthHeader), do not call file.readAll(), and call nam.put(req, &file)
so the network layer streams from the file; ensure the QFile remains alive for
the lifetime of the QNetworkReply (e.g. allocate QFile on the heap or otherwise
keep it in scope and close/delete it only after reply finishes) and update
references to remove payload and out.sizeBytes to reflect file.size().
- Around line 519-521: The synchronous wait using QEventLoop::exec() (the three
lines with QEventLoop loop; QObject::connect(reply, &QNetworkReply::finished,
&loop, &QEventLoop::quit); loop.exec();) can block the GUI thread for up to
timeoutMs in uploadFileContent; enforce that uploadFileContent (and any callers)
never run on the GUI thread by adding a runtime check at the start of
uploadFileContent (compare QThread::currentThread() to qApp->thread()) and
either assert/return an error or automatically offload the work to a worker
thread (e.g., QtConcurrent::run or QMetaObject::invokeMethod on a worker object
with Qt::QueuedConnection) so the blocking loop.exec() runs only on a non-GUI
thread; alternatively convert the function to an asynchronous API using the
QNetworkReply finished signal and a callback/future instead of blocking.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 56eb3b98-62ab-4f6f-bdcb-8978554750f4
📒 Files selected for processing (7)
src/CLIPipeline.cppsrc/CLIPipeline_test.cppsrc/QtMeshCloudClient.cppsrc/QtMeshCloudClient.hsrc/QtMeshCloudClient_test.cppsrc/ScanEngine.cppsrc/ScanEngine.h
|
1 participant
Summary
Adds the first editor-side client surface for QtMesh Cloud asset uploads, matching the qtmesh.dev upload API implemented for cloud asset retention.
Changes
QtMeshCloudClientwith bearer-token methods to:fetchRules,uploadScanReport) intact for CLI scan compatibility.Notes
This is the client/API foundation only. The editor still needs follow-up slices for browser/device login, token persistence, packaging/dependency discovery, and GUI upload flow from issue #684.
Validation
cmake --build build_local --target UnitTests -j2QT_QPA_PLATFORM=offscreen ./build_local/bin/UnitTests --gtest_filter='QtMeshCloudClient*'Focused test result: 21 QtMeshCloudClient tests passed.
Summary by CodeRabbit
New Features
--targetoption to scan CLI as a CI-friendly alternative to--profileBug Fixes
--targetand--profilevaluesTests