Skip to content

Conversation

@jaaydenh
Copy link
Contributor

Summary

Fix browser-tab favicon rendering when mux is loaded in Chrome by serving and referencing favicon assets in a way that works in both source-tree server mode and built/dist mode.

Background

In browser mode, users could see a tab title but no favicon. This came from a few compounding issues:

  • favicon links pointed at root-absolute paths and were hardcoded in runtime theme switching
  • source-mode static serving did not expose public/ assets at root
  • SPA fallback returned index.html for missing asset requests (including favicon lookups)
  • favicon files were PNG bytes with .ico extension, which can produce MIME/type mismatches in browser contexts

Implementation

  • Updated index.html favicon references to use relative PNG paths (favicon.png, icon.png) for browser compatibility behind URL prefixes.
  • Updated theme favicon switching in ThemeContext to use PNG assets and resolve URLs against document.baseURI.
  • Added resolveOrpcStaticDir() in src/node/orpc/server.ts to detect both dist and source layouts.
  • Updated static serving to:
    • serve resolvedStaticDir
    • also serve resolvedStaticDir/public when present
    • avoid SPA fallback for extension-bearing asset paths, so missing assets return 404 instead of HTML.
  • Updated ServerService to use resolveOrpcStaticDir().
  • Extended icon generation to emit both .ico and .png favicons.
  • Added dist/**/*.ico to npm files so ico assets are included in published package artifacts.
  • Added/updated tests covering static dir resolution, missing-asset fallback behavior, root public asset serving, and theme-driven favicon updates.

Validation

  • make static-check
  • bun test src/node/orpc/server.test.ts src/browser/contexts/ThemeContext.test.tsx

Risks

  • Low-to-moderate: touches shared static serving logic and favicon URL behavior.
  • Mitigations: coverage added for static-dir resolution and fallback behavior; targeted theme favicon test added.

Generated with mux • Model: openai:gpt-5.3-codex • Thinking: xhigh • Cost: $0.73

Switch the browser favicon links to PNG assets with relative paths, update theme-based favicon swapping to resolve against the document base URL, and harden static serving so missing asset requests are not rewritten to index.html.

Also add PNG favicon outputs to icon generation, include .ico files in npm packaging for compatibility, and extend tests around static asset fallback and themed favicon behavior.

---

_Generated with `mux` • Model: `openai:gpt-5.3-codex` • Thinking: `xhigh` • Cost: `$0.73`_

<!-- mux-attribution: model=openai:gpt-5.3-codex thinking=xhigh costs=0.73 -->
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4ff4c3bd8b

ℹ️ 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".

Comment on lines +765 to +766
if (path.extname(req.path)) {
return next();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve SPA fallback for dotted workspace IDs

The new path.extname(req.path) check classifies any URL whose final segment contains a dot as a static file request, so SPA fallback is skipped for routes like /workspace/project-1.2.3. That breaks browser-mode reload/direct-link flows because workspace URLs are path-based (/workspace/${encodeURIComponent(id)} in src/browser/contexts/RouterContext.tsx) and legacy IDs derived from workspace basenames can include dots (generateLegacyId in src/node/config.ts). In that scenario the server now returns 404 instead of index.html, preventing the app from loading.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant