Skip to content

fix(server): favicon resolution no longer pins the event loop - #5538

Merged
t3dotgg merged 1 commit into
pingdotgg:mainfrom
murenovich:fix/favicon-object-regex-backtracking
Aug 9, 2026
Merged

fix(server): favicon resolution no longer pins the event loop#5538
t3dotgg merged 1 commit into
pingdotgg:mainfrom
murenovich:fix/favicon-object-regex-backtracking

Conversation

@murenovich

@murenovich murenovich commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

What Changed

LINK_ICON_OBJ_RE is gone. Object icon metadata is now found by scanning brace-free runs instead of by one combined pattern:

-const LINK_ICON_OBJ_RE =
-  /(?=[^}]*\brel\s*:\s*["'](?:icon|shortcut icon)["'])(?=[^}]*\bhref\s*:\s*["']([^"'?]+))[^}]*/i;
+const ICON_REL_RE = /\brel\s*:\s*["'](?:icon|shortcut icon)["']/i;
+const ICON_HREF_RE = /\bhref\s*:\s*["']([^"'?]+)/i;

 function extractIconHref(source: string): string | null {
   const htmlMatch = source.match(LINK_ICON_HTML_RE);
   if (htmlMatch?.[1]) return htmlMatch[1];
-  const objMatch = source.match(LINK_ICON_OBJ_RE);
-  if (objMatch?.[1]) return objMatch[1];
+  for (const run of source.split("}")) {
+    if (!ICON_REL_RE.test(run)) continue;
+    const hrefMatch = run.match(ICON_HREF_RE);
+    if (hrefMatch?.[1]) return hrefMatch[1];
+  }
   return null;
 }

Five tests come with it. The object branch had no coverage at all — every existing case exercises the <link> branch — so this adds both key orders, a nested-object case, a no-href-then-valid case, and a large brace-sparse source that hangs the suite without the fix.

LINK_ICON_HTML_RE is untouched. It is already anchored on <link\b, so it was never part of the problem.

Why

Fixes #5537.

The old pattern began with a lookahead and no literal anchor, so the engine retried at every offset and rescanned forward with [^}]* from each one. On a brace-sparse file that is quadratic.

That turns one file into a full environment outage. A project with no icon and a large index.html lacking <link rel="icon"> — the shape of a generated single-file build — made resolvePath spin for minutes on the server's only thread. Every endpoint stopped answering, including /.well-known/t3/environment; the desktop client timed out at 10s, respawned the server, and it re-scanned and re-wedged. The environment never recovered on its own.

before after
1.6 MB icon source, no icon metadata killed at 30s, never completed 2 ms
200 KB ~4s <1 ms

Why runs rather than an anchored pattern

Anchoring the existing pattern on { is the obvious fix and it is not equivalent. It breaks two cases the current code handles:

source current anchored \{…\} this PR
{ attributes: {}, rel: "icon", href: "/x.svg" } /x.svg null /x.svg
[{ rel: "icon" }, { rel: "icon", href: "/x.svg" }] /x.svg null /x.svg

The old pattern accepted any position from which both rel and href were visible before the next } — which is exactly "both live in the same brace-free run". Walking those runs reproduces that rule directly: a run holding rel but no href falls through to the next candidate, and metadata beside a nested object still resolves because the run boundary sits at }, not at the enclosing object.

I checked this against the old pattern across both key orders, shortcut icon, query stripping, rel: "stylesheet", a TanStack head() block, and the two rows above: same result on every one.

UI Changes

None.

Verification

  • vp test run src/project/ProjectFaviconResolver.test.ts — 17 passed (12 existing + 5 new), 445 ms.
  • Reverting only the source change and keeping the new tests: the run was still going at 100s and had to be killed, confirming the regression test actually catches this.
  • vp lint on both changed files: clean.
  • tsgo --noEmit for apps/server: exit 0, no diagnostics in the changed files.

Branched off main @ e4abc31f.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes (N/A, no UI change)
  • I included a video for animation/interaction changes (N/A)

Model: Claude Opus 5 · Harness: Claude Code


Note

Medium Risk
Touches server-side favicon discovery on every workspace scan; behavior is intended to be equivalent for object metadata but the parsing path changed, so edge-case regressions are possible despite new tests.

Overview
Fixes event-loop wedging when ProjectFaviconResolver scans large project sources that have no icon metadata (e.g. generated single-file index.html).

Object-literal icon detection no longer uses the unanchored LINK_ICON_OBJ_RE regex (which retried at every offset and could run for minutes). extractIconHref now splits the source on } and, within each brace-free run, looks for rel: "icon" / shortcut icon and a matching href—same semantics as before for TanStack-style head() blocks, arbitrary key order, nested objects, and “icon without href then valid entry” cases. HTML <link rel="icon"> matching is unchanged.

Adds five tests covering object metadata cases plus a large-file timing guard (< 5s, expects null).

Reviewed by Cursor Bugbot for commit 6ba92f5. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Fix extractIconHref to resolve object-literal favicons without pinning the event loop

  • Replaces the single unanchored LINK_ICON_OBJ_RE regex with a segment-based scan in ProjectFaviconResolver.ts: the source is split on } into brace-free runs, each checked for rel: "icon" then href.
  • Fixes correctness issues where href before rel, extra nested properties, or rel-only segments caused resolution to fail.
  • Behavioral Change: scanning large sources without icon metadata now runs in linear time instead of quadratic time, preventing the event loop from stalling.

