From 34581e55e76b98152531909b46ffcb80529f54f0 Mon Sep 17 00:00:00 2001 From: Anso Date: Fri, 8 May 2026 11:38:35 -0400 Subject: [PATCH] fix(mesh): treat local-node aliases as always reachable in diagnostics (#994) MeshService.getRouteDiagnostic called PilotTunnelManager.hasActiveTunnel unconditionally for the alias's target node. Local nodes never establish pilot tunnels because they do not need them (mesh same-node traffic uses the localhost fast path), so the call always returned false and the diagnostic flipped to state: 'tunnel down' even on a working local route. The route detail sheet and node card consequently rendered every local mesh alias as a destructive 'tunnel down' pill. Added a private isMeshReachable helper that returns true for any local-type node and otherwise delegates to hasActiveTunnel. Used it in both getRouteDiagnostic and getStatus, replacing the inline ternary in getStatus that already had the right shape but lived as duplicated logic. The helper falls back to false when the node row is missing (orphaned alias from a partial cascade), matching the pre-existing conservative behavior. Verified live: GET /api/mesh/aliases/echo.audit-mesh-prod.Local.sencho/diagnostic previously returned state='tunnel down' on a healthy local route. --- .../__tests__/mesh-diagnostic-local.test.ts | 143 ++++++++++++++++++ backend/src/services/MeshService.ts | 22 ++- 2 files changed, 161 insertions(+), 4 deletions(-) create mode 100644 backend/src/__tests__/mesh-diagnostic-local.test.ts diff --git a/backend/src/__tests__/mesh-diagnostic-local.test.ts b/backend/src/__tests__/mesh-diagnostic-local.test.ts new file mode 100644 index 00000000..f7aefa53 --- /dev/null +++ b/backend/src/__tests__/mesh-diagnostic-local.test.ts @@ -0,0 +1,143 @@ +/** + * Regression guard for the M-12 fix: `MeshService.getRouteDiagnostic` and + * `MeshService.getStatus` must report `pilotConnected: true` for local nodes + * instead of asking `PilotTunnelManager.hasActiveTunnel`, which always returns + * false for local nodes (they do not have pilot tunnels because they do not + * need them — local mesh traffic uses the same-node fast path). Without this + * conditional, the route detail sheet renders every local alias as a + * destructive `tunnel down` pill on a working route. + */ +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; +import { setupTestDb, cleanupTestDb } from './helpers/setupTestDb'; + +let tmpDir: string; +let MeshService: typeof import('../services/MeshService').MeshService; +let DatabaseService: typeof import('../services/DatabaseService').DatabaseService; +let PilotTunnelManager: typeof import('../services/PilotTunnelManager').PilotTunnelManager; + +beforeAll(async () => { + tmpDir = await setupTestDb(); + ({ MeshService } = await import('../services/MeshService')); + ({ DatabaseService } = await import('../services/DatabaseService')); + ({ PilotTunnelManager } = await import('../services/PilotTunnelManager')); +}); + +afterAll(() => { + vi.restoreAllMocks(); + cleanupTestDb(tmpDir); +}); + +afterEach(() => { + vi.restoreAllMocks(); + const svc = MeshService.getInstance() as unknown as { + aliasCache: Map; + aliasByPort: Map; + }; + svc.aliasCache = new Map(); + svc.aliasByPort = new Map(); +}); + +describe('MeshService diagnostic — local-node pilot state (M-12)', () => { + it('reports pilotConnected: true and state: healthy for a local alias even though no pilot tunnel exists', async () => { + const svc = MeshService.getInstance(); + const db = DatabaseService.getInstance(); + const localNodeId = db.getNodes()[0].id; + db.insertMeshStack(localNodeId, 'audit-mesh-prod', 'tester'); + + // hasActiveTunnel always returns false for local nodes; the helper + // must short-circuit that, otherwise the diagnostic flips to + // 'tunnel down'. + vi.spyOn(PilotTunnelManager.getInstance(), 'hasActiveTunnel').mockReturnValue(false); + + (svc as unknown as { aliasCache: Map }).aliasCache = new Map([ + ['echo.audit-mesh-prod.local.sencho', { + host: 'echo.audit-mesh-prod.local.sencho', + nodeId: localNodeId, + nodeName: 'local', + stackName: 'audit-mesh-prod', + serviceName: 'echo', + port: 9000, + }], + ]); + + const diag = await svc.getRouteDiagnostic('echo.audit-mesh-prod.local.sencho'); + expect(diag.pilot.connected).toBe(true); + expect(diag.state).toBe('healthy'); + + db.deleteMeshStack(localNodeId, 'audit-mesh-prod'); + }); + + it('reports pilotConnected: false when the alias points to a node that no longer exists', async () => { + const svc = MeshService.getInstance(); + const db = DatabaseService.getInstance(); + // Create a remote node, register an alias pointing at it, opt the + // stack in (so the alias is not 'not authorized'), then delete the + // node row. The diagnostic should fall back to false rather than + // throw or treat a missing node as reachable. + const remoteNodeId = db.addNode({ + name: 'm12-orphan', + type: 'remote', + mode: 'pilot_agent', + compose_dir: '/tmp', + is_default: false, + api_url: '', + api_token: '', + }); + db.insertMeshStack(remoteNodeId, 'audit-mesh-orphan', 'tester'); + (svc as unknown as { aliasCache: Map }).aliasCache = new Map([ + ['echo.audit-mesh-orphan.m12-orphan.sencho', { + host: 'echo.audit-mesh-orphan.m12-orphan.sencho', + nodeId: remoteNodeId, + nodeName: 'm12-orphan', + stackName: 'audit-mesh-orphan', + serviceName: 'echo', + port: 9002, + }], + ]); + // Drop the node row but leave the alias cache + mesh_stacks intact + // (mirrors a race where listMeshStacks ran before the node delete + // cascade completed). The orphan path is already covered by the + // initial !node check; this asserts the fallback is false rather + // than the test-default true. + db.deleteMeshStack(remoteNodeId, 'audit-mesh-orphan'); + db.deleteNode(remoteNodeId); + + const diag = await svc.getRouteDiagnostic('echo.audit-mesh-orphan.m12-orphan.sencho'); + expect(diag.pilot.connected).toBe(false); + }); + + it('still reports pilotConnected: false and state: tunnel down for a remote alias when the tunnel is actually down', async () => { + const svc = MeshService.getInstance(); + const db = DatabaseService.getInstance(); + const remoteNodeId = db.addNode({ + name: 'm12-remote', + type: 'remote', + mode: 'pilot_agent', + compose_dir: '/tmp', + is_default: false, + api_url: '', + api_token: '', + }); + db.insertMeshStack(remoteNodeId, 'audit-mesh-pilot', 'tester'); + + vi.spyOn(PilotTunnelManager.getInstance(), 'hasActiveTunnel').mockReturnValue(false); + + (svc as unknown as { aliasCache: Map }).aliasCache = new Map([ + ['echo.audit-mesh-pilot.m12-remote.sencho', { + host: 'echo.audit-mesh-pilot.m12-remote.sencho', + nodeId: remoteNodeId, + nodeName: 'm12-remote', + stackName: 'audit-mesh-pilot', + serviceName: 'echo', + port: 9001, + }], + ]); + + const diag = await svc.getRouteDiagnostic('echo.audit-mesh-pilot.m12-remote.sencho'); + expect(diag.pilot.connected).toBe(false); + expect(diag.state).toBe('tunnel down'); + + db.deleteMeshStack(remoteNodeId, 'audit-mesh-pilot'); + db.deleteNode(remoteNodeId); + }); +}); diff --git a/backend/src/services/MeshService.ts b/backend/src/services/MeshService.ts index d56ad2b0..1c1aad45 100644 --- a/backend/src/services/MeshService.ts +++ b/backend/src/services/MeshService.ts @@ -669,6 +669,22 @@ export class MeshService extends EventEmitter { // --- Diagnostics --- + /** + * Whether mesh traffic to this node can flow. Local nodes are always + * reachable because mesh uses the same-node fast path on localhost. + * Remote nodes are reachable only when a pilot tunnel is registered. The + * literal `hasActiveTunnel(localNodeId)` would always be false (local + * nodes do not establish tunnels to themselves), so a direct call would + * render every local alias as `tunnel down` in the UI even on a working + * route. + */ + private isMeshReachable(nodeId: number): boolean { + const node = DatabaseService.getInstance().getNode(nodeId); + if (!node) return false; + if (node.type !== 'remote') return true; + return PilotTunnelManager.getInstance().hasActiveTunnel(nodeId); + } + public async getRouteDiagnostic(alias: string): Promise { const target = this.lookupAliasGlobal(alias); const lastError = this.routeErrorMap.get(alias) || null; @@ -678,8 +694,7 @@ export class MeshService extends EventEmitter { return { alias, target: null, pilot: { connected: false, lastSeen: null }, lastError, lastProbeMs, state: 'not authorized' }; } - const ptm = PilotTunnelManager.getInstance(); - const pilotConnected = ptm.hasActiveTunnel(target.nodeId); + const pilotConnected = this.isMeshReachable(target.nodeId); const node = DatabaseService.getInstance().getNode(target.nodeId); const lastSeen = node?.pilot_last_seen ?? null; const optedIn = DatabaseService.getInstance().isMeshStackEnabled(target.nodeId, target.stackName); @@ -739,7 +754,6 @@ export class MeshService extends EventEmitter { public async getStatus(): Promise { const db = DatabaseService.getInstance(); - const ptm = PilotTunnelManager.getInstance(); const nodes = db.getNodes(); const out: MeshNodeStatus[] = []; for (const node of nodes) { @@ -749,7 +763,7 @@ export class MeshService extends EventEmitter { nodeName: node.name, enabled: db.getNodeMeshEnabled(node.id), sidecarRunning: await this.isSidecarRunning(node.id), - pilotConnected: node.type === 'remote' ? ptm.hasActiveTunnel(node.id) : true, + pilotConnected: this.isMeshReachable(node.id), optedInStacks, activeStreamCount: Array.from(this.activeStreams.values()).length, });