fix: improve project favicon detection for monorepos

[ponbac]

The XXL tag seems a little weird for this PR? It is a net +288 LoC, excluding test files.

What Changed

Improve project favicon detection without introducing a broad recursive search.
Closes #1020.

Rules, in order:

• Check well-known favicon paths in the requested project root.
  Example: /repo/favicon.svg or /repo/public/favicon.ico.
• If none exist, look for <link rel="icon" ... href="..."> tags or object-style { href, rel: "icon" } declarations in a small set of common source files, then resolve that target.
  Example: /repo/index.html points to /brand/logo.svg, so the route first tries /repo/public/brand/logo.svg and then /repo/brand/logo.svg.
• If still nothing is found, repeat the same checks for first-level children of apps/ and packages/, plus other top-level directories in the requested root, using the same non-git directory ignore rules as workspaceEntries.
  Example: /repo/apps/web/public/favicon.png or /repo/frontend/public/favicon.png is found when the cwd is /repo.
• Prefer a root match over nested workspace matches.
  Example: /repo/favicon.svg wins over /repo/apps/web/public/favicon.ico.
• Fall back to the existing generated SVG when nothing is found.

I also extracted the shared gitignore probing into apps/server/src/gitIgnore.ts so both workspaceEntries and projectFaviconRoute use the same chunked git check-ignore --no-index -z --stdin filtering. I also extracted the shared workspace directory ignore policy into apps/server/src/workspaceIgnore.ts, so projectFaviconRoute now uses the same non-git skip rules as workspaceEntries when scanning nested roots instead of keeping a separate ignore list.

Why

The current behavior misses common monorepo layouts where the favicon lives under an app directory instead of the repo root.

There was already an earlier PR for this in the upstream repo: #690. That version was larger and messier. This keeps the rules simple on purpose, leaving more extensive changes to project icons for trusted maintainers.

UI Changes

Not applicable.

Checklist

• This PR is small and focused
• I explained what changed and why
• I included before/after screenshots for any UI changes
• I included a video for animation/interaction changes

Note

Improve project favicon detection to support monorepos with gitignore filtering

• Rewrites ProjectFaviconResolver.resolvePath to search nested apps/, packages/, and top-level child directories when no root favicon is found, enabling monorepo support.
• Adds gitignore-aware filtering via new gitIgnore.ts utilities (filterGitIgnoredPaths, isInsideGitWorkTree) that stream candidates to git check-ignore in 256 KiB chunks.
• Introduces workspaceIgnore.ts to skip well-known directories (.git, node_modules, .next, dist, etc.) during directory traversal.
• Adds workspaceEntries.ts for a cacheable, git-backed or filesystem-fallback workspace index with fuzzy-ranked search.
• Fixes a missing FileSystem service injection in the static file handler in wsServer.ts that previously caused runtime errors.
• Behavioral Change: resolver now fails open on filesystem or git errors (treats unreadable paths as misses) rather than throwing.

📊 Macroscope summarized c01d2ee. 6 files reviewed, 4 issues evaluated, 1 issue filtered, 2 comments posted

🗂️ Filtered Issues

apps/server/src/workspaceEntries.ts — 1 comment posted, 3 evaluated, 1 filtered

• line 427: Race condition in clearWorkspaceIndexCache: deleting from inFlightWorkspaceIndexBuilds does not cancel the in-flight promise. When that promise completes, its .then() handler will call workspaceIndexCache.set(cwd, next), re-populating the cache with potentially stale data after it was intentionally cleared. This defeats the purpose of clearing the cache. [ Already posted ]

>

---

Note

Medium Risk
Modifies server-side favicon resolution and adds git-driven ignore filtering/search across workspace roots, which could affect file discovery performance and edge cases around path/permission handling. Git/process failures are handled fail-open, reducing risk of hiding files but still changing behavior.

Overview
Project favicon resolution is expanded to better support monorepos. The resolver now checks well-known favicon paths and icon metadata in common source files in the requested root, then repeats the same checks across first-level apps/, packages/, and other top-level directories (skipping ignored workspace dirs), preferring root matches before falling back to the generated SVG.

Gitignore-aware filtering is added and shared. New gitIgnore.ts provides isInsideGitWorkTree and chunked filterGitIgnoredPaths (git check-ignore --no-index -z --stdin) with fail-open behavior; both the favicon resolver and the new workspaceEntries.ts use this to avoid considering gitignored candidates.

Testing and wiring updates. New unit tests cover gitignore chunking/fail-open and expanded favicon route scenarios (nested roots, unreadable files, gitignored root/app/source cases), and wsServer.ts now provides FileSystem to the static-file handler Effect pipeline.

^{Written by Cursor Bugbot for commit 7404ccc. This will update automatically on new commits. Configure here.}

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ff59c0e1-1c65-4d1d-85b1-5fb6ba5d773b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a4a8d330eb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[eddieajau]

My 2c

getIgnore.ts should be separated into individual commands for isInsideGitWorkTree, filterGitIgnoredPaths. You've done the right think pulling it out, but it could go further.

I would add unit tests for this file rather than relying on integration tests.

Since this is a new file, I would also add some solid docblocks to provide good human and AI context for the decisions being made. Hopefully the reviewers will pick that up and think it's a good idea for their code standards.

[ponbac]

My 2c

getIgnore.ts should be separated into individual commands for isInsideGitWorkTree, filterGitIgnoredPaths. You've done the right think pulling it out, but it could go further.

