feat(automation): MeshCore scope condition + self-origin guard (#3914)

[Yeraze]

Summary

Implements the #3914 request: answer a MeshCore message based on its channel scope — e.g. nudge newbies who post unscoped or on a very broad region (like de) toward better node config. Adds a first-class condition.meshcoreScope block so this is a single, friendly condition rather than a hand-wired chain. Also adds a general self-origin guard to the Automation Engine so our own traffic never triggers rules (most importantly, so an auto-reply can't re-trigger itself in an infinite mesh loop).

Changes

• New condition.meshcoreScope block (src/types/automation.ts, conditionEvaluator.ts, builder catalog.ts):
  • mode ∈ {named, unscoped, scoped}. named takes a comma-separated regions list plus an optional "also match unscoped" toggle → expresses "region de OR unscoped" in one block.
  • Reads the inbound scopeCode/scopeName already exposed by buildMeshCoreMessageContext ([FEAT] Configurable flood hop limit for automated Unscoped messages #3833). Meshtastic messages carry no scope and therefore never match (degrades gracefully). The reply half already works via action.sendMessage scopeMode: 'trigger'.
  • Graph validation: named requires regions or includeUnscoped; unknown modes rejected.
  • MeshCore scope tokens (scopeName/scopeCode/scoped/fromName) documented in the Substitutions drawer + typo-checker.
• Self-origin guard (automationEngineService.ts, engineContext.ts, meshNodeData.ts):
  • Drops events from our OWN node before any rule runs: message fromNodeNum (Meshtastic) / fromPublicKey (MeshCore, case-insensitive) == the source's local node, plus our own telemetry and node-updates.
  • Prevents action.sendMessage replies (which re-enter the bus via emitNewMessage/emitMeshCoreMessage) from re-triggering their own rule, and stops our own periodic telemetry/node-info from spuriously firing rules.
  • Identity resolved per-source/protocol via optional NodeDataProvider.getLocalNodeNum/getSelfPublicKey (real impls read the Meshtastic + MeshCore manager registries; absent in unit tests → no drop). Geofence checks intentionally exempt. Mirrors the legacy MeshCore auto-responder self-reply guard.

Issues Resolved

Fixes #3914

Documentation Updates

• docs/internal/dev-notes/AUTOMATION_ENGINE_PLAN.md — added condition.meshcoreScope to the block catalog and the self-origin guard to the engine safety rails.

Testing

• Unit tests pass (full suite: 7933 passed / 0 failed, success: true)
• TypeScript compiles cleanly (tsc --noEmit)
• Condition: unscoped / scoped / named (+ includeUnscoped incl. the "de OR unscoped" case) / Meshtastic non-match
• Graph validation of the new block (valid modes; empty named rejected)
• Engine self-exclusion: Meshtastic message, MeshCore message (case-insensitive key), telemetry
• Manual: in the builder, add a MeshCore source rule → When a message is received → If MeshCore message scope (named: de, also match unscoped) → Send a message (scope: match the triggering message's scope) and confirm unscoped/de senders get a reply while other regions don't, and the reply doesn't loop.

🤖 Generated with Claude Code

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: feat(automation): MeshCore scope condition + self-origin guard (#3914)

• Read and analyze changed files
• Review code quality and best practices
• Check for potential bugs or issues
• Review test coverage
• Post final review feedback

---

Overall this is a well-structured, focused PR. The implementation is clean, follows the project's established patterns, and comes with solid test coverage. A few observations below.

---

Code Quality & Best Practices

conditionEvaluator.ts:93–114 — condition.meshcoreScope

The implementation is clear and correct. The scopeCode === 0 unscoped check is the right approach. One subtle note: in named mode, if p.includeUnscoped === true and isUnscoped is true, the function returns true immediately at line 109 before it even checks whether scopeCode is undefined. That's intentional and correct — the comment documents it. The ordering (includeUnscoped check before the scopeName == null check) is intentional and avoids a false-negative for the named + includeUnscoped + unscoped message case.

automationEngineService.ts:363–374 — self-origin guard methods

private async isSelfMeshtastic(sourceId: string | null, fromNodeNum: number | null | undefined): Promise<boolean> {
  if (!this.data.getLocalNodeNum || fromNodeNum == null) return false;
  const local = await this.data.getLocalNodeNum(sourceId);
  return local != null && Number(local) === Number(fromNodeNum);
}

The Number(local) === Number(fromNodeNum) double-coercion is slightly defensive — getLocalNodeNum already returns number | null and fromNodeNum is typed as number | null | undefined. This is fine given the CLAUDE.md warning about unsigned 32-bit node IDs in Postgres, but worth noting the guard is deliberate.

meshNodeData.ts:67–84 — live registry reads

async getLocalNodeNum(sourceId) {
  try {
    if (!sourceId) return null;
    const nodeNum = sourceManagerRegistry.getManager(sourceId)?.getLocalNodeInfo()?.nodeNum;
    return nodeNum != null ? Number(nodeNum) : null;
  } catch {
    return null;
  }
},

Good: the try/catch ensures a disconnected source (no registered manager) doesn't throw into the hot path. The optional chaining throughout is appropriate.

---

Potential Bugs / Edge Cases

1. named mode default when mode is undefined

In conditionEvaluator.ts:105:

const mode = String(p.mode ?? 'named');

And in automation.ts:349:

const mode = p.mode == null ? 'named' : p.mode;

The validator defaults to named when mode is absent. This is consistent with the builder (which shows named first), but it means a block with no explicit mode would be validated as named — and then immediately fail with "requires params.regions or params.includeUnscoped" if neither is set. That's an appropriate and safe behavior (it rejects the broken config), but it's worth being aware that an omitted mode quietly becomes named.

2. onNode self-guard only covers Meshtastic

async onNode(kind, nodeNum, changedKeys, sourceId): Promise<number> {
  if (await this.isSelfMeshtastic(sourceId, nodeNum)) return 0;
  ...
}

There is no corresponding MeshCore self-guard for onNode. However, looking at the MeshCore architecture, node updates appear to come via onMessage which already has the isSelfMeshCore guard — so this seems intentional. The PR description also only mentions "telemetry and node-updates" in the Meshtastic context. If MeshCore ever adds a node-update event path directly, this would need revisiting.

3. geofence self-guard exemption is intentional but undocumented in code

checkGeofences has no self-origin guard, and the PR description says "Geofence checks intentionally exempt." A brief inline comment in checkGeofences noting this exemption would help future maintainers. Currently the reasoning is only in the PR description and the AUTOMATION_ENGINE_PLAN.md doc.

---

Performance Considerations

The self-origin guard adds two await calls per onMessage/onMeshCoreMessage invocation even if no automations are registered for that trigger type. Minor, but the pattern of checking this.index.get(ctx.triggerType)?.length === 0 before the guard would eliminate unnecessary async work on idle sources. Not a blocking concern at mesh-radio scale.

---

Security

No concerns. The isSelfMeshCore comparison is case-insensitive (.toLowerCase()) which correctly handles hex key casing differences. The regions param goes through resolveOperand (which handles {{ }} templates) and then a basic .split(',').map(trim) — no injection surface. The guard is fail-safe (absent provider → no drop, not a blanket allow).

---

Test Coverage

Coverage is good for a feature of this scope:

• ✅ unscoped / scoped modes in conditionEvaluator.test.ts
• ✅ named with region list (case-insensitive, comma-separated)
• ✅ named + includeUnscoped ("de OR unscoped" case — the primary motivating use-case)
• ✅ Meshtastic message non-match (no scopeCode field → never matches)
• ✅ Graph validation: valid modes, rejected empty-named block, unknown mode
• ✅ Self-origin guard: Meshtastic node-num, MeshCore public key (case-insensitive), telemetry
• ✅ Default data provider in unit tests (no getLocalNodeNum/getSelfPublicKey) → no drop

Missing test:

• onNode self-origin guard — there's a test for onMessage and onTelemetry self-exclusion, but no test for onNode (node-discovered / node-updated). Would round out the coverage.

---

Minor Nits

• catalog.ts:253 — regionSelect is introduced as a FieldKind. The PR adds the type to the union at catalog.ts:9 — confirm there's a corresponding renderer in the builder component that handles regionSelect. (Not in the changed files; presumably it already exists or is a follow-up.)
• SubstitutionsHelp.tsx:21–22 — the MeshCore tokens (fromName, scopeName, scopeCode, scoped) are grouped alongside decryptedBy with no (MeshCore) separator between the common and MeshCore-specific tokens. The inline (MeshCore) annotation in the description strings is fine, just relies on the user reading the descriptions.

---

Summary

This is a solid implementation. The logic is correct, the tests cover the main cases well, and the self-origin guard is a meaningful safety rail for the entire engine (not just this feature). The suggestions above are minor — the main actionable items are:

1. (Low) Add a self-origin guard test for onNode (node-updated/discovered).
2. (Very low) Add an inline comment in checkGeofences noting the intentional exemption from the self-guard.