Skip to content

fix(desktop): add Nerd Font fallbacks to terminal font stacks - #72153

Open
adurham wants to merge 1 commit into
NousResearch:mainfrom
adurham:upstream-pr/desktop-nerd-font-terminal
Open

fix(desktop): add Nerd Font fallbacks to terminal font stacks#72153
adurham wants to merge 1 commit into
NousResearch:mainfrom
adurham:upstream-pr/desktop-nerd-font-terminal

Conversation

@adurham

@adurham adurham commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes terminal glyphs rendering as tofu boxes in Hermes Desktop's
xterm.js terminals when the user's shell prompt assumes Nerd Font glyphs.

Root Cause

Both xterm.js terminal instances (interactive PTY terminal and the
read-only agent-output terminal) hardcode a font stack with no Nerd Font
fallback:
"'JetBrains Mono', 'Cascadia Code', 'SF Mono', Menlo, Consolas, monospace".

Shell prompts that assume Nerd Font glyphs (powerline separators,
starship/oh-my-posh icons) render as tofu boxes instead of falling back
to an installed Nerd Font — unlike native terminals (kitty, etc.), whose
OS-level font fallback picks up an installed Nerd Font automatically.
xterm.js uses its own font-family CSS property rather than OS fallback,
so the fallback chain must be explicit.

Changes Made

Add Hack Nerd Font Mono / FiraCode Nerd Font Mono / Symbols Nerd Font Mono to the fallback chain after JetBrains Mono in both:

  • apps/desktop/src/app/right-sidebar/terminal/use-agent-terminal.ts
  • apps/desktop/src/app/right-sidebar/terminal/use-terminal-session.ts

Related Issue

No existing issue found (searched gh search issues for "tofu box
terminal font", "Nerd Font desktop terminal", "powerline glyph missing
desktop" — no hits).

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

How to Test

  1. Set up a shell prompt that uses Nerd Font glyphs (starship, oh-my-posh,
    powerlevel10k with powerline separators).
  2. Open Hermes Desktop's terminal panel without a Nerd Font installed as
    the system default.
  3. Before this fix: prompt glyphs render as tofu boxes (□).
  4. After this fix: glyphs fall back correctly if any of the three listed
    Nerd Fonts are installed.
  5. vitest run src/app/right-sidebar/terminal — 25 passed, 0 failed (no
    regressions; a font-family string constant has no meaningful unit test
    to add).

Checklist

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs/issues (none found)
  • My PR contains only changes related to this fix
  • I've run the relevant test suite and all tests pass
  • Tested on macOS

adurham added a commit to adurham/hermes-agent that referenced this pull request Jul 26, 2026
…tems

Documents outcomes for the 6 desktop-category Bucket A items:

Submitted (4):
- PR NousResearch#72151 -- RAF-throttle blank transcript on session switch
- PR NousResearch#72152 -- profile deletion zombie backend + cross-window rail staleness
- PR NousResearch#72153 -- Nerd Font terminal fallback
- PR NousResearch#72155 -- desktop model picker hiding Anthropic

Deprioritized, needs hand-reconciliation (3): drag-to-reorder, workspace
tab close button, queued composer wrong-session delivery. All three
rejected 1+ files on git apply --check with real semantic drift (not
line-offset noise) against the fast-churning desktop session/composer
code. Flagged for revisit rather than force-applying a stale patch.

The profile-deletion and model-picker items both got real scrutiny beyond
"does it apply cleanly": profile-deletion required two rounds of external
consult after search-first turned up 2 already-merged PRs on the same
headline symptom -- verified directly (not assumed) that this fix's two
pieces are genuinely non-overlapping gaps, not stale duplicates. Model
picker's fix was confirmed to extend an already-established upstream
credential-detection pattern rather than introduce new CC-mimicry
plumbing, addressing the specific caution flagged in the original audit.

Saved verified patches to .upstream-candidates/ for reference.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) labels Jul 26, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Desktop fix. Current main still uses the no-Nerd-Font stack in both xterm.js constructors: apps/desktop/src/app/right-sidebar/terminal/use-agent-terminal.ts:48 and apps/desktop/src/app/right-sidebar/terminal/use-terminal-session.ts:509.

A search of apps/desktop/src found exactly those two new Terminal call sites, and PR commit 02ea9093d8ff0767280dcc20e15a2f999997b2b5 updates both. The patch stays within renderer presentation options and introduces no state, backend, configuration, or prompt-cache changes. The removed stack in the PR diff exactly matches current main at both locations, so salvage should be mechanical.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 30, 2026
adurham added a commit to adurham/hermes-agent that referenced this pull request Jul 31, 2026
…s on all 7 PRs

Documents the real external engagement on the 7 upstream PRs filed
2026-07-26, and the 3 substantive follow-up fixes pushed in response:

