fix(preview): resize hidden browser tabs reliably - #8469

💭 I'm reviewing the diff across the desktop, server, and web preview managers and contracts package for Effect service convention violations, checking import style and error type definitions like the resize conflict error. The error uses a tagged schema class with a static message, which seems fine for a pure validation error. I also want to verify weak-map closure state, predicate usage, and catchTag patterns, then look directly at the contracts file and the Manager.ts files to confirm import conventions and constructs like fnUntraced and SynchronizedRef.

1. browse_code — packages/contracts/src/preview.ts

2. browse_code — packages/contracts/src/preview.ts

💭 That consolidated "effect" import in the contracts file is pre-existing and unchanged, so I shouldn't flag it since it's not part of a touched service module. Moving on to check the desktop Manager.ts imports and new code.

3. browse_code — apps/desktop/src/preview/Manager.ts

💭 Imports look good. Now I want to check the new viewport-related functions added to Manager.ts around lines 2646-2900, making sure they follow existing conventions inside makeNativeOperations.

4. git_tools — Viewport

💭 I'm checking attemptPromise in Manager.ts, confirming it's an existing helper producing a generic error with operation context, then verifying the desktop service interface declares the new members inline consistent with existing style. Now I'm tracing the server's resize return type and its error channel to verify PreviewResizeConflictError construction lines up correctly. Next I want to check the web files — PreviewAutomationHosts.tsx and previewStateStore.ts — for any Promise-based Effect usage issues, so I'm grepping the changed web files for runPromise and ManagedRuntime references.

5. git_tools — runPromise|ManagedRuntime|Layer.succeed|["Service"]

6. browse_code — apps/server/src/preview/Manager.ts

💭 The server manager looks standard, so I'll check the resize section for error handling details.

7. browse_code — apps/server/src/preview/Manager.ts

💭 This all looks conventional. The preview IPC method and payload handling follow the standard pattern, so I want to check whether there's a duplicated schema or type definition for the viewport override on the web side. Scanning for stray lint suppressions like eslint-disable or @ts-ignore turns up nothing in the diff, and there don't appear to be any new catchIf/catchTag additions either. The viewport timeout error construction and the runBeforeDeadline helper look like plain promise-based logic rather than Effect services, which is fine, and the schema naming ("Automation" applied to a union used by both human and automation channels) is slightly misleading but not something flagged by the convention checks I'm running. I'm also checking that the lazy error factory in PreviewAutomationHosts is a legitimate closure for deferred construction rather than a disallowed mapError alias — it seems justified since the deadline helper needs it lazily. The attemptPromise usage in the desktop manager's applyViewportOverride matches existing patterns, so that's consistent too. Now I'm noting a minor duplication: the Manager interface inlines the viewport override type instead of reusing the non-exported PreviewViewportOverride type — not a listed convention violation, just redundant. I'm also scanning the web store and preview state files for Effect usage, checking whether new tests reflect genuine behavior changes rather than mechanical refactors, and confirming no stray Layer.succeed or ManagedRuntime additions crept into the touched web files.

8. git_tools — ^import|runPromise|ManagedRuntime

9. git_tools — ^import|from "effect"

💭 The IPC method correctly uses namespaced access. I'm checking whether ipc.ts's import from "effect" is a pre-existing pattern rather than something newly introduced, since the consolidated-import convention only applies to touched Effect service code, and this file is a contracts schema module, not a service. I don't spot any new eslint-disable or ts-expect-error suppressions in the diff, just as never casts in tests. The new PreviewResizeConflictError message is a static string despite having structured attributes, but that's not the same as storing an unstructured message as the only data, so it's not a real violation—just a stylistic choice compared to sibling errors that interpolate. I want to confirm the RPC error channel actually includes PreviewError so the new conflict error can propagate correctly, so I'm checking the RPC declaration around line 846 to see if it's scoped too narrowly.

10. browse_code — packages/contracts/src/rpc.ts

💭 That's fine. Now I want to double-check the desktop Manager code for similar issues, like WeakMap/WeakSet used as closure state inside makeNativeOperations, which should be acceptable since it's not a module-level global. I'll look at the relevant line ranges to be thorough.

11. browse_code — apps/desktop/src/preview/Manager.ts

12. browse_code — apps/desktop/src/preview/Manager.ts

💭 The rollback function swallowing errors is just error-handling policy, not a convention violation I need to flag. Nothing else breaks the enumerated Effect service conventions so far, and the web-side preview viewport functions look fine using plain promises in the React boundary. Now I'm checking the desktop preload script next.

13. git_tools — apps/web/src/components/preview/PreviewAutomationHosts.tsx 6866fd6..9425d6e

💭 I should check whether reusedExistingTab is now unused after removing that block, and similarly whether activeSnapshot's let declaration is still necessary if it's never reassigned. These are just lint cleanup concerns though, not related to the Effect service conventions.

14. git_tools — reusedExistingTab|activeSnapshot

💭 I'm checking whether everything conforms to Effect-service conventions, and the only remaining question is whether preview.ts's consolidated Schema import from "effect" counts as a violation since it touches Effect service code. That import is actually in a pre-existing schemas/contracts file, unchanged and legacy, so it shouldn't be flagged. The review comes up clean, so I need to respond with "All clear" — though I have to reconcile that with the requirement to call complete_check, so I'll set the title and summary fields to "All clear" and mark the outcome as success with no details.