feat(mobile): themes with appearance override and custom import - #6617
feat(mobile): themes with appearance override and custom import#6617amanthanvi wants to merge 42 commits into
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 scanning through this large PR to understand the Effect service convention changes, focusing on the TypeScript files that define or use Effect services like the mobile preferences module with its Context.Service and Schema definitions, along with related schema and state files. Looking at the mobile-preferences changes, I see it's importing from the contracts package and using
Schema.is()to create private constants for type predicates — these aren't exported or wrapped in redundant functions, so they're fine. The syncedClientPreferences schema module uses namespace imports for DateTime, Option, and Schema, which is allowed. Now I'm checking the server ws.ts file where new RPC handlers are being added, and I notice it's instantiating an OrchestrationGetSnapshotError with a message and cause — I need to verify this follows the convention about deriving wrapper messages from stable structural attributes. Looking at the test files, I see ManagedRuntime being used in the orchestration engine tests, which is acceptable for test harnesses. The mobile layer has React hooks and atoms rather than Effect services, which makes sense as an application boundary. The preferences store is using pipe operations on atoms, which is consistent with the state management pattern. For the error handling, I'm checking whether new error definitions were added to the Effect services or if they're reusing existing patterns. The ws.ts file constructs errors with a message and cause field, which appears to be the established convention in that file. I need to verify the retry logic that's being added. The new RPC handlers are reusing the existing error classes with static message strings that are part of the error class structure, which aligns with the established pattern. Looking at the server ws.ts patch handler, the message field follows the same existing class schema. Now I'm checking the mobile client preferences file for any Effect or runtime concerns — it's using atoms and React hooks at the application boundary, along with setTimeout for timing. The mobile preferences state layer uses Promise.allSettled and Date operations, which is appropriate for React hooks rather than Effect services. The new atom in preferences.ts uses runtime.fn returning Effect.void and updates the preferences through the atom setter, which follows the conventions. On the server side, the ProjectionSnapshotQuery layer adds getSyncedClientPreferences to the service shape, but the PR doesn't consolidate the domain/Services and domain/Layers modules into a single canonical module as the conventions suggest. Looking at the new files in the diff, none of them define Context.Service or Layer directly—they're mostly mobile UI components, theme files, and a database migration. The migration uses Effect.gen with SqlClient, matching the existing pattern. I should check whether any new code is importing from "effect" using named imports to ensure they're following the consolidated import convention. I'm noticing there are no consolidated effect imports in the changed files, which suggests the refactoring is keeping side effects localized. Looking at the mobile synced-client-preferences model and the use-thread-outbox-drain hook, I see there's a potential issue wherereadPlanModeEnabled()is reading directly from a module-level global registry inside a React hook, which violates the principle of keeping dependencies explicit rather than hidden. However, checking the diff more carefully,appAtomRegistrywas already being imported and used in that file before these changes, so this isn't a new pattern being introduced. On the web side, I'm seeing similar module-level singleton controllers likesyncedPlanModeHydrationControllerbeing instantiated in React hooks files, which creates mutable state at the module level rather than within Effect services. Looking at the server-sidews.tshandler, it's using existing helper functions likeserverCommandIdanddispatchNormalizedCommand, and the error handling for missing projections looks correct without needing a cause. Scanning through the diff forLayer.succeedcalls, I only see them in test files where they're legitimately mocking services for testing purposes. The key thing I'm checking now is whether any Effect services are being instantiated by passing runtime instances as factory arguments, which would indicate improper service consumption patterns. The rule about treating instructions as authoritative suggests I could flag this, but adding a method to an existing service interface counts as consuming rather than creating, so it doesn't trigger the cleanup requirement. Besides, hoisting the whole ProjectionSnapshotQuery service would be a large unrelated refactor that the guidelines explicitly say not to demand. I'll skip the flag and look for a canonical example in the workspace instead. whether the mobile preferences file uses.runPromise— it doesn't. Now I'm searching through the changed files for any instances ofrunPromiseandManagedRuntime. Looking at the diff, none of these Effect service convention violations are newly introduced—the SettingsRouteScreen.tsx changes only affected the Legacy section. I'm checking a few key files to make sure: the new mobile plan-mode.ts file contains pure helpers with imports from the contracts and shared packages, thread-composer-send.ts is also pure, and I'm verifying the server-side code as well. I'm resolving the conflict between the instruction to output "All clear" and the JSON schema requirement—the intent is to mark this as a successful pass with no findings, so I'll structure the response as minimal JSON with the summary set to "All clear".