Claude finished @Yeraze's task —— View job

PR Review: fix(meshcore): block DMs to repeaters with explanatory notice (#3755)

Tasks:

• Gather context on changed files
• Review MeshCoreDirectMessagesView.tsx
• Review MeshCoreMessageStream.tsx
• Review MeshCorePage.css
• Check test coverage
• Provide final feedback

Summary

This is a clean, well-scoped fix. The approach — replacing the send bar with an informational notice rather than disabling it or hiding the conversation — is the right UX choice. The implementation is minimal and doesn't regress existing behavior. A few observations below.

Code Quality

MeshCoreDirectMessagesView.tsx:244

const isRepeater = (selectedContact?.advType ?? 0) === 2;

The magic number 2 is repeated inline. The codebase already has a MeshCoreDeviceType.REPEATER constant (referenced in meshcoreManager.getAllNodes.test.ts). Using the named constant would make the intent self-documenting and future-proof if device type values ever change:

import { MeshCoreDeviceType } from '...';
const isRepeater = selectedContact?.advType === MeshCoreDeviceType.REPEATER;

MeshCoreMessageStream.tsx:317–321
The conditional rendering correctly gates on readOnlyNotice being truthy. No concern here.

MeshCoreDirectMessagesView.tsx:390–392

readOnlyNotice={isRepeater
  ? t('meshcore.repeater_no_dm', 'Repeaters cannot receive direct messages.')
  : undefined}

Passing undefined when not a repeater is the correct way to signal "no notice". The prop JSDoc in MeshCoreMessageStream also clearly documents this.

Potential Issues

Room servers (advType === 3) are excluded from the DM list entirely (line 186: c.advType !== 3 filter), so they're correctly handled already. However, it's worth confirming there are no edge cases where a room server contact could appear in the DM view with an advType other than what's stored (e.g., if advType is null or undefined for a newly discovered repeater). The ?? 0 default in isRepeater handles null/undefined gracefully.

Historical messages remain visible for repeaters — this is called out in the PR description and is intentional. It's the right decision.

Test Coverage Gap

The PR description claims "15 targeted tests pass (MeshCoreDirectMessagesView.test.tsx + MeshCoreMessageStream.test.tsx)", but reviewing both test files, no new tests were added for the repeater read-only notice feature specifically:

• MeshCoreDirectMessagesView.test.tsx has no test for isRepeater detection or readOnlyNotice rendering
• MeshCoreMessageStream.test.tsx has no test for the readOnlyNotice prop

The "15 tests" appear to be pre-existing tests that continue to pass, not new tests covering this feature. The following test cases are missing:

1. Selecting a repeater contact (advType === 2) should render the notice text instead of the send bar
2. Selecting a non-repeater contact should show the normal send bar (regression guard)
3. MeshCoreMessageStream with readOnlyNotice prop should render the notice div and suppress the send bar

CSS

MeshCorePage.css:805–813
The .meshcore-readonly-notice rule is clean and consistent with the surrounding send-bar styles. The border-top, background, and padding values correctly mirror .meshcore-send-bar so the layout height stays stable when switching between a repeater and non-repeater contact. No concerns.

Verdict

The logic is correct and the UX improvement is real. The only actionable feedback is:

1. Missing tests for the new behavior — this is the main gap. The feature is not regression-tested.
2. Optional: use MeshCoreDeviceType.REPEATER instead of the bare 2 literal for clarity.

Superseded by #3760, which keeps repeaters/unmessagable nodes listed with full telemetry and removes the messaging UI (showing node details instead), per maintainer direction on #3755.