Claude finished @Yeraze's task —— View job

PR Review

• Read changed files
• Analyze related code for context
• Provide review feedback

Summary: This is a clean, targeted security/correctness fix. The change is correct and the test is well-written. No blocking issues found.

Code Quality

sourceDashboardData.ts:105 — The one-line fix is exactly right:

- const localNode = await databaseService.nodes.getNode(localNodeInfo.nodeNum);
+ const localNode = await databaseService.nodes.getNode(localNodeInfo.nodeNum, source.id);

The getNode signature at nodes.ts:130 confirms that omitting sourceId does a cross-source first-match (eq(nodes.nodeNum, nodeNum) only), which is the documented legacy behavior. Passing source.id selects the correct WHERE clause (and(eq(nodes.nodeNum, nodeNum), eq(nodes.sourceId, sourceId))). The fix is minimal and correct.

The inline comment added at lines 99–104 is thorough — it explains the "why" (cross-source first-match risk), the historical origin, and the fallthrough behavior. This is good practice for a subtle concurrency/multi-source correctness fix.

Test Coverage

sourceRoutes.dashboard.test.ts:119-138 — The new test case covers the key invariants:

1. getNode is called with (nodeNum, source.id) — asserted at line 133 ✅
2. When getNode returns null (not yet stored under this source), the synthesized fallback record is used ✅
3. The synthesized node carries sourceId: 'src-mqtt' (line 137), confirming it's source-scoped ✅

One minor observation: the test uses mockResolvedValue(null) for getNode regardless of arguments (line 124), which means it would also pass if the implementation called getNode(nodeNum) without a sourceId — the mock doesn't verify the second argument is specifically source.id. However, the expect(...).toHaveBeenCalledWith(42, 'src-mqtt') assertion at line 133 catches this correctly. The test is sound.

Potential Issues

None found. The fallthrough path (lines 109–118 in sourceDashboardData.ts) was already source-scoped — it injects sourceId: source.id into the synthesized record, so the synthesized node was never the problem; only the DB-lookup path was. This fix closes that gap.

Performance

No regression. This is a strict improvement — the per-source WHERE clause can use the composite (nodeNum, sourceId) index (established in migration 029), whereas the old cross-source query would scan all rows matching nodeNum and return the first.

Security

This is a data isolation fix. Without it, on a multi-source deployment where two sources have heard the same nodeNum, the local node injection could pull another source's DB row (including its position data) into the current source's response. The fix correctly prevents this cross-source data leak.

Verdict: ✅ Approve. The fix is correct, minimal, well-commented, and well-tested. The PR description accurately describes the root cause and impact.