@tanrax can you resolve the conflicts here ?

@Yeraze Of course! I resolved the conflicts and ran the tests

Code Review

Overview

Adds four POST endpoints under /api/v1/actions/ (traceroute, request-position, request-nodeinfo, request-neighbors) so Bearer-token API clients can trigger node actions without session cookies/CSRF. Replicates the internal endpoint behavior including system-message creation and the 180s neighbor-info rate limit.

Critical Issues

1. Uses the deprecated singleton meshtasticManager — breaks 4.x multi-source architecture

import meshtasticManager from '../../meshtasticManager.js';
...
await meshtasticManager.sendTraceroute(destinationNum, channel);
await meshtasticManager.sendPositionRequest(destinationNum, channel);
await meshtasticManager.sendNodeInfoRequest(destinationNum, channel);
await meshtasticManager.sendNeighborInfoRequest(destinationNum, channel);
meshtasticManager.getLocalNodeInfo();

Per CLAUDE.md, that import is a @deprecated compatibility shim. In 4.x there is no global manager — every node IO must go through sourceManagerRegistry.getManager(sourceId). As-is these endpoints will only ever act on the default source and silently ignore the rest. The other v1 resource routers all live under /sources/:sourceId/... for this exact reason; actions should follow the same pattern (/api/v1/sources/:sourceId/actions/traceroute, with the default alias supported).

2. System-message id format violates the load-bearing rule

await databaseService.messages.insertMessage({
  id: `${packetId}`,                       // <-- wrong
  ...
});

The canonical row id is \${sourceId}_\${fromNum}_\${packetId}. Using just \${packetId} will cause cross-source duplicates in /api/unified/messages. The internal endpoints currently produce the canonical format — these new ones do not. Repeated in three places (request-position, request-nodeinfo) plus implicitly by collisions across sources.

3. Permission check pattern duplicates instead of using attachSource

if (!user?.isAdmin && !(user ? await hasPermission(user, 'traceroute', 'write') : false)) { ... }

Every other v1 route uses attachSource(resource, action) middleware so permissions are honored per source. Hand-rolling hasPermission here is global, which combined with #1 means a user with traceroute:write on source A can drive actions on whichever source the singleton happens to point at. This is the per-source permission boundary that migration 033 was meant to close.

4. Authentication appears un-applied on the router

Routes only read (req as any).user. I don't see Bearer auth (or any auth) applied on the new /actions router or in v1/index.ts for this mount. If the parent router doesn't wrap this with a token-verifying middleware, an unauthenticated request reaches the handler, user is undefined, and the check !user?.isAdmin && !(user ? ... : false) returns 403 — which works defensively but indicates the auth chain isn't intentional. Worth confirming a bearer-verifying middleware runs upstream and adjusting the early-return so anonymous requests get 401, not 403.

Moderate Issues

5. No handler tests

The only test change is one line in v1-api.test.ts asserting the endpoint is listed. The "Test plan" checkboxes are all unchecked. Given the project's testing standard (Vitest + repository mocks), at minimum each endpoint needs:

• 200 happy path,
• 400 missing destination,
• 403 missing permission,
• 429 rate-limited (request-neighbors),
• 403 not-local-and-not-0-hop (request-neighbors),
• system-message insertion verification (would catch issue feat: traceroute highlighting and UI improvements #2).

6. resolveDestination quirks

const num = parseInt(raw, raw.startsWith('0x') ? 16 : 10);

• Hex strings without ! or 0x prefix (e.g. \"a1b2c3d4\") silently parse to NaN. The PR description says hex strings are supported — they aren't, except via ! prefix or 0x prefix.
• parseInt('!00000000'.slice(1), 16) returns 0, then if (!destinationNum) rejects with 400. Probably intentional (nodeNum 0 isn't real), but combined with nodeNum: 0 being treated identically as "missing", it's ambiguous. Use explicit === undefined / == null checks instead of truthiness.
• Consider hardening: Number.isInteger(num) && num > 0 && num <= 0xFFFFFFFF.

7. Rate-limit map grows unbounded

const neighborInfoRequestTimestamps = new Map<number, number>();

Never pruned. Mostly cosmetic, but trivially fixed by deleting entries older than 2× the limit on each insert, or by reusing the rate-limit infrastructure already used by the internal endpoint. It is also not source-scoped, so the same destination across two sources shares one window.

8. Duplicated business logic

Each handler inlines the same "build system message → set channel to -1 if 0 → call insertMessage" recipe, already duplicated from the internal endpoints. Extracting an action helper (createOutgoingActionMessage(...) colocated with the manager) would eliminate the existing duplication and let the new handlers reuse it — which would also fix issue #2 in one place.

9. Channel override only on request-position

const channel = (typeof req.body.channel === 'number' && req.body.channel >= 0 && req.body.channel <= 7)
  ? req.body.channel : (node?.channel ?? 0);

The other three actions do not accept a channel override. Either expose it consistently or omit it — undocumented in the PR body either way.

Minor / Style

• (req as any).user — the project has a typed Request augmentation; prefer it.
• parseInt(...) ? : null returning null then checking !destinationNum conflates 0 and null. Use destinationNum == null.
• Number(destinationNum) === Number(localNodeNum) — destinationNum is already number; the coercion is redundant unless guarding against BIGINT from PG/MySQL paths (in which case do it at the source where the data crosses the boundary, not at the comparison).
• Comment // Handle \"!hex\" format — the kind of inline narration the repo style guide asks to skip.

Security

See #1, #3, #4. Net: with the singleton + global permission check, a token authorized for source A could potentially trigger sends on a different source.

Recommendation

Request changes. The route is useful, but it needs to (a) live under /sources/:sourceId/actions/... with attachSource, (b) drive through sourceManagerRegistry.getManager(sourceId), (c) emit messages with the \${sourceId}_\${fromNum}_\${packetId} id format, and (d) ship handler-level tests. Once those are in, the rest is polish.

Done! Can you check now please?

Pushed a follow-up refactor (4e90b38) addressing the remaining piece of code-review item #3 — the attachSource middleware pattern. The handlers now drop the inline 401/hasPermission blocks and delegate source resolution + per-source permission checks to attachSource(resource, action) per-route, matching every other v1 resource. Net effects:

• POST /api/v1/sources/default/actions/* now resolves the default alias to the first source the caller can write on, instead of falling back to the legacy singleton.
• Unknown sourceId returns 404 (consistent with the rest of v1), unauthorized callers get 401.
• Permission-check duplication removed from the four handlers.
• actions.test.ts updated: databaseService.sources.{getSource,getAllSources} mocked, 403 body assertions updated to the canonical { resource, action, sourceId } shape, and two new tests cover the default alias 200 / 404 paths.

All 47 actions tests pass locally; CI is running. Take a look when you have a moment — feel free to revert the commit if you'd prefer to land your version as-is and tackle this as a follow-up PR.