improve: desktop recording and editor responsiveness - #2273
Conversation
![superagent-security[bot]](https://avatars.githubusercontent.com/in/3287076?s=60&v=4){kind=small}
| app.state::<MainWindowReadyState>().set_ready(true); | ||
| tracing::info!("Main window frontend ready"); | ||
| #[cfg(debug_assertions)] | ||
| stop_editor_benchmark::run(app.clone()); |
There was a problem hiding this comment.
P3: Debug application startup can automatically start and stop a real Studio recording
Debug startup can invoke a benchmark that starts and stops a real recording and writes artifacts.
Move benchmark execution out of app startup into an explicit command with a strong opt-in.
AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.
<file name="apps/desktop/src-tauri/src/lib.rs">
<violation number="1" location="apps/desktop/src-tauri/src/lib.rs:7318">
<priority>P3</priority>
<title>Debug application startup can automatically start and stop a real Studio recording</title>
<evidence>The new debug-only startup path invokes stop_editor_benchmark::run(app.clone()). When CAP_STOP_EDITOR_BENCHMARK_OUTPUT is set, that routine waits, calls recording::start_recording, sleeps for a configurable interval, stops the recording, and writes benchmark artifacts. This creates an external recording and filesystem side effects merely by launching the debug application with the environment enabled.</evidence>
<recommendation>Do not launch the benchmark from the normal application-ready lifecycle. Run it through an explicit benchmark command or dedicated harness, and require an additional unmistakable opt-in/confirmation before starting capture and writing artifacts.</recommendation>
</violation>
</file>
|
hey @greptileai, please re-review the PR |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| Effect.catchAll((error) => | ||
| Effect.logWarning( | ||
| "Published reupload cache invalidation failed", | ||
| { | ||
| videoId, | ||
| error, | ||
| }, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
The replacement commits before CloudFront invalidation runs, and any invalidation failure is reduced to a warning. If CloudFront rejects or times out the request, the endpoint still reports success while the existing share URL can continue serving the cached pre-edit video until it expires. Make invalidation failure actionable or persist it for retry instead of reporting the reupload as fully successful.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/app/api/upload/[...route]/multipart.ts
Line: 635-643
Comment:
**Cache invalidation failures succeed**
The replacement commits before CloudFront invalidation runs, and any invalidation failure is reduced to a warning. If CloudFront rejects or times out the request, the endpoint still reports success while the existing share URL can continue serving the cached pre-edit video until it expires. Make invalidation failure actionable or persist it for retry instead of reporting the reupload as fully successful.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| yield* db.use((db) => | ||
| db.transaction(() => | ||
| Promise.all([ | ||
| db | ||
| .update(Db.videos) | ||
| .set({ | ||
| duration: updateIfDefined( | ||
| body.durationInSecs, | ||
| Db.videos.duration, | ||
| ), | ||
| width: updateIfDefined(body.width, Db.videos.width), | ||
| height: updateIfDefined(body.height, Db.videos.height), | ||
| fps: updateIfDefined(body.fps, Db.videos.fps), | ||
| }) | ||
| .where( | ||
| and( | ||
| eq(Db.videos.id, Video.VideoId.make(videoId)), | ||
| eq(Db.videos.ownerId, user.id), | ||
| ), | ||
| db.transaction(async (tx) => { | ||
| const replacement = replacesVideo | ||
| ? await prepareDesktopReupload(tx, video) | ||
| : {}; |
There was a problem hiding this comment.
The multipart object is overwritten before the replacement transaction validates and locks the recording's storage identity. If the storage changes concurrently, prepareDesktopReupload throws after result.mp4 has already been replaced, while the transaction rolls back its source update and upload-row deletion. This leaves completed replacement bytes paired with stale database and upload state, and the desktop reupload fails after transferring the entire file. Validate and retire the target before replacing the object, or clean up this post-completion failure.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/app/api/upload/[...route]/multipart.ts
Line: 601-605
Comment:
**Replacement precedes storage validation**
The multipart object is overwritten before the replacement transaction validates and locks the recording's storage identity. If the storage changes concurrently, `prepareDesktopReupload` throws after `result.mp4` has already been replaced, while the transaction rolls back its source update and upload-row deletion. This leaves completed replacement bytes paired with stale database and upload state, and the desktop reupload fails after transferring the entire file. Validate and retire the target before replacing the object, or clean up this post-completion failure.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.1 participant
Stopping a Studio recording opens the preparing editor in both Tauri and GPUI while finalization continues. The timeline shows the confirmed playable portion, preview and audio become available progressively, and successful publication transfers the existing playback session into the ordinary editor. Cancellation, stale frames, recovery locks and empty-output muxer shutdown retain explicit ownership and regression coverage.
Also includes crop alignment and playhead improvements, recording replacement/upload invalidation fixes, the Windows hidden-window frame fix, development startup/cache improvements, and the benchmark and regression harnesses for these changes.
Completed verification on the final source:
Testing was stopped after the Windows sandbox reached its disk-space reserve. The final Windows protocol tests, Tauri, muxer, integration, explicit GPU, canonical, hardware and remaining strict checks are incomplete. Existing ignored tests remain documented. Strict GPUI Clippy and Linux export test Clippy retain verified unchanged baseline findings; Linux Tauri static-library packaging also exhausted disk space, while its unchanged main source passed all six tests through a direct native build.
This is not an all-tests-passed or packaged-release sign-off. Manual application verification, including audible continuity during preparation and the editor handoff, remains outstanding. See the benchmark and verification report for exact scopes, historical comparisons and limitations.
The PR is not yet safe to merge because successful existing-link reuploads can serve stale cached media, and a concurrent storage change can leave replacement bytes and database upload state inconsistent.
Findings
Fix with agent prompt
Summary
Reviews (1) · Last reviewed commit: "improve: desktop recording and editor re..."