fix(auto-ack): skip incoming tapback/reaction packets in Auto-Acknowledge

[Yeraze]

Summary

• Fixes [BUG] meshtastic auto aknowledge tapback is also responding to others tapbacks #4569 — the Auto-Acknowledge tapback cell was responding to other users' tapbacks/reactions, wasting airtime since apps never display a reply to a reaction.
• Meshtastic tapback reactions are delivered as TEXT_MESSAGE_APP packets with the emoji flag set (payload text is the emoji itself). checkAutoAcknowledge never checked this flag, so any incoming reaction packet was processed like a normal message and could trigger an Auto-Ack reply/tapback.
• checkAutoAcknowledge (src/server/meshtasticManager.ts) now returns early whenever the incoming packet carries the emoji flag, before any regex matching or settings reads.

Test plan

• Added src/server/meshtasticManager.autoAckSkipReaction.test.ts: a reaction packet (emoji: 1) never reaches messageQueue.enqueue, while a normal text message still does (control case).
• npx vitest run on all existing Auto-Ack-related suites (meshtasticManager.autoAck*.test.ts, utils/autoAckDecision.test.ts) — all pass.
• npm run lint:ci — clean, no ratchet growth.
• npx tsc --noEmit — clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MSo3oMwB3PiR4t3m8JZAG9

---

Generated by Claude Code

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: fix(auto-ack): skip incoming tapback/reaction packets in Auto-Acknowledge

• Gather context and read changed files
• Review implementation in meshtasticManager.ts
• Review test coverage in meshtasticManager.autoAckSkipReaction.test.ts
• Provide final feedback

---

Summary

This is a clean, well-scoped fix. The core change is correct and the placement of the early-return guard is optimal.

---

Implementation Review (src/server/meshtasticManager.ts)

✅ Correct placement of the guard

The emoji check at meshtasticManager.ts:10197–10200 sits after the per-packet dedup guard (which is intentional — dedup prevents duplicate processing regardless of type) but before any settings reads. This is exactly right: it avoids unnecessary DB round-trips for a fast-path rejection.

✅ Truthy check is appropriate

emoji is typed as number | undefined on TextMessage (meshtasticManager.ts:313), and the upstream extraction at line 6278 normalizes it to undefined when <= 0. So if (message.emoji) correctly rejects any positive non-zero value while passing through the undefined case for normal text messages.

One observation worth considering: handleAutoPingCommand (meshtasticManager.ts:10556) and checkAutoResponder (meshtasticManager.ts:10965) are also called on the same incoming text message flow (lines 6396, 6399). Neither currently guards against the emoji flag. A tapback packet won't match a ping regex (^ping \d+$) and likely won't match a user's configured auto-responder regex either, but for correctness and consistency, those functions could benefit from the same early-return. This is a follow-up candidate, not a blocker.

---

Test Review (src/server/meshtasticManager.autoAckSkipReaction.test.ts)

✅ Tests the right behavior

• Case 1 (line 75): reaction packet with emoji: 1 → enqueue never called. This is the regression guard for [BUG] meshtastic auto aknowledge tapback is also responding to others tapbacks #4569.
• Case 2 (line 95): normal text 'test' with no emoji flag → enqueue is called. This confirms the fix doesn't regress the happy path.

✅ Mock setup is complete and correct

The settingsFor helper at line 43 sets autoAckEnabled: 'true' and all reply/tapback settings to 'true', which means the test would exercise the full auto-ack path for a normal message — making the control case meaningful.

Minor nit: The comment on line 80–82 says '👍' would not normally match the regex, but autoAckRegex: null in settingsFor maps to the default '^(test|ping)' per the comment on line 46. The emoji 👍 wouldn't match either way, but the comment accurately states the real intent (rejection before regex matching) — no action needed, just noting the slight imprecision.

Minor nit: The test uses (manager as any).checkAutoAcknowledge(...) and (manager as any).messageQueue.enqueue — this is consistent with the existing meshtasticManager.autoAck*.test.ts pattern for private method testing, so no concerns there.

---

Security / Airtime

No security concerns. The change only affects outbound behavior — it prevents MeshMonitor from transmitting unnecessary ack packets in response to tapback reactions, which is a net positive for airtime usage.

---

Verdict

Approved — the fix is minimal, correctly placed, and well-tested. The only optional follow-up is adding the same emoji guard to checkAutoResponder and handleAutoPingCommand for completeness, but neither is a regression risk given their regex-based matching.

[Yeraze]

Thanks for the review. Agreed on the observation about checkAutoResponder/handleAutoPingCommand — leaving that out of scope here since neither is what was reported in #4569 and both are already gated by regex matching that a raw emoji payload is very unlikely to satisfy, so the airtime-waste risk they'd carry is much lower than the tapback case. Happy to file a follow-up issue if you'd like it tracked separately.

-- Authored by Roger 🤓

---

Generated by Claude Code

[Yeraze]

Superseded by #4574, which merged as 776ef487 and closed #4569.

This PR got there first — opened 22:44 UTC, ~20 minutes before #4574 — and I missed it when I picked up the issue. Apologies for the duplicated effort.

The two fixes are the same idea in the same function, with one real difference worth keeping from this one:

if (message.emoji)          // here — truthy
if (message?.emoji != null) // what merged

They diverge only on emoji: 0. This PR treats it as an ordinary message; the merged guard skips it as a tapback. This PR's behaviour is the better one. Upstream normalizes 0 → undefined (decodedEmoji > 0 ? decodedEmoji : undefined), so it can't occur today — but if that normalization ever changed, the merged guard would silently stop acking legitimate messages.

I'm porting that to main in a follow-up rather than merging this, since merging would stack a second redundant guard in the same function.

Closing as superseded — not as wrong.