fix(meshcore): preserve node name on empty advert + improve metadata text contrast by Yeraze · Pull Request #3771 · Yeraze/meshmonitor

Yeraze · 2026-06-26T07:40:07Z

Summary

Two small fixes for issues reported today:

Fix 1 — MeshCore node name lost after empty advert (#3756)

Root cause: Zero-hop repeaters (and some MeshCore firmware builds) send contact_advertised events with adv_name: "". The ?? null-coalescing operator passes an empty string through unchanged (since "" is not null/undefined), so the stored node name was silently overwritten with an empty value on every re-advert.

Fix: Change both the in-memory update (handleBridgeEvent line 1112) and the DB write (persistContact line 1473) to use || instead of ??. This makes empty strings fall back to the previously known value, the same way null/undefined already did.

Test: Added a regression test in meshcoreManager.contactPersistence.test.ts that fires a named advert followed by an empty-name re-advert and asserts both the in-memory contact and the upsertNode DB call retain the original name.

Fix 2 — MeshCore message metadata text readability (#3769)

Problem: The timestamp, hop-route, and scope metadata lines in MeshCore channel messages used --ctp-overlay0, which is too faint in Catppuccin light themes and still quite subtle in dark themes.

Fix: Switch all three CSS classes (mc-message-time, mc-message-route, mc-message-scope) from --ctp-overlay0 to --ctp-subtext0. This gives better contrast in both light and dark mode while keeping the metadata visually secondary to the message body.

Files changed

File	Change
`src/server/meshcoreManager.ts`	`??` → `
`src/server/meshcoreManager.contactPersistence.test.ts`	Regression test for empty adv_name
`src/components/MeshCore/MeshCorePage.css`	`--ctp-overlay0` → `--ctp-subtext0` for time/route/scope (3 lines)

Test plan

Run vitest run src/server/meshcoreManager.contactPersistence.test.ts — new regression test should pass
Connect a zero-hop repeater (MeshCore firmware v1.16) — node name should persist after re-advert and not revert to "Unknown"
Open MeshCore channel messages — timestamp, hop-route, and scope text should be visibly more readable, especially in light theme

🤖 Generated with Claude Code

https://claude.ai/code/session_01UKnaQv47NKrW5Srxk88HHG

Generated by Claude Code

…3756) Zero-hop repeaters (and some MeshCore firmware builds) send `contact_advertised` events with `adv_name: ""`. The `??` operator passes an empty string through because `""` is not `null`/`undefined`, so the stored name was silently overwritten with an empty value on every re-advert. Change both the in-memory update (handleBridgeEvent) and the DB write (persistContact) to use `||` instead of `??` so empty strings fall back to the previously known value, the same way `null`/`undefined` already did. Adds a regression test that fires a named advert followed by an empty-name re-advert and asserts both the in-memory contact and the DB call retain the original name. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UKnaQv47NKrW5Srxk88HHG

Scope and hop-route metadata lines (and the timestamp) were using --ctp-overlay0, which is too faint in Catppuccin light themes and still subtle in dark themes. Switch to --ctp-subtext0 for all three classes to give better contrast in both light and dark mode while keeping them visually secondary to the message body. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UKnaQv47NKrW5Srxk88HHG

github-actions · 2026-06-26T07:40:21Z

Claude finished @Yeraze's task —— View job

PR Review

Overall this is a clean, well-scoped PR. Both fixes are correct and the regression test is solid. A few observations below.

Fix 1 — `??` → `||` for empty advert name (`meshcoreManager.ts`)

Correctness: The fix is right. ?? only catches null/undefined, passing empty strings through. || correctly falls back when adv_name is falsy (empty string, null, or undefined). Both the in-memory update (line 1112) and the DB write (line 1473) are covered, which is exactly what's needed.

Minor observation: The persistContact call at line 1473 already does contact.advName || contact.name || null. Since line 1112 now sets advName correctly before persisting, the fallback chain in persistContact is the right layered defence. No concern here.

One thing to double-check: There are other call sites that set advName on a contact — notably the bulk refresh path at line 2090 (advName: c.adv_name) and the MQTT/initialisation path at line 2137 (advName: n.name ?? undefined). The bulk refresh at line 2090 uses a direct assignment without the || existing.advName guard. If a bulk refresh returns an empty adv_name, that path would still wipe the name. This wasn't changed in this PR and may be intentional (bulk refresh is a full read from device, not a push event), but it's worth being aware of.

Fix 2 — CSS contrast (`MeshCorePage.css`)

Lines 721, 724, 725: The change from --ctp-overlay0 to --ctp-subtext0 for .mc-message-time, .mc-message-route, and .mc-message-scope is the right move. In Catppuccin Latte (light), overlay0 is #9ca0b0 (WCAG contrast ~2.5:1 on the white base), while subtext0 is #6c6f85 (~4.0:1) — a meaningful accessibility improvement.

Consistency note: .mc-delivery-sent and .mc-delivery-sending (lines 741, 744) still use --ctp-overlay0. These are also secondary text in the message stream. If the goal is consistent metadata readability, those could be updated in a follow-up, though they're delivery indicators rather than content metadata so a slightly dimmer treatment may be intentional.

Test coverage (`meshcoreManager.contactPersistence.test.ts`)

The new regression test at line 293 ("does not overwrite a known contact name with an empty adv_name") is well structured:

Fires a named advert to establish baseline
Clears mock call history
Fires empty-name re-advert
Asserts both in-memory advName and the DB upsertNode call use the preserved name

Two micro-notes:

The test uses await Promise.resolve(); await Promise.resolve(); to flush the void this.persistContact(...) microtask. This is the same pattern as the existing tests, so it's fine, but it is fragile if persistContact is ever wrapped in an additional async layer. The existing flush = () => new Promise(r => setTimeout(r, 0)) helper (line 45) is more robust (drains microtasks + one macrotask). The new test could use await flush() for consistency with the "fires a new node discovered notification" test — not a blocker.
The existing test at line 252 ("does not notify for a brand-new contact that has no display name yet") sends an advert with no adv_name key at all (not even ""). The new test covers the empty-string case specifically. Together they give good coverage of the edge cases.

Summary

Both fixes are correct, minimal, and well-tested. No blocking issues. The only thing worth a follow-up look is whether the bulk-refresh path (c.adv_name at line 2090) should also get the || existing.advName guard, but that's separate from the push-event bug fixed here.

Yeraze · 2026-06-26T12:42:59Z

Superseded. The #3769 metadata-contrast half of this PR already shipped in #3778 (time/route/scope set to --ctp-overlay1), so this PR now conflicts on MeshCorePage.css. PRs #3771 and #3772 are also duplicates of each other. The still-relevant part — the #3756 node-name-on-empty-advert fix (??→||) — is being re-submitted as a fresh minimal PR rebased on current main with no CSS changes. Closing in favor of that.

claude added 2 commits June 26, 2026 07:39

Yeraze mentioned this pull request Jun 26, 2026

[BUG] MeshCore: Unnamed node in the node list #3756

Closed

Yeraze mentioned this pull request Jun 26, 2026

[FEAT] MeshCore: Colors for scope and path display in messages not well readable #3769

Closed

Yeraze closed this Jun 26, 2026

This was referenced Jun 26, 2026

fix(meshcore): preserve node name on empty advert + mute route/scope colors #3772

Closed

fix(meshcore): preserve node name on empty advert (#3756) #3785

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(meshcore): preserve node name on empty advert + improve metadata text contrast#3771

fix(meshcore): preserve node name on empty advert + improve metadata text contrast#3771
Yeraze wants to merge 2 commits into
mainfrom
claude/great-dijkstra-a6ar7a

Yeraze commented Jun 26, 2026

Uh oh!

github-actions Bot commented Jun 26, 2026 •

edited

Loading

Uh oh!

Yeraze commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Yeraze commented Jun 26, 2026

Summary

Fix 1 — MeshCore node name lost after empty advert (#3756)

Fix 2 — MeshCore message metadata text readability (#3769)

Files changed

Test plan

Uh oh!

github-actions Bot commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review

Fix 1 — ?? → || for empty advert name (meshcoreManager.ts)

Fix 2 — CSS contrast (MeshCorePage.css)

Test coverage (meshcoreManager.contactPersistence.test.ts)

Summary

Uh oh!

Yeraze commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Jun 26, 2026 •

edited

Loading

Fix 1 — `??` → `||` for empty advert name (`meshcoreManager.ts`)

Fix 2 — CSS contrast (`MeshCorePage.css`)

Test coverage (`meshcoreManager.contactPersistence.test.ts`)