fix(meshtastic): stop auto-acknowledging tapbacks

[Yeraze]

Summary

Fixes #4569. Reported by Snayler.

checkAutoAcknowledge (meshtasticManager.ts) reads hopStart, hopLimit, viaMqtt, relayNode and text off the incoming message — but never emoji, the field that marks a TEXT_MESSAGE_APP packet as a reaction. The value is already extracted and set on the message object upstream (normalized to undefined when absent or zero); it was simply never consulted. So an inbound tapback flowed through the normal auto-ack path and drew a response, either a text reply or the hop-count tapback.

That response is pure waste, exactly as reported: clients don't render a reaction to a reaction, so the sender never sees it. It only costs airtime.

Why this went unreported for so long: the default regex is ^(test|ping), which a bare emoji can't match. The bug only fires for operators running a permissive pattern (., .*) to ack everything — where every reaction on the channel drew a reply.

The fix

One guard at the top of checkAutoAcknowledge: skip any message carrying an emoji value.

It sits ahead of the per-packet dedup guard on purpose, so a skipped tapback never consumes a slot in the bounded autoAckProcessedPackets set (capped at 1000, trimmed to 500). There's a test for that ordering.

MQTT-ingested messages don't call this function, so this single site covers the feature.

Test plan

• npx tsc --noEmit and npx tsc -p tsconfig.server.json --noEmit — clean
• npm run lint:ci — clean, no baseline growth
• Full npx vitest run: 13,504 passed, 0 failed
• New src/server/meshtasticManager.autoAckTapbackSkip.test.ts (5 tests): tapback skipped under a match-everything regex; any non-zero emoji counts, not just 1; ordinary message still acked and explicit-null emoji still acked (the two controls that stop an over-broad guard from passing vacuously); and the dedup-slot ordering above.
• Sibling autoAckTapbackEmoji.test.ts still green — the hop-count emoji MeshMonitor sends is unaffected.

Red/green verified: with the guard commented out, exactly the three tapback assertions fail and both controls keep passing. The tests fail for the right reason, not by accident.

Note on the test harness

The suite enables the tapback cells and leaves the reply cells off, matching the sibling autoAckTapbackEmoji suite. Enabling the reply cells needs channel/token plumbing this harness doesn't stub — my first draft did that and the control tests failed silently because the reply path threw inside the outer try. The tapback path alone proves the guard, since it's the response an ordinary message produces under these settings.

---

Generated by Claude Code

https://claude.ai/code/session_01EtJnjbUgYwJfNU6XXACbFf

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: fix(meshtastic): stop auto-acknowledging tapbacks

• Read changed files
• Review code quality and correctness
• Check test coverage
• Post final feedback

---

Overall: LGTM — clean, well-targeted, well-tested fix.

Code Quality

meshtasticManager.ts:10192 — The guard is correct:

if (message?.emoji != null) {

The != null check (null and undefined but not 0) is exactly right given the upstream normalization at line 6278:

const emoji = (decodedEmoji !== undefined && decodedEmoji > 0) ? decodedEmoji : undefined;

Zero and absent both normalize to undefined, so != null correctly passes through emoji=0 (no reaction) while catching any truthy reaction value. The optional chaining on message?. is a minor defensive nicety that doesn't hurt.

Placement — Putting this check before the dedup guard (line 10201+) is the right call. As the comment explains, a rejected tapback shouldn't consume a slot in autoAckProcessedPackets. The ordering is verified by the fifth test.

Comment quality — Unusually thorough inline documentation. The explanation of why this went unreported (default regex can't match bare emoji) is genuinely useful operational context.

Test Coverage

Five tests in the new file are well-chosen:

1. Tapback is skipped under permissive regex (emoji: 1)
2. Ordinary message still acked (positive control — critical for preventing false-pass)
3. Any non-zero emoji value treated as tapback (emoji: 2)
4. Explicit null emoji doesn't skip (the null vs undefined distinction)
5. Dedup slot not burned by tapback (ordering invariant)

The explicit note about using a permissive regex (.) instead of the default ^(test|ping) is important — without it, all tapback tests would pass vacuously with the guard removed.

The null-control test (case 4) deserves specific praise: null vs undefined is the subtle edge the != null check handles, and testing it explicitly guards against someone "simplifying" the check to if (message?.emoji) later, which would incorrectly skip emoji: 0 messages (not that those should exist post-normalization, but defensive).

Minor Observations (non-blocking)

1. The test helper run() creates a new MeshtasticManager per call (line 98), which means autoAckProcessedPackets is always fresh. The final dedup-ordering test (case 5) correctly uses a single manager instance for both calls. No issue — just worth noting the intentional asymmetry.

2. emoji: null path from other transports — The test at line 136 covers this and the code handles it correctly. Worth noting for reviewers: the ?? at line 13795 (emoji: msg.emoji ?? undefined) means a DB-stored null becomes undefined before reaching checkAutoAcknowledge, so the emoji: null test is actually exercising a theoretical path that current code doesn't produce. Still a valid test — it documents the intended behavior if the normalization ever changes or a different code path is added.

3. message as any parameter type — The method signature already has message: any at line 10172, so this isn't new. Noted in case there's a future effort to type this parameter.

Security / Performance

No concerns. This is strictly a guard that exits early — it can only reduce airtime usage, never increase it.

---

The fix is minimal, correct, and well-documented. The test suite exercises the exact failure modes described in the PR body and includes the controls needed to prevent vacuous passes.