Skip to content

refactor(desktop): Colocate pre-ready Electron service

MacroscopeApp / Macroscope - Effect Service Conventions failed Aug 2, 2026 in 1m 46s

Effect Service Conventions: 1 issue found

  • apps/desktop/src/shell/DesktopShellEnvironment.ts: ShellEnvironmentConfig accepts a FileSystem.FileSystem service instance as a plain field, hiding an owned Effect dependency from installShellEnvironment/installPosixEnvironment requirements instead of acquiring it via yield* FileSystem.FileSystem and providing it in make (as is already done for ChildProcessSpawner).

Previously flagged items are resolved: DesktopPreReadyPlatform.ts now owns the tag plus canonical make/layer with an inline Context.Service interface, and ElectronSafeStorage.ts exports a real make.

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.

Files examined at head a88e1a3: apps/desktop/src/app/DesktopPreReadyPlatform.ts, DesktopEarlyElectronStartup.ts, DesktopStatePaths.ts, DesktopApp.ts, DesktopEnvironment.ts, apps/desktop/src/electron/ElectronSafeStorage.ts, ElectronProtocol.ts, ElectronApp.ts, apps/desktop/src/shell/DesktopShellEnvironment.ts, apps/desktop/src/settings/DesktopAppSettings.ts, apps/desktop/src/main.ts, plus touched test files.

Checks performed: namespace subpath imports for Effect modules; canonical module order (imports, errors/schemas, Context.Service tag with inline interface, make, layer); exported make/layer naming; Foo["Service"] usage; no new ManagedRuntime.make, runPromise, or runPromiseExit; Layer.succeed occurrences; error classes using Schema.TaggedErrorClass with structural attributes, derived messages, and preserved cause; no compatibility re-export shims left behind.

Notes (not flagged): ElectronProtocol.layerSchemePrivileges is an additional descriptive layer alongside the canonical layer and is acceptable; ElectronSafeStorage.selectedStorageBackend intentionally models unavailability as Option.none() for advisory logging.


🔍 Agent activity — 1 tool call

💭 I'm checking the current state of the PR files to see if the previous issues were resolved. DesktopPreReadyPlatform.ts looks good now with the make/layer and inline Context.Service interface added, and ElectronSafeStorage's make is properly exported. Let me scan through the files to identify any remaining violations. The prior issues look resolved. Moving through the other files now—DesktopEarlyElectronStartup and DesktopStatePaths are straightforward helpers, so those are fine. ElectronProtocol added layerSchemePrivileges, so I need to verify the naming conventions align with other layers in that module. DesktopShellEnvironment added fileSystem to the config, which is properly acquired through the effect system in the make function, though I'm checking how FileSystem is imported. There's a process.getuid?.() call in installPosixEnvironment that's a module global, but it's not an Effect service so that's acceptable. Now I need to look at the ElectronProtocol export changes, the main.ts modifications, and DesktopApp.ts. There's a potential issue where resolveEarlyLinuxElectronOptionsFromProcess is called synchronously inside an Effect generator in DesktopApp, reading process env and filesystem directly rather than through the environment services—which could violate the dependency hiding rule, though it's at a documented boundary and arguably acceptable for a pure synchronous read. I should check the DesktopApp context to see if this warrants flagging. I'm checking the main.ts file for layer convention violations, particularly around how resolveEarlyLinuxElectronOptionsFromProcess() is being called within the startup Effect—it's reading from the filesystem and process environment synchronously, which might be circumventing the proper FileSystem and environment service abstractions that the conventions are meant to enforce. Looking at ElectronProtocol.ts next, I notice layerSchemePrivileges as an implementation-specific layer name in a service module that already exports a layer, which could be redundant or a naming convention issue. I should verify whether ElectronProtocol properly exports both make and layer, and check how ElectronSafeStorage.make uses its dependencies. ElectronProtocol has both layer and layerSchemePrivileges as named layers, which is fine—implementation-specific naming is allowed. But I'm noticing registerDesktopSchemePrivilegesSync is exported without any apparent usage, so I need to verify whether it's actually being called internally or if it's dead code. The configureElectronBeforeReady call to registerSchemesAsPrivileged was removed from main.ts, which is acceptable. In DesktopShellEnvironment, there's a test-only export of resolveDefaultLinuxDbusSessionBusAddress that duplicates logic already inlined in the production path—the helper exists only for tests, which is a minor convention issue but not a violation. I'm checking for any misuse of Layer.succeed, ManagedRuntime, or runPromise in the changed files, and there's nothing new there. DesktopEarlyElectronStartup imports pure helpers like fromLenientJson and namespace imports for Option and Schema, which are fine. DesktopStatePaths is a pure helper module with no service layer concerns. Now I'm looking at the HostProcessPlatform import to verify it follows the same conventions. The try/catch in selectedStorageBackend swallows errors into Option.none for advisory logging, which is borderline against the convention of modeling failures with typed errors in Effect. But since the function deliberately returns an optional type, treating unavailability as none is a reasonable design choice, so I won't flag it. Moving on to check DesktopShellEnvironment.ts for make and config threading. I'm refactoring how ShellEnvironmentConfig handles the FileSystem service—instead of having it acquired from the environment inside installPosixEnvironment, I'm now passing it directly as a config field to helper functions, which aligns with the convention that factories should accept service implementations as explicit dependencies rather than letting them surface implicitly in the Effect type. The issue is that this violates the module's own pattern: spawner is acquired via yield* and provided through Effect.provideService, so fileSystem should follow the same approach rather than being hidden in the config struct. This creates a type signature mismatch where installShellEnvironment only declares ChildProcessSpawner as a requirement while the FileSystem dependency is concealed, which the check should flag as a violation. I should target line 17 in the diff where the fileSystem field is added to ShellEnvironmentConfig. There's also a minor concern about resolveDefaultLinuxDbusSessionBusAddress being exported only for tests, though that's acceptable given the behavior change is covered by tests. The boundary layer code like DesktopPreReadyPlatform.make using Electron's imperative APIs is fine, but I need to examine whether resolveEarlyLinuxElectronOptionsFromProcess reading the filesystem synchronously during startup in DesktopApp.ts crosses into service construction territory where it shouldn't. Looking at the other changes: DesktopEarlyElectronStartup.ts is pure and uses Schema/Option correctly, and ElectronSafeStorage now requires HostProcessPlatform which surfaces as a layer requirement that gets provided in main.ts. The real question is whether FileSystem could be acquired directly inside installPosixEnvironment as an Effect, which would make the requirement flow through installShellEnvironment naturally without needing the make function to provide it explicitly. Actually, since installIntoProcess is typed as Effect.Effect<void> in the service shape, the make function needs to provide both the spawner and fileSystem via Effect.provideService. I'll write up the review comment now and mark this as a failure since there's a clear convention violation. The comment goes on line 17. Since this spans multiple changes—removing the field, adding a yield in the helper, and using provideService in make—I'll describe it rather than provide a diff. I should also verify this wasn't already flagged in prior runs; the earlier comments were on different components, so this is a new finding worth posting. Let me confirm the exact line numbers and submit.

1. modify_pr — One finding: a service instance is threaded through a config record instead of being acquired from the Effect environment.


Agent Credits: 83 credits