fix(desktop): throttle hidden preview rendering - #7445
Effect Service Conventions: All clear
All clear
Details
Note
Your check run agent prompt is: .macroscope/check-run-agents/effect-service-conventions.md
More information about how Check Run Agents work can be found in our Help Center.
🔍 Agent activity — reasoning, no tool calls
💭 Good, the previous issue about fabricating an Error for PreviewOperationError seems resolved now that there's a proper PreviewMainWindowClosedError class. Now I want to check the rest of the changed code for convention violations, starting with the imports and surrounding regions in Manager.ts. I'm looking at the mutable closure state like frameCaptureWindowOpen and currentMainWindow inside makeNativeOperations — this is state rather than a hidden dependency, so the module-globals convention doesn't strictly apply, though the repo's preference for Refs makes me wonder if this should be refactored too. Now I'm moving on to check the new PreviewMainWindowClosedError class and the surrounding error handling area. Good, that's addressed with a dedicated tagged error now. Checking the rest against existing conventions: the operation discriminator matches the established PreviewOperationError pattern, and logging the error object as an annotation in the warning mirrors what's already done elsewhere in the file, so both look fine. The catch-all error handling is acceptable since the whole error channel is covered, and the new error class is properly exported and added to the PreviewManagerError union. I don't think any predicates are needed here. I should verify whether PreviewMainWindowClosedError needs to surface elsewhere, like in IPC handlers with exhaustive tag matching, by checking consumers of PreviewManagerError and startRecording's error handling. The DesktopWindow.ts change removing backgroundThrottling from webPreferences looks fine too. Now I want to look at the setMainWindow implementation for conventions around Fiber.join and mutable variables. These concerns are about correctness and concurrency rather than actual Effect service convention violations, which is outside the check's scope. The runFork usage in the closed callback is an existing allowed pattern for native callback boundaries, and since this is a genuine behavior change, adding tests is justified. I'm now verifying the test file's imports of Exit, Cause, Option, and Effect match expected usage. Imports look fine. Now I want to check if PreviewManagerError is consumed elsewhere, like in IPC error mapping with an exhaustive switch, so I should grep for PreviewWebviewNotInitializedError usages to see if any consumers need mechanical updates. The new error class placement after
makematches the file's existing layout, even though canonical order technically calls for errors before the tag and make. Since this is pre-existing structure rather than something introduced by the change, flagging it would be unrelated repo-wide cleanup rather than a real issue with this diff. No DI concerns either — no new service-instance parameters, no ManagedRuntime, no runPromise, andrunForkappears to be an existing runtime-safe helper. This looks clean overall, and the prior finding has been addressed, so I'll report success with a concise "All clear" summary in the required JSON format.
Agent Credits: 45 credits