Skip to content

fix(desktop): stop duplicate authorized clients after restarts - #4687

Closed
Noojuno wants to merge 8 commits into
pingdotgg:mainfrom
Noojuno:fix/desktop-auth-session-reuse
Closed

Noojuno wants to merge 8 commits into
pingdotgg:mainfrom
Noojuno:fix/desktop-auth-session-reuse

Merge branch 'main' into fix/desktop-auth-session-reuse

cea8f2f
Select commit
Loading
Failed to load commit list.
MacroscopeApp / Macroscope - Effect Service Conventions succeeded Aug 7, 2026 in 8s

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

💭 Looking at the current state of the code to see if that storeError helper issue at line 71 of DesktopLocalEnvironmentAuthTokenStore.ts has been resolved in the latest version. I'm checking the conventions for error handling in the code. The storeError helper has been removed as expected, and the DesktopLocalEnvironmentAuthTokenStoreError follows the allowed pattern with an operation field that takes three values and derives a generic message. For the Effect.catch usage, I need to verify whether catchTags would be more appropriate for handling statically known tagged failures, or if catch is justified here since the entire error channel needs to be handled. The error wrapping here mirrors the existing SessionBootstrapError pattern, so I'm keeping the cause-only structure consistent rather than adding the httpBaseUrl context — it's a local 127.0.0.1 URL anyway, and the convention allows this when the adjacent error follows the same approach. Now I'm looking at how the service definitions are ordered in the token store module. I need to verify that ElectronSafeStorage exists with the right shape and check if fetchRemoteSessionState is available in the authorization module. The openStore function creates a new ElectronStore on each call wrapped in Effect.try, which isn't a violation but worth noting. Since ElectronSafeStorage isn't in the diff, it's pre-existing. I should also verify how the layer wiring in main.ts merges the DesktopLocalEnvironmentAuthTokenStore.layer. The prior fix resolved the issue, so I'm checking if there are any new violations. The convention about not adding large tests for mechanical refactors makes sense, but since the backend behavior actually changed here, focused tests are justified. Now I'm considering whether logging the error object with its cause in Effect.catch could leak sensitive data—the convention warns against copying raw payloads into logs, and in this case the error's cause is a filesystem or Electron error that might contain problematic details. Looking at the test setup, the DesktopLocalEnvironmentAuthTokenStoreError with its three operation literals and derived messages are explicitly allowed. The test imports and layer configurations using ["Service"] type indexing look correct, and the new test file's satisfies clause for ElectronSafeStorage is properly typed. Everything appears clean, so I'll respond with success and set the title to "Effect Service Conventions: All clear" with the summary matching. One final check on whether readToken in the test and Effect.fromResult raise any convention issues—they don't. The DesktopLocalEnvironmentAuthError schema now includes the token store error, which is fine. I'm confident this is clean.