[5/5] fix(mobile): enable environments from the composer - #5599
DominicVonk wants to merge 7 commits into
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
💭 Now I'm checking whether the previous issues have been resolved, then moving through the codebase to verify other conventions. I need to validate that the
Semaphoreimport fromeffect/Semaphoreis actually available in this repo by looking at other usages, and I should also review thedisabledEnvironmentIdsmutable module to ensure it follows the expected patterns. The persistence error helper with its message pattern is pre-existing, so adding new operation literals doesn't introduce new violations. ConnectionActivationStore fits fine in the persistence module alongside ConnectionTargetStore. I'm checking the registry's setEnabled method and noting the mutable Set for disabledEnvironmentIds, though that's not a listed convention issue. Now moving on to review the desktop changes. Looking at the code changes across the codebase, I'm checking whether the new patterns follow existing conventions. The mobile storage implementation uses the established Context.add pattern, the client runtime's catalog atoms are fine, and I'm examining the newsetEnabledmethod in the registry to ensure it aligns with the rest of the codebase. I'm checking whether thecatchTagcalls in the registry file are actually part of the changed hunks or if they're pre-existing lines that shouldn't be flagged as violations. Looking at the diff, thecatchTagline is unchanged context—it was already there before the PR. The rule says to report violations "introduced or retained," but the guidance also warns against flagging unrelated legacy code. Since this isn't something the PR added, I'll skip it. The other cases look fine:Effect.catchwith full error handling is allowed, the mutable Set closure isn't a module global, and the storage.ts issues appear resolved. For the new items, the UI onClick without void is minor and not an Effect convention issue. The imports are correct—EnvironmentIdas a named import and Effect subpaths like Semaphore are already used elsewhere in the repo. TheConnectionPersistenceErroroperation literals follow the existing discriminator pattern. ThefollowNetworkStatushelper placed after the layer definition doesn't violate the canonical ordering rules since those elements themselves are still in order. Checking the dependency flow:followNetworkStatusneeds Connectivity and ConnectionWakeups, which the registry already has and passes through viaEffect.provideServiceintoEnvironmentSupervisor.make—this is pre-existing and consistent. The layer uses ConnectionBlockedError doesn't have a cause field, so I can't preserve the cause chain without modifying the error class itself — not worth flagging as a finding. Let me verify the mobile storage layer fixes are correct now — the labels do match properly. I'm also checking the activation store in the connection storage file, which uses a pre-existing helper pattern that extracts the message from ConnectionTransientError. Now I need to look at the connections state file to see if there are any remaining convention violations.
Agent Credits: 210 credits