Macroscope summarized 6ba92f5.

`LINK_ICON_OBJ_RE` was unanchored, so it restarted at every offset in an
icon source file and rescanned forward from each one. A project with no
icon file and a large `index.html` that has no `<link rel="icon">` made
favicon resolution spin for minutes on the server's only thread, so every
connection stopped being answered and the desktop client dropped into a
permanent reconnect loop. Measured on a 1.6 MB generated `index.html`:
200 KB already cost ~4s, and the full file never completed.

That pattern accepted any position from which both `rel` and `href` were
visible before the next `}`, which is exactly "both live in the same
brace-free run". Walking those runs directly gives the same answers in
linear time, and keeps working where a single anchored pattern would not:
runs holding `rel` but no `href` fall through to the next candidate, and
metadata sitting beside a nested object still resolves. The same 1.6 MB
file now finishes in 2 ms.

The object branch had no test coverage, so this adds both key orders, a
nested-object case, a no-href-then-valid case, and a large brace-sparse
source that hangs without the fix.

Model: Claude Opus 5 · Harness: Claude Code

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

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 Plus

Run ID: a858e088-969f-4e69-b585-d08691daad3e

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

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

Comment @coderabbitai help to get the list of available commands.

@macroscopeapp

macroscopeapp Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved 6ba92f5

Straightforward performance fix replacing a quadratic regex pattern with a linear split-and-scan approach for favicon resolution. The change is isolated, well-documented, and includes comprehensive test coverage including a performance regression test.

You can customize Macroscope's approvability policy. Learn more.

@t3dotgg
t3dotgg enabled auto-merge (squash) August 9, 2026 21:08
@github-actions github-actions Bot added the size:S 10-29 changed lines (additions + deletions). label Aug 9, 2026
@t3dotgg
t3dotgg merged commit deb901d into pingdotgg:main Aug 9, 2026
18 of 20 checks passed
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Aug 9, 2026
## What's Changed
* fix(server): favicon resolution no longer pins the event loop by @murenovich in pingdotgg/t3code#5538
* fix(shared): bound the file-link label so bracket runs stop rescanning by @tsouth89 in pingdotgg/t3code#5782

## New Contributors
* @murenovich made their first contribution in pingdotgg/t3code#5538
* @tsouth89 made their first contribution in pingdotgg/t3code#5782

**Full Changelog**: pingdotgg/t3code@v0.0.33-nightly.20260809.1045...v0.0.33-nightly.20260809.1047

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.33-nightly.20260809.1047
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Aug 10, 2026
## What's Changed
* fix(mobile): reconnects no longer shift the thread list by @t3dotgg in pingdotgg/t3code#5372
* feat(web): drag pinned threads into your own order by @t3dotgg in pingdotgg/t3code#5581
* chore(ci): vouch StiensWout by @t3-code[bot] in pingdotgg/t3code#5637
* feat(desktop): remember recently used sites in the Browser panel by @chrisdeeming in pingdotgg/t3code#5270
* chore: vouch chrisdeeming by @t3-code[bot] in pingdotgg/t3code#5641
* feat(web): make sidebar artwork theme-aware by @maria-rcks in pingdotgg/t3code#5636
* fix(web): reconnect the composer seam for remote non-Git projects by @caezium in pingdotgg/t3code#5633
* fix(web): show Stop button while input is pending by @ipanasenko in pingdotgg/t3code#5554
* feat(web): fold plan mode and token-by-token output into Legacy features by @t3dotgg in pingdotgg/t3code#5664
* feat: sidebar v2 is now the default sidebar by @t3dotgg in pingdotgg/t3code#5672
* fix(server): stop PR status lookups amplifying GitHub rate limits by @t3dotgg in pingdotgg/t3code#5673
* fix(web): delay transient reconnect warnings by @t3-code[bot] in pingdotgg/t3code#5670
* fix(web): inherit terminal size in simple typography by @chrisdeeming in pingdotgg/t3code#5628
* fix(server): stop the reaper from silently killing live background subagents by @t3dotgg in pingdotgg/t3code#5677
* fix(desktop): zoom shortcuts no longer die when the preview browser has focus by @t3dotgg in pingdotgg/t3code#5691
* feat(mobile): one sheet for model and thread settings by @t3dotgg in pingdotgg/t3code#5625
* feat(usage): usage page reading provider transcripts across environments by @t3dotgg in pingdotgg/t3code#5684
* fix(web): usage chart no longer makes Claude look like the bigger spender by @t3dotgg in pingdotgg/t3code#5697
* fix(web): persist diff view mode by @leorivastech in pingdotgg/t3code#5731
* feat(web): show how many subagents are running at a glance by @t3dotgg in pingdotgg/t3code#5745
* fix(web): add missing cursor-pointer styling to dropdowns and interactive buttons by @naMqe-h in pingdotgg/t3code#5716
* fix(server): stop Claude resume handshakes from completing turns that never ran by @gfsaaser24 in pingdotgg/t3code#5710
* chore: vouch gfsaaser24 by @t3dotgg in pingdotgg/t3code#5761
* chore: vouch saphid by @t3dotgg in pingdotgg/t3code#5763
* fix(server): stop Codex threads with queued follow-ups by @t3dotgg in pingdotgg/t3code#5762
* fix(web): usage page loses the cost quality panel, gains a back button by @t3dotgg in pingdotgg/t3code#5756
* feat(server): agents can now open the images you paste into chat by @t3dotgg in pingdotgg/t3code#5757
* fix(web): pinned reorder no longer reshuffles while writes land by @t3dotgg in pingdotgg/t3code#5767
* feat(web): overhaul project settings into a real settings page by @t3dotgg in pingdotgg/t3code#5768
* fix(web): usage totals no longer jump while devices report in by @t3dotgg in pingdotgg/t3code#5772
* fix(server): settle no longer leaves monitors and dev servers running by @t3dotgg in pingdotgg/t3code#5774
* feat: pick worktree or current checkout per project by @t3dotgg in pingdotgg/t3code#5766
* fix(web): sidebar rows show the branch again, not a truncated plan step by @t3dotgg in pingdotgg/t3code#5776
* feat(server): vp run migrate-dev-db seeds worktree dev dbs with real data by @t3dotgg in pingdotgg/t3code#5773
* feat(web): keep unsent drafts one click away in the sidebar by @t3dotgg in pingdotgg/t3code#5777
* feat(web): project icons can be chosen manually by @t3dotgg in pingdotgg/t3code#5775
* fix(server): one greedy agent process no longer takes down the whole server by @t3dotgg in pingdotgg/t3code#5788
* ci: label-gated hosted-web preview deploys by @t3dotgg in pingdotgg/t3code#5465
* Add cross-platform mobile usage dashboard by @juliusmarminge in pingdotgg/t3code#5743
* fix(web): preserve desktop route during Clerk auth by @wobsoriano in pingdotgg/t3code#5770
* fix(web): match create theme and import theme buttons to the standard outline style by @UtkarshUsername in pingdotgg/t3code#5860
* fix(server): favicon resolution no longer pins the event loop by @murenovich in pingdotgg/t3code#5538
* fix(shared): bound the file-link label so bracket runs stop rescanning by @tsouth89 in pingdotgg/t3code#5782
* fix(web): thread title button no longer eats the drag area by @nathangerday in pingdotgg/t3code#5857
* fix(web): unify usage page chrome by @t3-code[bot] in pingdotgg/t3code#5823
* fix(shell): add ~/.local/bin to the Windows CLI resolver so native-installed providers are found by @arhxam in pingdotgg/t3code#5074
* fix(web): match settings search shortcut styling to command palette's by @UtkarshUsername in pingdotgg/t3code#5841
* fix(mobile): long-pressing a thread row no longer navigates into the thread by @juliusmarminge in pingdotgg/t3code#5901
* fix(server): usage no longer double-counts forked Codex sessions by @t3dotgg in pingdotgg/t3code#5887
* fix(server): sandbox user-provided SVGs by @t3dotgg in pingdotgg/t3code#5916
* fix(web): match usage titlebar text styling by @t3-code[bot] in pingdotgg/t3code#5897
* Move project settings to contextual project routes by @juliusmarminge in pingdotgg/t3code#5923
* Retain thread sidebar data when navigating to /settings so back navigation is instant by @juliusmarminge in pingdotgg/t3code#5930
* Automate production mobile EAS releases by @juliusmarminge in pingdotgg/t3code#5609
* Add settings and usage breadcrumbs by @juliusmarminge in pingdotgg/t3code#5929
* fix(web): correct model picker trigger padding by @Chrono-byte in pingdotgg/t3code#5935
* fix(web): show worktree icon in sidebar v2 by @tris203 in pingdotgg/t3code#5909
* fix(web): enable restore defaults after theme mix changes by @Lucenx9 in pingdotgg/t3code#5928
* fix(web): trait menu closes after you pick a level by @t3dotgg in pingdotgg/t3code#5879
* fix(web): align project name with headline by @carterwsmith in pingdotgg/t3code#5864
* fix(web): update pills use readable theme foregrounds by @chrisdeeming in pingdotgg/t3code#5938
* fix(web): use themed confirmation dialogs by @StiensWout in pingdotgg/t3code#5624
* fix(web): use import/export-appropriate icons for theme buttons by @UtkarshUsername in pingdotgg/t3code#5964
* fix(mobile): detect PowerShell cmdlet errors in work log rows by @myacoub91 in pingdotgg/t3code#5726
* fix(mobile): stop Android user bubbles with code blocks from overlapping by @Brechard in pingdotgg/t3code#5659
* fix(mobile): parse EAS fingerprint JSON by @juliusmarminge in pingdotgg/t3code#5991

