fix(server): cache project favicon resolution - #9080

1. browse_code — apps/server/src/project/ProjectFaviconResolver.ts

💭 I'm checking the code against Effect conventions—imports, service definition, and error handling all look consistent. The cache key uses a null-byte separator which isn't a convention issue, and the new tests are justified since the behavior actually changed. I'm double-checking the TestClock-based test and confirming the cache invalidation logic re-resolves correctly on the second call. Looking more closely at the resolvePath interface's error channel typing, and now examining whether importing TestClock as a named export from effect/testing conflicts with the namespace-import convention for Effect library modules.

2. git_tools — TestClock

3. browse_code — apps/server/src/project/ProjectFaviconResolver.test.ts

💭 I'm noting that this import style is a minor deviation from the repo's dominant convention of importing Effect modules as namespaces from subpaths, but since similar patterns exist elsewhere, it's not really flag-worthy as a repo-wide cleanup issue. I'm now looking more carefully at the caching design itself — the key encoding logic seems safe against edge cases like undefined or empty favicon paths, but I'm spotting a potential real bug: the cache lookup uses the raw cwd string directly, while resolution elsewhere normalizes the workspace root, meaning different cwd values for the same workspace could produce different cache keys. I'm also checking whether errors from stat failures carry the right unnormalized workspace root context, and whether the resolver's dependencies stay properly scoped in closures rather than leaking into module globals, which seems fine here. What concerns me more is whether Cache.makeWith in this Effect version requires a Scope, since if it does, Layer.effect wouldn't typecheck and they'd need Layer.scoped instead — I want to verify this against the actual Cache API. Since node_modules isn't available to check, I'll search the repo for other places using Cache to see how it's typically wired up.

4. git_tools — effect/Cache

5. git_tools — Cache.(make|makeWith|get|invalidate)

💭 I want to check RepositoryIdentityResolver.ts in the same directory to see how it structures its caching, so I can match its conventions for key composition and layer type.

6. browse_code — apps/server/src/project/RepositoryIdentityResolver.ts

💭 This new code follows the existing pattern closely, so it's conventional. I'm checking for issues around error handling and change discipline—tests were added appropriately, and the stat error omission looks fine since it's optional; I'm now considering whether the cache hit conversion to a failure path raises any concerns with the Effect-service conventions. Checking whether Cache.makeWith needs a Scope in this version—it doesn't, based on how RepositoryIdentityResolver uses it the same way. The resolvePath logic after invalidation and the null-handling case look fine, as do the module-level helper functions like faviconCacheKey. The type annotations and error channel types line up with the existing interface, and I'm now looking at how TestClock imports are styled across other files in the repo. The TestClock import convention is just a style preference, not a hard rule, so I'll leave that alone. There don't seem to be other call sites affected—only the two files are changed, and adding focused tests for the caching behavior change seems justified rather than excessive. Now I'm considering whether the second test case about cache invalidation after a favicon deletion is set up correctly. I don't see a real Effect-service-convention violation here; the per-layer cache instantiation is fine. I do notice a minor inaccuracy in a comment—it claims "12 well-known paths" but the actual count of candidate paths plus source files is 28, not 20—though this is a documentation nit rather than a convention issue, so it's not something I'd flag as a failure. Separately, I'm weighing a conflict between the instruction that a clean review should just say "All clear" and the requirement that my output conform to a JSON schema; I need to figure out how "defaults to failure" should be interpreted in that context. I'm doing one more pass checking resolvePath's error handling and cache key logic for real issues, but nothing stands out as a genuine convention violation, so I'm leaning toward a clean result within the required schema.