- NousResearch#72054 closed as superseded, but merged anyway via NousResearch#74139 (contributor
  CrowLoki's reconciliation with NousResearch#62026, credited via Co-authored-by).
- NousResearch#72087, NousResearch#72151, NousResearch#72152, NousResearch#72153, NousResearch#72155, NousResearch#72164 all reviewed by the
  repo's automated sweeper -- keep_open/high on all 6.
- Fixed NousResearch#72087 (payload-proportional test assertions, catching a future
  allowlist-regression risk flagged by both the sweeper and an
  independent contributor who measured it precisely on their own fork).
- Fixed NousResearch#72152 (extracted ProfileRail's focus/visibilitychange wiring
  into a tested hook, matching the directory's own established
  use-profile-prewarm.ts pattern).
- Rebased NousResearch#72155 past a real merge conflict (an unrelated upstream
  test-pruning pass removed 3 tests my diff's context touched).

All fixes verified by simulating the exact regression each review was
warning about and confirming the new tests catch it, then restoring the
real fix.

Also noted a real environment issue found this session: the `upstream`
remote's SSH URL intermittently fails to connect from this network; a
one-off HTTPS fetch into a separate ref works around it without touching
the configured remote.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Retargeted after an upstream refactor (since this PR was originally
filed) introduced user-configurable terminal fonts: the two hardcoded
font-family strings this PR originally patched no longer exist as such
-- both call sites now derive their font family from
resolveTerminalFontFamily(), which falls back to the single
DEFAULT_TERMINAL_FONT_FAMILY constant in terminal-font.ts when the user
hasn't configured a custom font.

The underlying bug is unchanged and still real: DEFAULT_TERMINAL_FONT_FAMILY
had no Nerd Font fallback, so an unconfigured install renders powerline
separators and Codicon-range glyphs some CLI output emits as tofu boxes.
Since the fix is now centralized in one constant instead of two call
sites, the change is actually simpler than the original PR.

Existing tests reference DEFAULT_TERMINAL_FONT_FAMILY symbolically (not
as a hardcoded string literal), so they needed no changes and still pass
unmodified -- confirms the fix is contained to the one constant.

Tests: terminal-font.test.ts 8/8 passed. Broader regression sweep of the
terminal directory (use-agent-terminal, use-terminal-session,
use-terminal-font, persistent, rail, links, revive-buffer): 9 files,
61 tests passed, 0 regressions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@adurham
adurham force-pushed the upstream-pr/desktop-nerd-font-terminal branch from 02ea909 to fa59e5d Compare August 2, 2026 16:54
@adurham

adurham commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

Rebased and retargeted -- an upstream refactor since this PR was filed introduced user-configurable terminal fonts (resolveTerminalFontFamily() + DEFAULT_TERMINAL_FONT_FAMILY in terminal-font.ts), replacing the two hardcoded fontFamily strings this PR originally touched. The underlying bug is unchanged and still real for anyone on the unconfigured default -- just centralized in one constant now instead of two call sites, so the fix is actually simpler.

Existing tests reference the constant symbolically rather than as a string literal, so they needed no changes and pass as-is (8/8). Ran the full terminal directory as a regression check given the refactor: 9 files, 61 tests, 0 failures.

adurham added a commit to adurham/hermes-agent that referenced this pull request Aug 2, 2026
…Research#72153 both hit real upstream refactors

Documents rebasing both PRs onto main a few days after the first fix
round, finding both had real merge conflicts caused by unrelated
upstream work independently touching the exact code they target:

- NousResearch#72087: upstream's own NousResearch#73298 refactor extracted a shared
  _wire_message_shadow() helper and independently reintroduced the same
  underscore-mismatch bug this PR describes (verified 4.04x overcount on
  unpatched main). Retargeted the fix into the new shared helper.
- NousResearch#72153: upstream landed a user-configurable terminal font feature that
  replaced the two hardcoded strings this PR touched. The underlying bug
  was still real against the new default constant; fix collapsed to a
  single-constant change.

All 6 open PRs confirmed mergeable as of today.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
adurham added a commit to adurham/hermes-agent that referenced this pull request Aug 4, 2026
Post-merge audit of FORK.md against upstream v2026.8.3 found two fork
changes the merge silently dropped:

- terminal-font: Nerd Font fallbacks in DEFAULT_TERMINAL_FONT_FAMILY.
  Upstream's font-picker refactor (terminal-font.ts) replaced the two
  hardcoded fontFamily strings the fork had patched (FORK.md 2026-07-22,
  upstream PR NousResearch#72153); the merge took upstream's file verbatim, reviving
  the tofu-box bug. Re-applied as the one-constant form.

- config: agent.pin_anthropic_token default (FORK.md 2026-07-25).
  Upstream's config split (config.py -> config_defaults.py) dropped the
  fork's key; the reader in agent/anthropic_adapter.py fails safe, but
  the documented default belongs in DEFAULT_CONFIG.

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

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants