Claude finished @Yeraze's task —— View job

PR Review: refactor(server) — extract mesh-request, telemetry, connection, device & status route groups

Tasks:

• Gather context and understand request
• Review route files (meshRequestRoutes, telemetryRoutes, connectionRoutes, deviceStatusRoutes, statusRoutes)
• Review test files
• Review server.ts modifications
• Post final review

Overall this is a clean, well-structured refactor that follows the established pattern. The handler logic is faithfully extracted with auth/middleware preserved in the correct order. Tests are appropriately scoped and cover the key validation branches. Here are a few observations:

Findings

1. connectionRoutes.ts:33 — direct req.session.userId access

The GET /connection handler branches on req.session.userId directly rather than req.user:

// connectionRoutes.ts:33
if (!req.session.userId) {

Every other extracted route uses req.user (set by the auth middleware). This req.session access is consistent with the original server.ts and technically correct (session is populated before auth middleware runs), but it creates an inconsistency in the new module. Not a bug — just worth noting since it means the test shim in connectionRoutes.test.ts had to add a manual req.session initializer to work around it (line 50).

2. meshRequestRoutes.ts:188 — module-level rate-limit map persists across test runs

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

This is a module-level singleton that survives across vi.clearAllMocks() calls. The tests work around this correctly by using unique destination numbers (0x0000aaaa, 0x0000bbbb) per test, and the rate-limit test is effectively a stateful integration test. This is fine for now, but worth documenting: if the test file ever gains a beforeEach that reuses the same destination num, the rate-limit test could become order-dependent.

3. connectionRoutes.ts:123 — IP validation regex admits some edge cases

const ipRegex = /^(?:(?:(?:25[0-5]|2[0-4]\d|1\d{2}|[1-9]?\d)\.){3}...)|[\w.-]+)(?::\d{1,5})?$/;

The [\w.-]+ hostname branch will accept single characters (e.g. a), which is technically valid as a hostname. More importantly, \w includes digits, so 12345 alone passes as a hostname. The port is validated separately in 1-65535 range (line 130). This is pre-existing behavior moved verbatim — not a new regression, and the scope of this PR is extraction only.

4. telemetryRoutes.ts:264 — (req as any).user cast

const nodes = await filterNodesByChannelPermission(allNodes, (req as any).user);

All other callers in the same file use req.user (typed via the Express session augmentation). This is one of the two carried-over no-explicit-any warnings mentioned in the PR description. Minor — but if you're ever touching this route again, it's a candidate for cleaning up.

5. statusRoutes.ts:11 — getAllManagers() as any[] cast

const managers = sourceManagerRegistry.getAllManagers() as any[];

getAllManagers() returns a typed manager array from the registry. The cast is needed because virtualNodeServer isn't part of the declared manager interface. Fine for now, but a typed accessor or a union type narrowing would be cleaner long-term.

6. Tests — missing coverage for GET /connection with authenticated user exposing nodeIp

connectionRoutes.test.ts tests the anonymous path (nodeIp hidden), but there is no test for the authenticated path where nodeIp should be present. Since req.session.userId is what gates this, and the mock shim doesn't populate session.userId, the authenticated branch is untested. Low priority since the logic is trivial, but a gap worth closing.

Summary

• Code quality: Good. Handlers moved verbatim, middleware order preserved, consistent with existing route modules.
• Potential bugs: None identified that are introduced by this PR.
• Performance: No regressions. The module-level neighborInfoRequestTimestamps map is appropriate for a rate-limiter.
• Security: The requireAdmin() gate on /device/security-keys is correctly preserved. The anonymous IP-hiding logic on GET /connection is correctly preserved.
• Test coverage: 33 tests covering the key validation and happy paths. The one gap (authenticated nodeIp exposure) is low risk.

This is a mechanical extraction PR and achieves its goal cleanly. The observations above are minor and mostly pre-existing — none are blockers.