## New Contributors
* @chrisdeeming made their first contribution in pingdotgg/t3code#5270
* @wobsoriano made their first contribution in pingdotgg/t3code#5770
* @murenovich made their first contribution in pingdotgg/t3code#5538
* @tsouth89 made their first contribution in pingdotgg/t3code#5782
* @nathangerday made their first contribution in pingdotgg/t3code#5857
* @carterwsmith made their first contribution in pingdotgg/t3code#5864
* @myacoub91 made their first contribution in pingdotgg/t3code#5726

**Full Changelog**: pingdotgg/t3code@v0.0.32...v0.0.33

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.33
sandscooling pushed a commit to sandscooling/t3code that referenced this pull request Aug 10, 2026
… cases

The v0.0.33 sync landed five new assertions in this file, from pingdotgg#5538 and
pingdotgg#5775, and all five fail on Windows. They compare resolvePath's output against
a forward-slash literal, and resolvePath returns a native absolute path, so on
this machine the two strings name the same file and disagree on spelling.

This is the exact assumption 2effde5 removed from the assertions that
existed at the time, and it left withPosixSeparators in the file for the
purpose. The new assertions simply predate nothing and were written on a
POSIX machine. They now go through the same helper, which is every remaining
unwrapped call, so the file no longer has two conventions in it.

Nineteen of nineteen pass. Nothing about the resolver changed: backslashes are
correct in its output, because its consumer feeds the result to path.relative.

Worth sending upstream. The suite is red on Windows without it, and the fix
costs upstream nothing on the platforms where it already passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Unanchored favicon regex pins the server event loop, wedging the environment into a permanent reconnect loop

2 participants