Stabilize preview browser surfaces, automation, and recording - #3565
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
💭 I'm checking the diff against Effect service conventions, specifically looking at how the code handles schema predicates. The changes include importing Schema from Effect and adding a predicate helper, which should follow the pattern of exporting direct schema predicates like
isFoo. Looking at the actual usage in the files, I see thatisPreviewTabIdis used inside a function that does additional work beyond just type checking, so that's not a violation. Similarly, inbrowserRecording.ts, the privateisBrowserRecordingOperationErrorpredicate is wrapped byisStartupWaitTimeout, which adds an extra condition checking the operation field, so that's also fine and not a redundant wrapper. Checking the imports — they're usingSchemaas a namespace from "effect/Schema", which aligns with the conventions. The new error classes likeBrowserRecordingRequiresVisibleTabErrorfollow the pattern of usingSchema.TaggedErrorClasswith proper field definitions, andBrowserRecordingOperationErrorextends the operation enum with "wait-startup" as an additional discriminator value. Now looking at therecordingStartupCancelledErrorhelper function — it's constructing aBrowserRecordingOperationErrorwith specific operation and tabId values, which is more than just a simple passthrough wrapper, so this should be acceptable under the conventions. I'm reconsidering whether browserRecording.ts even falls under the Effect Service Conventions scope since it's Promise-based rather than Effect-based. Looking at PreviewAutomationHosts.tsx now, I see a helperraisePreviewAutomationHostErrorthat just throws an error passed to it — it's not constructing the error, just rethrowing it, which seems like an odd pattern but doesn't quite match the convention about error-construction helpers. This looks more like a stylistic choice to make the function returnneveras an expression rather than a statement. Checking the contracts file and broker imports, the schema additions look fine, and the namespace imports from effect subpaths are all correct per the conventions. Looking more closely at the predicate convention,isBrowserRecordingOperationErroris a privateSchema.isconstant, but it's wrapped byisStartupWaitTimeoutwhich adds an extra condition checkingerror.operation === "wait-startup", so it's not redundant — the wrapper has different behavior even though the type guard narrows to the same type. Now examiningisPreviewTabIdin the broker to see if it follows the same pattern. helper is borderline acceptable since it provides reusable context like the default cause and gets used multiple times, plus browserRecording.ts isn't an Effect service anyway—it's Promise-based. The conventions primarily target Effect services, so strict error helper rules might not apply here. Given the PR is about preview tab targeting and recording features rather than service refactoring, I should focus on whether there are actual Effect service violations in the changed scope, and I'm not seeing concrete ones. I need to restructure the repo parameter as an object with a url property instead. The broker file is importing correctly with namespace subpaths. Looking atpreviewAutomation.tsin contracts, it's already importing from "effect" but that import was pre-existing, not part of the diff I'm reviewing. The convention about flagging consolidated named imports from "effect" in touched Effect service code might not apply here since the import line itself wasn't modified. Actually,previewAutomation.tsis a pure schema definitions file, not service code, so it wouldn't fall under that convention anyway. The PR is about tab targeting features, not a service refactor. The broker'smakestructure is unchanged and the only schema modification is addingtabIdExplicitto the request, which is fine. I'm not finding any concrete violations of the Effect service conventions here. Now I need to format the response properly — the schema requires JSON output with a state, title, and summary. Since there are no findings, I'll set the state to success and put "All clear" in the summary as requested, keeping the title concise to satisfy the schema requirements.
Agent Credits: 63 credits