Skip to content

Add user-local project icons - #4401

Open
colonelpanic8 wants to merge 10 commits into
pingdotgg:mainfrom
colonelpanic8:feat/project-icons
Open

Add user-local project icons#4401
colonelpanic8 wants to merge 10 commits into
pingdotgg:mainfrom
colonelpanic8:feat/project-icons

fix(web): revise favicons for normalized icon keys

6f052c8
Select commit
Loading
Failed to load commit list.
MacroscopeApp / Macroscope - Effect Service Conventions succeeded Jul 28, 2026 in 12s

Effect Service Conventions: No issues found

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 examining the diff to identify any Effect service convention violations, starting with the key modified files that either define or consume these services. I'll use the head commit hash. I'm checking the error classes like AssetProjectFaviconResolutionError, AssetProjectFaviconInspectionError, and AssetProjectFaviconNotFoundError to see if the new resolution error duplicates existing ones and whether they follow the established conventions used in services like RepositoryIdentityResolver and serverSettings. The error naming convention looks consistent—renaming from InspectionError to ResolutionError for the resolve operation makes sense semantically. The ProjectFaviconResolutionError uses Schema.Defect() for cause, which follows the existing pattern, and the operation discriminator with multiple values is allowed under the conventions. I'm checking whether the error types are being used appropriately across the asset access layer. and used in the codebase, so I need to verify whether it's a new addition to the contracts file or if it was already defined there. I'm checking the contracts asset errors to see if there's any redundancy in how the discriminators or error distinctions are defined. I'm checking whether AssetProjectFaviconResolutionError is actually new to this diff or if it was already defined in the contracts. Looking at the changes to assets.ts, I only see the revision field being added, so I need to verify where this error class actually comes from. Now I'm evaluating the actual convention violations in the changed scope. The ProjectFaviconResolver service now has both resolve and resolvePath methods, but having a convenience wrapper isn't a violation—that rule applies to error constructors, not service methods. The imports and service definitions look fine, and I'm checking whether the dependency acquisition in AssetAccess follows the rules, specifically how ServerSettingsService is being acquired. In the React components, the hooks and atoms are used appropriately without any hidden runtime calls in the domain services. Looking at error handling, ProjectFaviconResolutionError uses Effect.catchTags correctly, and the error structure with multiple operations and a generic message derived from the operation is the allowed pattern—no redundant discriminator issue here. Now I'm wondering if resolvePath in ProjectFaviconResolver is still actively used or if it's become dead code that only appears in tests. resolvePath is only used in tests now, not in production code, so it's really just a test convenience method. The conventions don't explicitly prohibit helper methods on service interfaces, so while it's borderline, I wouldn't call it a clear violation. Let me look more carefully at the conventions to see if there's an actual issue here. The ProjectFaviconResolver interface has both methods taking the same options and returning overlapping data, which is a mild design smell but not a listed violation. The ServerSettingsService usage looks fine—it acquires from the environment properly, and the test seam pattern with Effect.provideService is acceptable. The resolve method returning { path, source } | null is good. Now I'm checking whether the code properly uses the service interface reference pattern like ProjectFaviconResolver["Service"]["resolve"]. The imports and service structure all follow Effect conventions properly—namespace imports for Effect modules, service tags with inline interfaces, canonical make and layer exports, dependencies resolved through yield*, and no hidden runtimes in domain services. Error classes are also correctly structured with struct. I'm double-checking the resolvePath convenience method on ProjectFaviconResolver—it's not a convention violation since it's a genuine utility used by tests, not just a wrapper forcing Layer.effect. Both service modules have proper make implementations, so that's solid. The conventions don't flag helper methods like this, only error-specific helpers, so there's nothing to flag here. Now I need to reconcile the output format: the instructions say to respond with "All clear" when there are no findings, but the JSON schema requires a specific structure. The check defaults to failure when violations exist, but since I'm finding no clear violations, I should return success with "All clear" as the content. I need to resolve the conflict between the JSON schema requirement and the instruction to output "All clear" on a single line. I'll structure the response as JSON with the summary field containing "All clear" and set the state to success with an appropriate title indicating no violations were found.