[codex] Add authenticated cloud file uploads#698
Conversation
|
Warning Review limit reached
More reviews will be available in 14 minutes and 51 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR implements a complete QtMesh Cloud file upload flow: adds CloudUploadPlanner utilities (slug generation, role inference, descriptor building), tests, build updates, MainWindow menu/handler for multi-file upload with project selection/creation, and QtMeshCloudClient support for project listing and cancellable uploads. ChangesCloud File Upload Feature
Sequence DiagramsequenceDiagram
participant User
participant MainWindow
participant CloudAuth
participant FileDialog
participant CloudUploadPlanner
participant QtMeshCloudClient
participant UploadWorker
participant ProgressDialog
User->>MainWindow: Click "Upload Files…"
MainWindow->>CloudAuth: Check sign-in status
MainWindow->>FileDialog: Show file picker
FileDialog-->>MainWindow: Return selected paths
MainWindow->>MainWindow: Prompt for project name / choose existing
MainWindow->>CloudUploadPlanner: Generate slug from name
CloudUploadPlanner-->>MainWindow: Normalized project slug
MainWindow->>QtMeshCloudClient: fetchProjects / create project (handle 409)
QtMeshCloudClient-->>MainWindow: Project ID, upload URLs
MainWindow->>CloudUploadPlanner: Build asset descriptors
CloudUploadPlanner-->>MainWindow: Descriptors (role, mime, size)
MainWindow->>UploadWorker: Start upload on worker thread (pass atomic cancel)
UploadWorker->>QtMeshCloudClient: uploadFileContent (cancellable)
QtMeshCloudClient-->>UploadWorker: Upload result (or canceled)
UploadWorker->>ProgressDialog: Update progress
UploadWorker-->>MainWindow: Return outcome (ok/error/canceled)
MainWindow->>User: Show completion summary / open project URL
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: c930ab9ab5
ℹ️ 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".
| slug = QStringLiteral("%1-%2") | ||
| .arg(slug, QDateTime::currentDateTimeUtc().toString(QStringLiteral("yyyyMMddhhmmss"))); |
There was a problem hiding this comment.
When the initial generated slug is already at CloudUploadPlanner's 64-character limit, appending -yyyyMMddhhmmss on a 409 produces a 79-character slug. In the duplicate-long-project-name case the retry no longer satisfies the planner's own slug-length constraint and is likely to be rejected by the API instead of resolving the conflict; truncate the base slug before adding the timestamp suffix.
Useful? React with 👍 / 👎.
| if (mainFileId.isEmpty()) | ||
| mainFileId = uploads.at(i).fileId; |
There was a problem hiding this comment.
For multi-file uploads this marks whichever file happens to be returned first by the file dialog as mainFileId. If the user selects a texture/material/sidecar before the mesh, or the dialog returns an alphabetically earlier texture, completeUpload is told that the non-model file is the main asset even though descriptors include roles; that can make the cloud project scan/viewer attach to the wrong file. Prefer the first descriptor with role model and fall back only when no model is present.
Useful? React with 👍 / 👎.
mainwindow.cpp references CloudUploadPlanner; include the source in TEST_SRC_FILES so MaterialEditorQML_* executables link successfully. Co-authored-by: Cursor <cursoragent@cursor.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/mainwindow.cpp`:
- Around line 2533-2599: The worker currently only checks canceled between
network calls so canceling during an in-flight upload doesn't abort the active
QNetworkReply; update the upload path to be abortable by passing a cancellation
hook into QtMeshCloudClient::uploadFileContent (and/or completeUpload) so the
client can abort the active QNetworkReply immediately. Concretely, change
QtMeshCloudClient::uploadFileContent signature to accept a cancellation callback
or pointer to an atomic_bool (e.g. std::function<bool()> isCanceled or const
std::atomic_bool* canceled) and inside uploadFileContent store the
QNetworkReply*, connect the cancellation hook to call reply->abort(), and
poll/observe the hook while the reply is in-flight; in the worker lambda pass
&canceled (or a small lambda capturing &canceled) into uploadFileContent so
canceled.store(true) triggers immediate abort, and keep setting outcome.canceled
when abort is observed so the modal loop can quit promptly.
- Around line 2501-2506: The retry path for createProject uses the original slug
and simply appends a timestamp suffix, which can exceed the 64-char limit;
update the retry to re-normalize/truncate the slug before appending the "-%2"
timestamp so the final slug stays within 64 chars. Locate
CloudUploadPlanner::makeProjectSlug usage and the slug variable, then compute
the collision-safe base by truncating the original slug to (64 - 1 -
timestampLength) characters (accounting for the dash) or by invoking a helper
that enforces the 64-char limit, then append QStringLiteral("-%2") with
QDateTime::currentDateTimeUtc().toString(...) and call
QtMeshCloudClient::createProject(token, projectName, slug) with that normalized
retry slug. Ensure the final slug length is validated before retrying.
🪄 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: 3c04cc6f-615a-47d8-8f0c-b4c68995a35e
📒 Files selected for processing (6)
src/CMakeLists.txtsrc/CloudUploadPlanner.cppsrc/CloudUploadPlanner.hsrc/CloudUploadPlanner_test.cppsrc/mainwindow.cppsrc/mainwindow.h
Let uploads target existing cloud projects via GET /v1/projects, abort in-flight PUTs when the user cancels, re-normalize slug collision retries, and prefer model files as the upload main file. Co-authored-by: Cursor <cursoragent@cursor.com>
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 (2)
src/QtMeshCloudClient_test.cpp (1)
183-217: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftAdd tests for the new behavior, not just the argument guards.
This change adds project-list parsing and cooperative upload cancellation, but the new assertions only cover fast-fail input validation. Please add coverage for at least one parsed
fetchProjects()response and one canceleduploadFileContent()path.As per coding guidelines "
**/*_test.cpp: Add Google Test unit tests for new functionality."🤖 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_test.cpp` around lines 183 - 217, Your tests only cover argument-guard fast-fail paths; add unit tests that exercise the new parsing and cancellation behavior: add one test for QtMeshCloudClient::fetchProjects that injects a mocked successful HTTP response (JSON with at least one project entry) and assert result.ok is true and parsed project fields match the JSON, and add one test for QtMeshCloudClient::uploadFileContent that simulates a cooperative cancellation (construct an UploadTarget configured to cancel or trigger the cancel path used by UploadTarget) and assert the call returns ok==false with errorString indicating cancellation; follow the existing test patterns in the file for injecting network responses/timeouts and reference QtMeshCloudClient::fetchProjects, QtMeshCloudClient::uploadFileContent and QtMeshCloudClient::UploadTarget to locate code to exercise.src/mainwindow.cpp (1)
2627-2680:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPropagate cancellation into
completeUpload()as well.
canceledis only checked beforeQtMeshCloudClient::completeUpload(...). If the user hits Cancel during the finalization request, the modal loop still waits for that network call to finish, so the workflow can hang on a slow/stalled/files/completeeven though PUT uploads are now abortable.🤖 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/mainwindow.cpp` around lines 2627 - 2680, The finalization call doesn't propagate the cancellation flag into QtMeshCloudClient::completeUpload, so a user cancel during that network request will still block; update the worker lambda to call a cancellation-aware overload of completeUpload (or modify QtMeshCloudClient::completeUpload) to accept the same &canceled token, pass &canceled into the call, and after the call check for result.canceled and set outcome.canceled (and set outcome.errorString on error) just like you do for uploadFileContent so cancellation during completeUpload is handled and reported.
🤖 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 `@src/QtMeshCloudClient.cpp`:
- Around line 594-615: The code currently treats
root.value("projects").toArray() as [] when the field is missing or not an array
and still returns out.ok = true; change the parsing logic in the function that
calls parseJsonObjectBody (the loop that fills out.projects and sets out.ok) to
explicitly verify that root.contains("projects") and that
root.value(QStringLiteral("projects")).isArray(); if the check fails set a
descriptive out.errorString (e.g. "missing or invalid 'projects' field") and
return out without setting out.ok = true; keep the existing ProjectSummary
extraction loop for the valid-case array and only set out.ok = true after
successful validation.
---
Outside diff comments:
In `@src/mainwindow.cpp`:
- Around line 2627-2680: The finalization call doesn't propagate the
cancellation flag into QtMeshCloudClient::completeUpload, so a user cancel
during that network request will still block; update the worker lambda to call a
cancellation-aware overload of completeUpload (or modify
QtMeshCloudClient::completeUpload) to accept the same &canceled token, pass
&canceled into the call, and after the call check for result.canceled and set
outcome.canceled (and set outcome.errorString on error) just like you do for
uploadFileContent so cancellation during completeUpload is handled and reported.
In `@src/QtMeshCloudClient_test.cpp`:
- Around line 183-217: Your tests only cover argument-guard fast-fail paths; add
unit tests that exercise the new parsing and cancellation behavior: add one test
for QtMeshCloudClient::fetchProjects that injects a mocked successful HTTP
response (JSON with at least one project entry) and assert result.ok is true and
parsed project fields match the JSON, and add one test for
QtMeshCloudClient::uploadFileContent that simulates a cooperative cancellation
(construct an UploadTarget configured to cancel or trigger the cancel path used
by UploadTarget) and assert the call returns ok==false with errorString
indicating cancellation; follow the existing test patterns in the file for
injecting network responses/timeouts and reference
QtMeshCloudClient::fetchProjects, QtMeshCloudClient::uploadFileContent and
QtMeshCloudClient::UploadTarget to locate code to exercise.
🪄 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: 0d6c7357-a283-415f-a140-0994b6133dfd
📒 Files selected for processing (6)
src/CloudUploadPlanner_test.cppsrc/QtMeshCloudClient.cppsrc/QtMeshCloudClient.hsrc/QtMeshCloudClient_test.cppsrc/mainwindow.cpptests/CMakeLists.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- src/CloudUploadPlanner_test.cpp
Treat a missing or non-array projects field as an error instead of silently presenting an empty project list in the upload picker. Co-authored-by: Cursor <cursoragent@cursor.com>
|
1 participant
Summary
Adds a first authenticated QtMesh Cloud upload flow from the editor.
Cloud > Upload Files..., prompting sign-in when needed and using the stored QtMesh Cloud session token.CloudUploadPlannerfor project slug generation, asset role inference, MIME lookup, and upload descriptor creation.Links #684.
Validation
cmake --build build_local --target QtMeshEditor -j2cmake --build build_local --target UnitTests -j2./build_local/bin/UnitTests --gtest_filter=CloudUploadPlanner.*Notes
The worktree still has an unrelated existing
src/dependencies/ogre-proceduralsubmodule marker that is not included in this PR.Summary by CodeRabbit
New Features
Tests