feat(meshcore-vn): relay GetNeighbours via SendBinaryReq(50) to physical node (#3904) (#3993)

Final gap of issue #3904: after login through the MeshCore Virtual Node, the
meshcore-flutter app's "neighbours" action fell through to `default:` in
`dispatchCommand` and returned Err(UnsupportedCmd).

Corrects the record: neighbours is NOT SendRawData(25) (the issue's inference).
It is SendBinaryReq(50) — a GENERIC binary-request command dispatched on an
inner sub-type — with sub-type GetNeighbours(0x06), answered by an async
BinaryResponse(0x8C) push. Verified against the vendored reference lib
`@liamcottle/meshcore.js` (`getNeighbours`, `sendCommandSendBinaryReq`,
`onBinaryResponsePush`, `sendBinaryRequest`), the SAME companion protocol
meshcore-flutter speaks.

Confirmed protocol:
- App→VN: `[50][targetPubkey:32][reqData]`; for neighbours
  `reqData = [0x06][request_version=0][count][offset:u16LE][order_by]`
  `[pubkey_prefix_len][random_tag:u32LE]`.
- VN→App: async BinaryResponse(0x8C) `[0x8C][reserved:1][tag:u32LE]`
  `[totalCount:u16LE][resultsCount:u16LE]` then resultsCount ×
  `[prefix:pubkey_prefix_len][heardSecondsAgo:u32LE][snr:int8=snr*4]`.
- Tag correlation: the client matches the push by `BinaryResponse.tag ==
  Sent.expectedAckCrc` (NOT the request's random_tag), so we echo the request's
  random_tag into BOTH the Sent's expectedAckCrc and the BinaryResponse tag.

Implementation (mirrors the #3961/#3991 Sent-ack → async-push template):
- `dispatchCommand` → `handleSendBinaryReq`: parse envelope, dispatch on the
  inner sub-type. GetNeighbours(0x06) implemented; any other sub-type gets an
  explicit Err(UnsupportedCmd) so future ones are obvious, not silently dropped.
- `handleGetNeighboursReq`: reply Sent (tag echoed), call
  `manager.getNeighbours(publicKey, {count, offset, orderBy})`, push a
  BinaryResponse re-serialized with the requested pubkey_prefix_len stride and
  snr as round(snr*4) int8.
- Ungated by allowAdminCommands — read-only follow-up to login, like
  SendTelemetryReq/SendStatusReq (the real node answers based on the session).
- Graceful: bad envelope/inner blob → Err(IllegalArg); null result (non-repeater
  / no session / disconnected) → empty BinaryResponse (client tolerates it);
  manager throw → log and emit nothing (app times out, matching siblings).

Codec: adds `parseSendBinaryReq`, `parseGetNeighboursReq`,
`encodeBinaryResponsePush`, `encodeNeighboursPayload`, `PushCodes.BinaryResponse
= 0x8C`, and `BinaryRequestTypes`.

VALIDATED END-TO-END against a live repeater: drove the real meshcore.js client
over TCP against the VN on :5000, handshook, and called
`getNeighbours('661b0674…')` — the VN relayed 16-total/10-returned real
neighbours from the physical node, tag-correlated (Sent expectedAckCrc ==
BinaryResponse tag). Tests mirror the sibling VN suites.


Claude-Session: https://claude.ai/code/session_015AFBA76hsjqhsXe1BdnYub

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Author: Yeraze

src/server/meshcoreCompanionCodec.ts

@@ -96,6 +96,17 @@ export const PushCodes = {
   LogRxData: 0x88,
   TraceData: 0x89,
   TelemetryResponse: 0x8b,
+  BinaryResponse: 0x8c,
+} as const;
+
+/**
+ * Inner sub-type of a SendBinaryReq(50) generic binary request (mirrors
+ * meshcore.js `Constants.BinaryRequestTypes`, which mirror the firmware
+ * `REQ_TYPE_*` defines). `req_data[0]` selects the request.
+ */
+export const BinaryRequestTypes = {
+  GetTelemetryData: 0x03, // REQ_TYPE_GET_TELEMETRY_DATA
+  GetNeighbours: 0x06, // REQ_TYPE_GET_NEIGHBOURS
 } as const;
 
 /** Error codes carried by an Err(1) response. */
@@ -244,6 +255,29 @@ export interface SendLoginCmd { publicKey: string; password: string; }
 export interface SendTracePathCmd { tag: number; auth: number; flags: number; path: Buffer; }
 export interface SendTelemetryReqCmd { publicKey: string; }
 export interface SendStatusReqCmd { publicKey: string; }
+/**
+ * A decoded SendBinaryReq(50): the target public key plus the inner request
+ * blob. `reqType` is `reqData[0]` (a `BinaryRequestTypes` value); the caller
+ * dispatches on it and re-parses `reqData` with the matching sub-parser.
+ */
+export interface SendBinaryReqCmd { publicKey: string; reqType: number; reqData: Buffer; }
+/** Parsed GetNeighbours(0x06) request parameters (inner sub-type of SendBinaryReq). */
+export interface GetNeighboursReq {
+  count: number;
+  offset: number;
+  orderBy: number;
+  pubkeyPrefixLen: number;
+  /** The app's random blob (`req_data[7..10]`); echoed back as the correlation tag. */
+  tag: number;
+}
+/** One neighbour entry for the GetNeighbours response payload. */
+export interface NeighbourEntry {
+  /** Public-key prefix as a lowercase hex string (may be longer than requested). */
+  publicKeyPrefix: string;
+  heardSecondsAgo: number;
+  /** SNR in dB (already /4, as MeshCoreManager returns it). */
+  snr: number;
+}
 export interface SetRadioParamsCmd { freq: number; bw: number; sf: number; cr: number; }
 export interface SetTxPowerCmd { power: number; }
 export interface SetAdvertLatLonCmd { lat: number; lon: number; }
@@ -320,6 +354,44 @@ export function parseSendStatusReq(payload: Buffer): SendStatusReqCmd {
   return { publicKey: payload.subarray(1, 33).toString('hex') };
 }
 
+/**
+ * SendBinaryReq(50): `[code][targetPublicKey:32][reqData…]` (see meshcore.js
+ * `sendCommandSendBinaryReq`). A generic binary-request envelope; the inner
+ * `reqData[0]` byte selects the request (a `BinaryRequestTypes` value). Returns
+ * the target public key (lowercase hex), the sub-type, and the full inner blob
+ * for the matching sub-parser. Requires at least one inner byte so `reqType` is
+ * always defined.
+ */
+export function parseSendBinaryReq(payload: Buffer): SendBinaryReqCmd {
+  if (payload.length < 1 + 32 + 1) throw new Error('SendBinaryReq: short payload');
+  const reqData = Buffer.from(payload.subarray(33));
+  return {
+    publicKey: payload.subarray(1, 33).toString('hex'),
+    reqType: reqData[0],
+    reqData,
+  };
+}
+
+/**
+ * Parse the GetNeighbours(0x06) inner request blob (mirrors meshcore.js
+ * `getNeighbours`):
+ *   `[type:u8][request_version:u8][count:u8][offset:u16LE][order_by:u8]`
+ *   `[pubkey_prefix_len:u8][random_tag:u32LE]` — 11 bytes.
+ * `random_tag` is the app's correlation blob (echoed back as the BinaryResponse
+ * tag). Throws on a short buffer so the dispatcher can reply Err(IllegalArg).
+ */
+export function parseGetNeighboursReq(reqData: Buffer): GetNeighboursReq {
+  if (reqData.length < 1 + 1 + 1 + 2 + 1 + 1 + 4) throw new Error('GetNeighbours: short payload');
+  return {
+    // reqData[0] = type, reqData[1] = request_version (ignored)
+    count: reqData[2],
+    offset: reqData.readUInt16LE(3),
+    orderBy: reqData[5],
+    pubkeyPrefixLen: reqData[6],
+    tag: reqData.readUInt32LE(7),
+  };
+}
+
 /**
  * SetRadioParams(11): `[code][freq:u32LE][bw:u32LE][sf:u8][cr:u8]`.
  * Wire freq is kHz and bw is Hz; returned in MHz / kHz for MeshCoreManager.setRadio.
@@ -761,6 +833,58 @@ export function encodeStatusResponsePush(
   return Buffer.concat([head, encodeRepeaterStatusData(status)]);
 }
 
+/**
+ * Encode a BinaryResponse(0x8C) push — the reply to a SendBinaryReq the app
+ * initiated. Layout mirrors meshcore.js `onBinaryResponsePush`:
+ * `[0x8C][reserved:1][tag:u32LE][responseData:rest]`.
+ *
+ * The client correlates this push to its pending request by comparing `tag` to
+ * the `expectedAckCrc` it received in the preceding Sent(6) response (see
+ * meshcore.js `sendBinaryRequest`), so the caller MUST have echoed this same
+ * `tag` value into that Sent frame.
+ */
+export function encodeBinaryResponsePush(tag: number, responseData: Buffer | Uint8Array): Buffer {
+  const head = Buffer.alloc(1 + 1 + 4);
+  head[0] = PushCodes.BinaryResponse;
+  head[1] = 0; // reserved
+  head.writeUInt32LE(tag >>> 0, 2);
+  return Buffer.concat([head, Buffer.from(responseData)]);
+}
+
+/**
+ * Serialize the GetNeighbours(0x06) response payload (inverse of meshcore.js
+ * `getNeighbours`'s response decode):
+ *   `[totalCount:u16LE][resultsCount:u16LE]` then `resultsCount` ×
+ *   `[prefix:pubkeyPrefixLen][heardSecondsAgo:u32LE][snr:int8]`.
+ *
+ * - `pubkeyPrefixLen` sets the per-entry prefix STRIDE and MUST equal the value
+ *   the app sent in its request (it slices exactly that many bytes back out).
+ *   Each entry's hex prefix is truncated/zero-padded to that length; the manager
+ *   fetches an 8-byte prefix, so requests for a longer prefix are zero-padded.
+ * - `snr` is a dB value; the wire carries `snr×4` (int8), matching how the app
+ *   divides the byte by 4 on decode.
+ */
+export function encodeNeighboursPayload(
+  total: number,
+  neighbours: NeighbourEntry[],
+  pubkeyPrefixLen: number,
+): Buffer {
+  const stride = Math.max(0, pubkeyPrefixLen);
+  const b = Buffer.alloc(2 + 2 + neighbours.length * (stride + 4 + 1));
+  let o = 0;
+  b.writeUInt16LE(total & 0xffff, o); o += 2;
+  b.writeUInt16LE(neighbours.length & 0xffff, o); o += 2;
+  for (const n of neighbours) {
+    const prefixBytes = pubKeyHexToBytes(n.publicKeyPrefix); // hex → bytes (zero-fills short/undefined)
+    const prefix = Buffer.alloc(stride); // zero-padded to the requested stride
+    prefixBytes.copy(prefix, 0, 0, Math.min(stride, prefixBytes.length));
+    prefix.copy(b, o); o += stride;
+    b.writeUInt32LE((n.heardSecondsAgo ?? 0) >>> 0, o); o += 4;
+    b.writeInt8(clampInt8(Math.round((n.snr ?? 0) * 4)), o); o += 1;
+  }
+  return b;
+}
+
 /**
  * Encode a LogRxData(0x88) push — the node forwarding a raw received OTA packet
  * to the app. This is the diagnostic "packet feed" that tools like

src/server/meshcoreVirtualNodeServer.test.ts

@@ -8,6 +8,7 @@ import {
   ResponseCodes,
   ErrorCodes,
   PushCodes,
+  BinaryRequestTypes,
   FRAME_APP_TO_NODE,
   FRAME_NODE_TO_APP,
   SUPPORTED_COMPANION_PROTOCOL_VERSION,
@@ -130,6 +131,19 @@ class FakeManager extends EventEmitter implements MeshCoreVirtualNodeManager {
   requestNodeStatus(publicKey: string) {
     return this.requestNodeStatusMock(publicKey) as Promise<Record<string, number> | null>;
   }
+  getNeighboursMock = vi.fn().mockResolvedValue({
+    total: 3,
+    neighbours: [
+      { publicKeyPrefix: 'aabbccddeeff0011', heardSecondsAgo: 42, snr: 5.25 },
+      { publicKeyPrefix: '1122334455667788', heardSecondsAgo: 3600, snr: -2.5 },
+    ],
+  });
+  getNeighbours(publicKey: string, opts?: { count?: number; offset?: number; orderBy?: number }) {
+    return this.getNeighboursMock(publicKey, opts) as Promise<{
+      total: number;
+      neighbours: { publicKeyPrefix: string; heardSecondsAgo: number; snr: number }[];
+    } | null>;
+  }
   emitMessage(msg: MeshCoreMessage) { this.emit('message', msg); }
   emitSendConfirmed(data: { ackCode: number; roundTripMs: number }) { this.emit('send_confirmed', data); }
   emitOtaPacket(data: { snr?: number | null; rssi?: number | null; raw_hex?: string | null }) {
@@ -1009,3 +1023,168 @@ describe('MeshCoreVirtualNodeServer — SendStatusReq relay (#3904)', () => {
     expect(manager.requestNodeStatusMock).not.toHaveBeenCalled();
   });
 });
+
+// SendBinaryReq(50)/GetNeighbours(0x06): reply Sent (with the request's
+// random_tag echoed as expectedAckCrc), then push BinaryResponse(0x8C) carrying
+// the re-serialized neighbour list correlated by the same tag (#3904 final gap).
+// The neighbours protocol is SendBinaryReq(50)/GetNeighbours(0x06), NOT
+// SendRawData(25) as the issue originally inferred.
+describe('MeshCoreVirtualNodeServer — SendBinaryReq/GetNeighbours relay (#3904)', () => {
+  let server: MeshCoreVirtualNodeServer;
+  let client: TestClient;
+  let manager: FakeManager;
+
+  const REMOTE_KEY = 'e5'.repeat(32);
+  const REMOTE_KEY_BYTES = Buffer.from(REMOTE_KEY, 'hex');
+  const TAG = 0x11223344;
+
+  async function startWith(allowAdminCommands: boolean): Promise<void> {
+    manager = new FakeManager();
+    server = new MeshCoreVirtualNodeServer({ port: 0, manager, allowAdminCommands, databaseService: CHANNELS_DB });
+    await server.start();
+    client = new TestClient();
+    await client.connect(server.getListeningPort()!);
+  }
+  afterEach(async () => { client?.close(); await server?.stop(); });
+
+  // [50][pubkey:32][0x06][version:0][count][offset:u16LE][orderBy][prefixLen][tag:u32LE]
+  function neighboursFrame(
+    publicKeyHex: string,
+    opts: { count?: number; offset?: number; orderBy?: number; prefixLen?: number; tag?: number } = {},
+  ): number[] {
+    const { count = 10, offset = 0, orderBy = 0, prefixLen = 8, tag = TAG } = opts;
+    const req = Buffer.alloc(11);
+    req[0] = BinaryRequestTypes.GetNeighbours;
+    req[1] = 0; // request_version
+    req[2] = count;
+    req.writeUInt16LE(offset, 3);
+    req[5] = orderBy;
+    req[6] = prefixLen;
+    req.writeUInt32LE(tag >>> 0, 7);
+    return [CommandCodes.SendBinaryReq, ...Buffer.from(publicKeyHex, 'hex'), ...req];
+  }
+
+  it('replies Sent (tag echoed) then pushes a tag-correlated BinaryResponse with the neighbour list', async () => {
+    await startWith(false); // ungated
+    const frames = client.expectFrames(2);
+    client.send(neighboursFrame(REMOTE_KEY, { count: 5, offset: 2, orderBy: 1, prefixLen: 8 }));
+    const [sent, push] = await frames;
+
+    // Sent(6): [code][result:i8][expectedAckCrc:u32LE][estTimeout:u32LE]
+    expect(sent[0]).toBe(ResponseCodes.Sent);
+    expect(sent.readUInt32LE(2)).toBe(TAG); // expectedAckCrc echoes the request tag
+
+    // BinaryResponse(0x8C): [code][reserved:1][tag:u32LE][total:u16LE][count:u16LE][entries…]
+    expect(push[0]).toBe(PushCodes.BinaryResponse);
+    expect(push[1]).toBe(0); // reserved
+    expect(push.readUInt32LE(2)).toBe(TAG); // tag correlates to the Sent's expectedAckCrc
+    const body = push.subarray(6);
+    expect(body.readUInt16LE(0)).toBe(3); // totalCount (from manager.total)
+    expect(body.readUInt16LE(2)).toBe(2); // resultsCount (neighbours.length)
+
+    // stride = prefixLen(8) + heard(4) + snr(1) = 13 bytes per entry
+    const e0 = body.subarray(4, 4 + 13);
+    expect(e0.subarray(0, 8)).toEqual(Buffer.from('aabbccddeeff0011', 'hex'));
+    expect(e0.readUInt32LE(8)).toBe(42); // heardSecondsAgo
+    expect(e0.readInt8(12)).toBe(21); // snr 5.25 dB → round(5.25*4)=21
+
+    const e1 = body.subarray(4 + 13, 4 + 26);
+    expect(e1.subarray(0, 8)).toEqual(Buffer.from('1122334455667788', 'hex'));
+    expect(e1.readUInt32LE(8)).toBe(3600);
+    expect(e1.readInt8(12)).toBe(-10); // snr -2.5 dB → round(-2.5*4)=-10
+
+    expect(push.length).toBe(6 + 4 + 2 * 13);
+    expect(manager.getNeighboursMock).toHaveBeenCalledWith(REMOTE_KEY, { count: 5, offset: 2, orderBy: 1 });
+  });
+
+  it('respects the requested pubkey_prefix_len for the entry stride', async () => {
+    await startWith(false);
+    const frames = client.expectFrames(2);
+    client.send(neighboursFrame(REMOTE_KEY, { prefixLen: 6 }));
+    const [, push] = await frames;
+    const body = push.subarray(6);
+    expect(body.readUInt16LE(2)).toBe(2); // 2 entries
+    // stride now 6 + 4 + 1 = 11 bytes/entry
+    expect(push.length).toBe(6 + 4 + 2 * 11);
+    const e0 = body.subarray(4, 4 + 11);
+    expect(e0.subarray(0, 6)).toEqual(Buffer.from('aabbccddeeff', 'hex')); // first 6 prefix bytes
+    expect(e0.readUInt32LE(6)).toBe(42);
+    expect(e0.readInt8(10)).toBe(21);
+  });
+
+  it('relays neighbours even when allowAdminCommands is off (read-only follow-up to login)', async () => {
+    await startWith(true);
+    const frames = client.expectFrames(2);
+    client.send(neighboursFrame(REMOTE_KEY));
+    const [sent, push] = await frames;
+    expect(sent[0]).toBe(ResponseCodes.Sent);
+    expect(push[0]).toBe(PushCodes.BinaryResponse);
+  });
+
+  it('pushes an empty BinaryResponse (total=0/count=0) when there are no neighbours', async () => {
+    await startWith(false);
+    manager.getNeighboursMock.mockResolvedValueOnce({ total: 0, neighbours: [] });
+    const frames = client.expectFrames(2);
+    client.send(neighboursFrame(REMOTE_KEY));
+    const [sent, push] = await frames;
+    expect(sent[0]).toBe(ResponseCodes.Sent);
+    expect(push[0]).toBe(PushCodes.BinaryResponse);
+    expect(push.readUInt32LE(2)).toBe(TAG);
+    const body = push.subarray(6);
+    expect(body.readUInt16LE(0)).toBe(0); // total
+    expect(body.readUInt16LE(2)).toBe(0); // count
+    expect(push.length).toBe(6 + 4); // header + [total][count] only
+  });
+
+  it('pushes an empty BinaryResponse when the manager returns null (non-repeater / disconnected)', async () => {
+    await startWith(false);
+    manager.getNeighboursMock.mockResolvedValueOnce(null);
+    const frames = client.expectFrames(2);
+    client.send(neighboursFrame(REMOTE_KEY));
+    const [sent, push] = await frames;
+    expect(sent[0]).toBe(ResponseCodes.Sent);
+    expect(push[0]).toBe(PushCodes.BinaryResponse);
+    const body = push.subarray(6);
+    expect(body.readUInt16LE(0)).toBe(0);
+    expect(body.readUInt16LE(2)).toBe(0);
+  });
+
+  it('replies Sent but pushes nothing when the manager throws', async () => {
+    await startWith(false);
+    manager.getNeighboursMock.mockRejectedValueOnce(new Error('bridge down'));
+    const sent = await client.request(neighboursFrame(REMOTE_KEY));
+    expect(sent[0]).toBe(ResponseCodes.Sent);
+    const second = await Promise.race([
+      client.expectFrames(1).then((f) => f[0]),
+      new Promise<null>((r) => setTimeout(() => r(null), 100)),
+    ]);
+    expect(second).toBeNull();
+  });
+
+  it('replies Err(IllegalArg) on a short SendBinaryReq envelope, without querying the node', async () => {
+    await startWith(false);
+    // [50][pubkey:32] with no inner req_data byte → too short.
+    const res = await client.request([CommandCodes.SendBinaryReq, ...REMOTE_KEY_BYTES]);
+    expect(res[0]).toBe(ResponseCodes.Err);
+    expect(res[1]).toBe(ErrorCodes.IllegalArg);
+    expect(manager.getNeighboursMock).not.toHaveBeenCalled();
+  });
+
+  it('replies Err(IllegalArg) on a short GetNeighbours inner blob, without querying the node', async () => {
+    await startWith(false);
+    // Valid envelope + sub-type but truncated params (< 11 inner bytes).
+    const res = await client.request([CommandCodes.SendBinaryReq, ...REMOTE_KEY_BYTES, BinaryRequestTypes.GetNeighbours, 0, 5]);
+    expect(res[0]).toBe(ResponseCodes.Err);
+    expect(res[1]).toBe(ErrorCodes.IllegalArg);
+    expect(manager.getNeighboursMock).not.toHaveBeenCalled();
+  });
+
+  it('replies Err(UnsupportedCmd) for an unknown inner sub-type, without querying the node', async () => {
+    await startWith(false);
+    // Envelope + an unimplemented sub-type (0x03 GetTelemetryData is not handled here).
+    const res = await client.request([CommandCodes.SendBinaryReq, ...REMOTE_KEY_BYTES, BinaryRequestTypes.GetTelemetryData, 0, 0]);
+    expect(res[0]).toBe(ResponseCodes.Err);
+    expect(res[1]).toBe(ErrorCodes.UnsupportedCmd);
+    expect(manager.getNeighboursMock).not.toHaveBeenCalled();
+  });
+});