I would add unit tests for this file rather than relying on integration tests.

Since this is a new file, I would also add some solid docblocks to provide good human and AI context for the decisions being made. Hopefully the reviewers will pick that up and think it's a good idea for their code standards.

Not sure about separating them right now, but I also feel like isInsideGitWorkTree() should live somewhere else. It does not really have anything to do with .gitignore.

I agree with the unit tests, added now.

Also added an explanatory comment to the filterGitIgnoredPaths(). I should maybe add additional ones, but I am not sure what is preferred here.

[macroscopeapp]

🟢 Low src/workspaceEntries.ts:336

Line 336 skips nested directories with ignored names by checking isPathInIgnoredWorkspaceDirectory(dirent.name) with only the directory name. This incorrectly excludes paths like src/node_modules because the check passes only "node_modules", and the function looks at the first segment of the input path. However, src/node_modules should be allowed since only the root-level directory is meant to be ignored. The filesystem fallback incorrectly filters these nested directories while buildWorkspaceIndexFromGit correctly includes them (it checks full paths). Consider using the full relativePath instead of dirent.name for the directory ignore check.

-        if (dirent.isDirectory() && isPathInIgnoredWorkspaceDirectory(dirent.name)) {
+        if (dirent.isDirectory() && isPathInIgnoredWorkspaceDirectory(relativePath)) {

🤖 Copy this AI Prompt to have your agent fix this:

In file apps/server/src/workspaceEntries.ts around lines 336-337:

Line 336 skips nested directories with ignored names by checking `isPathInIgnoredWorkspaceDirectory(dirent.name)` with only the directory name. This incorrectly excludes paths like `src/node_modules` because the check passes only `"node_modules"`, and the function looks at the first segment of the input path. However, `src/node_modules` should be allowed since only the root-level directory is meant to be ignored. The filesystem fallback incorrectly filters these nested directories while `buildWorkspaceIndexFromGit` correctly includes them (it checks full paths). Consider using the full `relativePath` instead of `dirent.name` for the directory ignore check.

Evidence trail:
apps/server/src/workspaceEntries.ts lines 336 (checks `dirent.name` only), 343-345 (constructs `relativePath` after the early skip), 248 (git version uses full path), 256 (git version uses full directoryPath). apps/server/src/workspaceIgnore.ts lines 13-21 (function checks first segment only). Commit: REVIEWED_COMMIT

[macroscopeapp]

🟢 Low src/workspaceEntries.ts:411

The cache eviction loop uses !oldestKey to check for undefined, but !"" evaluates to true, so if cwd is an empty string the loop breaks early instead of deleting the entry. This allows the cache to exceed WORKSPACE_CACHE_MAX_KEYS. Consider using oldestKey === undefined instead.

Suggested change

	while (workspaceIndexCache.size > WORKSPACE_CACHE_MAX_KEYS) {
	const oldestKey = workspaceIndexCache.keys().next().value;
	if (!oldestKey) break;
	if (oldestKey === undefined) break;

🤖 Copy this AI Prompt to have your agent fix this:

In file apps/server/src/workspaceEntries.ts around lines 411-413:

The cache eviction loop uses `!oldestKey` to check for undefined, but `!""` evaluates to `true`, so if `cwd` is an empty string the loop breaks early instead of deleting the entry. This allows the cache to exceed `WORKSPACE_CACHE_MAX_KEYS`. Consider using `oldestKey === undefined` instead.

Evidence trail:
apps/server/src/workspaceEntries.ts lines 411-415 at REVIEWED_COMMIT - shows the cache eviction loop using `if (!oldestKey) break;` which would break on empty string since `!""` === `true`. The loop should use `oldestKey === undefined` to correctly detect when the iterator is exhausted.

[macroscopeapp]

🔴 Critical Layers/ProjectFaviconResolver.ts:76

Errors from PlatformError.PlatformError are not caught by Effect.catchTag("PlatformError", ...) because the union type has concrete tags like "SystemError" and "BadArgument", not "PlatformError". When file system operations fail, the error propagates instead of returning Option.none(). Consider using Effect.catchAll or catching the specific error tags.

-  return effect.pipe(
-    Effect.map(Option.some),
-    Effect.catchTag("PlatformError", () => Effect.succeed(Option.none<A>())),
-  );
+  return effect.pipe(
+    Effect.map(Option.some),
+    Effect.catchAll(() => Effect.succeed(Option.none<A>())),
+  );

🤖 Copy this AI Prompt to have your agent fix this:

In file apps/server/src/project/Layers/ProjectFaviconResolver.ts around lines 76-83:

Errors from `PlatformError.PlatformError` are not caught by `Effect.catchTag("PlatformError", ...)` because the union type has concrete tags like `"SystemError"` and `"BadArgument"`, not `"PlatformError"`. When file system operations fail, the error propagates instead of returning `Option.none()`. Consider using `Effect.catchAll` or catching the specific error tags.

Evidence trail:
1. apps/server/src/project/Layers/ProjectFaviconResolver.ts lines 72-78 (REVIEWED_COMMIT) - shows `platformErrorToNone` function using `Effect.catchTag("PlatformError", ...)`
2. https://github.com/Effect-TS/effect/blob/main/packages/platform/src/Error.ts - shows `PlatformError = BadArgument | SystemError` union type, where `BadArgument` has `_tag: "BadArgument"` and `SystemError` has `_tag: "SystemError"` (neither has `_tag: "PlatformError"`)
3. package.json shows effect version 4.0.0-beta.45