Skip to content

Commit 64081b6

Browse files
Yerazeclaude
andauthored
feat(db)!: withSourceScope fails closed — omitting sourceId throws, ALL_SOURCES sentinel for intentional cross-source (#3962) (#3976)
* feat(db): fail-closed withSourceScope + ALL_SOURCES sentinel (#3962 Task 1.1 WP-A) Add ALL_SOURCES unique symbol and SourceScope type. Rewrite withSourceScope to throw on missing/empty sourceId (fail-closed) and return undefined (no WHERE clause) for ALL_SOURCES. Export from barrel index.ts. Tests: unit Probe subclass tests all 5 cases; perSource integration test seeds two sources and verifies isolation, ALL_SOURCES cross-source, and fail-closed throw. @ts-expect-error proves omission is a Tier-2 compile error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AphLfgUJWaFNAgU5UXdJ4n * feat(db): widen repo signatures to SourceScope, fix external omit-sites (#3962 WP-B) Tier-1: widen sourceId params across 7 repos (channels, messages, neighbors, nodes, notifications, telemetry, traceroutes) from string to SourceScope so ALL_SOURCES threads through cleanly. Legacy *Sqlite sync twins updated to handle ALL_SOURCES (treat as no-filter) in hand-rolled patterns. Tier-2: make leading-param aggregates required (getAllNodes, getNodeCount, getActiveNodeCount, getAllChannels, getChannelCount, getMessageCount, getAllNeighborInfo) so omit-sites are compile errors. External call sites (#1-#14 legit-cross-source): pass ALL_SOURCES + comment in purgeRoutes, systemRoutes, webSocketService, mqttBridgeManager, database.ts init cache warm-up, exportData, getAllNodesAsync, getAllNeighborInfo facade, deviceBackupService, meshtasticManager, dbNodeMapper, autoDelete, automation/meshNodeData, analysisRoutes, v1 routes, channelRoutes, server.ts. Bug fix (#15): server.ts getNodeCount in /nodes/refresh now passes refreshSourceId (matching the adjacent getChannelCount) instead of leaking all-source count. CLI fix: test-postgres.ts passes ALL_SOURCES to getAllNodes/getNodeCount/ getMessageCount. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AphLfgUJWaFNAgU5UXdJ4n * fix(task-1.1): normalize undefined→ALL_SOURCES in Tier-1 method bodies Tier-1 optional sourceId methods (sourceId?: SourceScope) were passing undefined directly to withSourceScope, which now throws fail-closed. Fix: add ?? ALL_SOURCES before each withSourceScope call so omitting sourceId silently falls back to cross-source behavior (preserving backward compat for callers like purgeAllNodesAsync that call without an explicit scope). Also: - Fix existing tests for Tier-2 required methods (getAllNodes, getNodeCount, getAllChannels, getChannelCount, getAllNeighborInfo) to pass ALL_SOURCES explicitly — these tests verify cross-source counting/retrieval which is intentional. - Add ALL_SOURCES: Symbol to the database.service.test.ts mock factory so vitest does not raise "no such export" when database.ts imports it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AphLfgUJWaFNAgU5UXdJ4n * fix(task-1.1): revert ?? ALL_SOURCES from Tier-2 required method bodies Tier-2 methods (getAllNodes, getNodeCount, getChannelCount, etc.) have sourceId: SourceScope (required, no ?) — they must still throw fail-closed when undefined reaches them via 'as any' or a programming error. The previous commit incorrectly applied ?? ALL_SOURCES to these too, which caused the WP-A perSource tests to resolve instead of reject. Only Tier-1 optional methods (sourceId?: SourceScope) get ?? ALL_SOURCES in their bodies so that callers like purgeAllNodesAsync that omit the argument preserve the old all-source semantics without crashing. Full Vitest suite: success=true, 0 failed, 0 suitesFailed, 8696 tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AphLfgUJWaFNAgU5UXdJ4n * fix(task-1.1): restore fail-closed semantics — revert repo-body defaulting, fix every omit-site explicitly Orchestrator review found that commit 75ea1f7 defeated the fail-closed mechanism by adding `sourceId ?? ALL_SOURCES` inside the 7 repository files: an omitted sourceId silently became a cross-source query again. This commit restores the spec's design: 1. All 48 internal `withSourceScope(table, sourceId ?? ALL_SOURCES)` lines in the repos reverted to plain `withSourceScope(table, sourceId)` so omission propagates to the helper and THROWS at runtime. 2. Every caller surfaced by the throw (full suite + a manual sweep of all guarded Tier-1 methods' production callers) fixed explicitly at the call site: - Real sourceId where one is in scope (leak fixes): * database.ts deleteNode() -> deleteNeighborInfoInvolvingNode now scoped to the node's source (matches deleteNodeAsync) * meshtasticManager pending-DM check -> this.sourceId - Explicit ALL_SOURCES + comment where cross-source is by design: * legacy/deprecated facades in database.ts with no sourceId param (getMessagesAsync, getMessagesByChannel, getTelemetryByNodeAsync, purgeNodeTraceroutes/Telemetry, clearNeighborInfoForNode, etc.) * getRecentEstimatedPositions (est-pos pooled per #3271) * purgeAllNodesAsync global purge -> deleteAllRouteSegments * optional-source routes (server.ts unread/poll/active-nodes, v1/*, tracerouteRoutes, routeSegmentRoutes, embedPublicRoutes, telemetryRoutes) via `x ?? ALL_SOURCES` 3. Facade signatures that now carry the sentinel widened to SourceScope (getAllTraceroutes, getUnreadCountsByChannelAsync, getBatchUnreadDMCountsAsync); their raw-SQL SQLite twins normalize the symbol back to undefined so it is never bound as a query parameter. Same hazard fixed in the hand-rolled getNodeTelemetryTypes filter. 4. Test call sites updated to pass ALL_SOURCES (or a concrete id) explicitly; two mock-assertion tests now expect the sentinel. Gate: tsc clean; full Vitest suite success:true, 0 failed, 0 suites failed (8696 tests); §6 grep sweep clean (remaining bare calls are MeshCore manager methods outside the guarded hierarchy). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AphLfgUJWaFNAgU5UXdJ4n * test(db): pass ALL_SOURCES in PG-gated averaged-telemetry test (fail-closed compliance) The PostgreSQL-only test path passed undefined sourceId, which now throws under the fail-closed guard; it only runs in CI where PG is available. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AphLfgUJWaFNAgU5UXdJ4n * ci: raise Quick Tests timeout to 25 min — suite outgrew the 15-min cap again (#3385 redux) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AphLfgUJWaFNAgU5UXdJ4n --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 4e677ba commit 64081b6

54 files changed

Lines changed: 611 additions & 334 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/pr-tests.yml

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,11 @@ jobs:
2222
quick-test:
2323
name: Quick Tests
2424
runs-on: ubuntu-latest
25-
# The full unit suite (npm run test:run) now runs right at ~10 min on CI,
26-
# so a 10-min cap intermittently cancels the job at the boundary even though
27-
# every step passes. Give it headroom so the check stops flaking (#3385).
28-
timeout-minutes: 15
25+
# The full unit suite (npm run test:run) keeps growing: ~10 min when the
26+
# cap was first raised (#3385), ~14 min by 2026-07 — so the 15-min cap
27+
# started timing the job out at the boundary again (GitHub reports the
28+
# timeout as "cancelled"). Keep real headroom above the observed runtime.
29+
timeout-minutes: 25
2930

3031
services:
3132
postgres:

src/cli/test-postgres.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { createPostgresDriver } from '../db/drivers/postgres.js';
1212
import { SettingsRepository } from '../db/repositories/settings.js';
1313
import { NodesRepository } from '../db/repositories/nodes.js';
1414
import { MessagesRepository } from '../db/repositories/messages.js';
15+
import { ALL_SOURCES } from '../db/repositories/index.js';
1516

1617
const POSTGRES_URL = process.env.DATABASE_URL || 'postgres://meshmonitor:meshmonitor_dev@localhost:5432/meshmonitor';
1718

@@ -198,16 +199,16 @@ async function testNodes(nodesRepo: NodesRepository): Promise<boolean> {
198199
}
199200
console.log(' ✅ getNode works');
200201

201-
// Test getAllNodes
202-
const allNodes = await nodesRepo.getAllNodes();
202+
// Test getAllNodes (intentional cross-source: CLI test checks all sources)
203+
const allNodes = await nodesRepo.getAllNodes(ALL_SOURCES);
203204
if (allNodes.length < 1) {
204205
console.error(` ❌ getAllNodes returned ${allNodes.length} nodes, expected at least 1`);
205206
return false;
206207
}
207208
console.log(' ✅ getAllNodes works');
208209

209-
// Test getNodeCount
210-
const count = await nodesRepo.getNodeCount();
210+
// Test getNodeCount (intentional cross-source: CLI test checks all sources)
211+
const count = await nodesRepo.getNodeCount(ALL_SOURCES);
211212
if (count < 1) {
212213
console.error(` ❌ getNodeCount returned ${count}, expected at least 1`);
213214
return false;
@@ -258,8 +259,8 @@ async function testMessages(messagesRepo: MessagesRepository): Promise<boolean>
258259
}
259260
console.log(' ✅ getMessages works');
260261

261-
// Test getMessageCount
262-
const count = await messagesRepo.getMessageCount();
262+
// Test getMessageCount (intentional cross-source: CLI test checks all sources)
263+
const count = await messagesRepo.getMessageCount(ALL_SOURCES);
263264
if (count < 1) {
264265
console.error(` ❌ getMessageCount returned ${count}, expected at least 1`);
265266
return false;

src/db/repositories/base.ts

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,21 @@ export type MySQLDrizzle = MySql2Database<typeof schema>;
2020
// Union type for all database types
2121
export type DrizzleDatabase = SQLiteDrizzle | PostgresDrizzle | MySQLDrizzle;
2222

23+
/**
24+
* Explicit opt-in sentinel for the rare, intentional cross-source query.
25+
* Passing this to withSourceScope (or any scoped repo method) means
26+
* "I really do want rows from EVERY source" — e.g. system stats, global
27+
* purge, process-wide cache warm-up, full backup export.
28+
*
29+
* A unique symbol (not a magic string) so it can never arrive off the wire /
30+
* from JSON, can never collide with a real sourceId, and forces callers to
31+
* import it explicitly.
32+
*/
33+
export const ALL_SOURCES: unique symbol = Symbol('ALL_SOURCES');
34+
35+
/** A resolved source scope: a concrete sourceId, or the all-sources sentinel. */
36+
export type SourceScope = string | typeof ALL_SOURCES;
37+
2338
/**
2439
* Base repository providing common functionality
2540
*/
@@ -217,17 +232,24 @@ export abstract class BaseRepository {
217232
}
218233

219234
/**
220-
* Return a Drizzle WHERE condition that filters by sourceId.
235+
* Return a Drizzle WHERE condition that scopes a query to one source.
221236
*
222-
* Returns `undefined` when no sourceId is given — Drizzle's `and(...)` treats
223-
* undefined entries as no-ops, so existing callers that omit sourceId continue
224-
* to see all rows regardless of their source_id value.
237+
* FAIL-CLOSED: a missing/empty sourceId THROWS — the #1 hard rule
238+
* (per-source isolation) is now enforced at runtime, not by caller discipline.
239+
* For an intentional cross-source query, pass the ALL_SOURCES sentinel, which
240+
* returns `undefined` (no WHERE clause).
225241
*
226-
* Usage:
227242
* .where(and(eq(nodes.nodeNum, num), this.withSourceScope(nodes, sourceId)))
228243
*/
229-
protected withSourceScope(table: any, sourceId?: string): SQL | undefined {
230-
if (!sourceId) return undefined;
244+
protected withSourceScope(table: any, sourceId: SourceScope | undefined): SQL | undefined {
245+
if (sourceId === ALL_SOURCES) return undefined; // explicit opt-out
246+
if (sourceId === undefined || sourceId === null || sourceId === '') {
247+
throw new Error(
248+
'withSourceScope: sourceId is required. Pass a concrete sourceId, or the ' +
249+
'ALL_SOURCES sentinel for an intentional cross-source query. ' +
250+
'Omitting sourceId used to silently return rows from every source (data leak).',
251+
);
252+
}
231253
return eq(table.sourceId, sourceId);
232254
}
233255

src/db/repositories/channels.test.ts

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import { describe, it, expect, beforeEach, afterEach, afterAll, beforeAll } from 'vitest';
1212
import * as schema from '../schema/index.js';
1313
import { ChannelsRepository } from './channels.js';
14+
import { ALL_SOURCES } from './base.js';
1415
import {
1516
TestBackend,
1617
createPostgresBackend,
@@ -270,7 +271,7 @@ function runChannelsTests(getBackend: () => TestBackend) {
270271
await repo.upsertChannel({ id: 0, name: 'Primary', psk: 'p0', role: 1 });
271272
await repo.upsertChannel({ id: 1, name: 'One', psk: 'p1', role: 2 });
272273

273-
const channels = await repo.getAllChannels();
274+
const channels = await repo.getAllChannels(ALL_SOURCES);
274275
expect(channels.length).toBe(3);
275276
expect(channels[0].id).toBe(0);
276277
expect(channels[1].id).toBe(1);
@@ -337,7 +338,7 @@ function runChannelsTests(getBackend: () => TestBackend) {
337338
expect(repo.getChannelCountSync('src-a')).toBe(2);
338339
expect(repo.getChannelCountSync('src-b')).toBe(1);
339340
// Unscoped sees everything (legacy behaviour).
340-
expect(repo.getChannelCountSync()).toBe(3);
341+
expect(repo.getChannelCountSync(ALL_SOURCES)).toBe(3);
341342
});
342343

343344
it('getChannelCount - returns correct count', async () => {
@@ -347,13 +348,13 @@ function runChannelsTests(getBackend: () => TestBackend) {
347348
return;
348349
}
349350

350-
expect(await repo.getChannelCount()).toBe(0);
351+
expect(await repo.getChannelCount(ALL_SOURCES)).toBe(0);
351352

352353
await repo.upsertChannel({ id: 0, name: 'Primary', psk: 'p0', role: 1 });
353-
expect(await repo.getChannelCount()).toBe(1);
354+
expect(await repo.getChannelCount(ALL_SOURCES)).toBe(1);
354355

355356
await repo.upsertChannel({ id: 1, name: 'Secondary', psk: 'p1', role: 2 });
356-
expect(await repo.getChannelCount()).toBe(2);
357+
expect(await repo.getChannelCount(ALL_SOURCES)).toBe(2);
357358
});
358359

359360
it('deleteChannel - removes a channel by ID', async () => {
@@ -370,7 +371,7 @@ function runChannelsTests(getBackend: () => TestBackend) {
370371

371372
expect(await repo.getChannelById(1)).toBeNull();
372373
expect(await repo.getChannelById(0)).not.toBeNull();
373-
expect(await repo.getChannelCount()).toBe(1);
374+
expect(await repo.getChannelCount(ALL_SOURCES)).toBe(1);
374375
});
375376

376377
it('deleteChannel - no-op for non-existent channel', async () => {
@@ -382,7 +383,7 @@ function runChannelsTests(getBackend: () => TestBackend) {
382383

383384
await repo.upsertChannel({ id: 0, name: 'Primary', psk: 'p0', role: 1 });
384385
await repo.deleteChannel(99);
385-
expect(await repo.getChannelCount()).toBe(1);
386+
expect(await repo.getChannelCount(ALL_SOURCES)).toBe(1);
386387
});
387388

388389
it('cleanupInvalidChannels - removes channels outside 0-7 range', async () => {
@@ -403,11 +404,11 @@ function runChannelsTests(getBackend: () => TestBackend) {
403404
await backend2.exec(`INSERT INTO channels (id, name, psk, role, ${cat}, ${uat}) VALUES (8, 'Invalid8', 'psk', 2, 0, 0)`);
404405
await backend2.exec(`INSERT INTO channels (id, name, psk, role, ${cat}, ${uat}) VALUES (100, 'Invalid100', 'psk', 2, 0, 0)`);
405406

406-
expect(await repo.getChannelCount()).toBe(5);
407+
expect(await repo.getChannelCount(ALL_SOURCES)).toBe(5);
407408

408409
const deleted = await repo.cleanupInvalidChannels();
409410
expect(deleted).toBe(2);
410-
expect(await repo.getChannelCount()).toBe(3);
411+
expect(await repo.getChannelCount(ALL_SOURCES)).toBe(3);
411412
expect(await repo.getChannelById(8)).toBeNull();
412413
expect(await repo.getChannelById(100)).toBeNull();
413414
});
@@ -424,7 +425,7 @@ function runChannelsTests(getBackend: () => TestBackend) {
424425

425426
const deleted = await repo.cleanupInvalidChannels();
426427
expect(deleted).toBe(0);
427-
expect(await repo.getChannelCount()).toBe(2);
428+
expect(await repo.getChannelCount(ALL_SOURCES)).toBe(2);
428429
});
429430

430431
it('cleanupInvalidChannels - preserves out-of-range channels owned by a MeshCore source', async () => {
@@ -455,7 +456,7 @@ function runChannelsTests(getBackend: () => TestBackend) {
455456
// A legacy NULL-sourceId channel at idx 9 (implicitly Meshtastic; should be removed).
456457
await backend.exec(`INSERT INTO channels (id, name, psk, ${cat}, ${uat}) VALUES (9, 'Legacy-Nine', 'aGVsbG8=', 0, 0)`);
457458

458-
expect(await repo.getChannelCount()).toBe(3);
459+
expect(await repo.getChannelCount(ALL_SOURCES)).toBe(3);
459460

460461
const deleted = await repo.cleanupInvalidChannels();
461462
expect(deleted).toBe(2);
@@ -492,11 +493,11 @@ function runChannelsTests(getBackend: () => TestBackend) {
492493
await backend2.exec(`INSERT INTO channels (id, name, psk, role, ${cat}, ${uat}) VALUES (3, 'Empty3', NULL, NULL, 0, 0)`);
493494
await backend2.exec(`INSERT INTO channels (id, name, psk, role, ${cat}, ${uat}) VALUES (4, 'Empty4', NULL, NULL, 0, 0)`);
494495

495-
expect(await repo.getChannelCount()).toBe(5);
496+
expect(await repo.getChannelCount(ALL_SOURCES)).toBe(5);
496497

497498
const deleted = await repo.cleanupEmptyChannels();
498499
expect(deleted).toBe(2);
499-
expect(await repo.getChannelCount()).toBe(3);
500+
expect(await repo.getChannelCount(ALL_SOURCES)).toBe(3);
500501
expect(await repo.getChannelById(3)).toBeNull();
501502
expect(await repo.getChannelById(4)).toBeNull();
502503
// Protected channels still exist
@@ -521,7 +522,7 @@ function runChannelsTests(getBackend: () => TestBackend) {
521522

522523
const deleted = await repo.cleanupEmptyChannels();
523524
expect(deleted).toBe(0);
524-
expect(await repo.getChannelCount()).toBe(2);
525+
expect(await repo.getChannelCount(ALL_SOURCES)).toBe(2);
525526
});
526527

527528
it('cleanupEmptyChannels - returns 0 when no empty channels', async () => {

src/db/repositories/channels.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* Supports SQLite, PostgreSQL, and MySQL through Drizzle ORM.
66
*/
77
import { eq, and, gt, isNull, or, lt, count, notInArray } from 'drizzle-orm';
8-
import { BaseRepository, DrizzleDatabase } from './base.js';
8+
import { BaseRepository, DrizzleDatabase, SourceScope } from './base.js';
99
import { DatabaseType, DbChannel } from '../types.js';
1010
import { logger } from '../../utils/logger.js';
1111

@@ -64,7 +64,7 @@ export class ChannelsRepository extends BaseRepository {
6464
/**
6565
* Get all channels ordered by ID, optionally scoped to a source.
6666
*/
67-
async getAllChannels(sourceId?: string): Promise<DbChannel[]> {
67+
async getAllChannels(sourceId: SourceScope): Promise<DbChannel[]> {
6868
const { channels } = this.tables;
6969
const result = await this.db
7070
.select()
@@ -78,7 +78,7 @@ export class ChannelsRepository extends BaseRepository {
7878
/**
7979
* Get the total number of channels, optionally scoped to a source.
8080
*/
81-
async getChannelCount(sourceId?: string): Promise<number> {
81+
async getChannelCount(sourceId: SourceScope): Promise<number> {
8282
const { channels } = this.tables;
8383
const whereClause = this.withSourceScope(channels, sourceId);
8484
const result = whereClause
@@ -339,7 +339,7 @@ export class ChannelsRepository extends BaseRepository {
339339
* Synchronously get all channels (SQLite only), optionally scoped to a
340340
* source (#3712).
341341
*/
342-
getAllChannelsSync(sourceId?: string): DbChannel[] {
342+
getAllChannelsSync(sourceId?: SourceScope): DbChannel[] {
343343
const db = this.getSqliteDb();
344344
const { channels } = this.tables;
345345
const rows = db
@@ -355,13 +355,11 @@ export class ChannelsRepository extends BaseRepository {
355355
* Synchronously count channels (SQLite only), optionally scoped to a
356356
* source (#3712).
357357
*/
358-
getChannelCountSync(sourceId?: string): number {
358+
getChannelCountSync(sourceId?: SourceScope): number {
359359
const db = this.getSqliteDb();
360360
const { channels } = this.tables;
361361
const whereClause = this.withSourceScope(channels, sourceId);
362-
const rows = whereClause
363-
? db.select({ count: count() }).from(channels).where(whereClause).all()
364-
: db.select({ count: count() }).from(channels).all();
362+
const rows = db.select({ count: count() }).from(channels).where(whereClause).all();
365363
return Number((rows[0] as any).count);
366364
}
367365

src/db/repositories/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
* Central export point for all repository classes.
55
*/
66

7-
export { BaseRepository } from './base.js';
8-
export type { DrizzleDatabase, SQLiteDrizzle, PostgresDrizzle } from './base.js';
7+
export { BaseRepository, ALL_SOURCES } from './base.js';
8+
export type { DrizzleDatabase, SQLiteDrizzle, PostgresDrizzle, SourceScope } from './base.js';
99
export { SettingsRepository } from './settings.js';
1010
export { ChannelsRepository, type ChannelInput } from './channels.js';
1111
export { NodesRepository, type NodesCacheHook } from './nodes.js';

src/db/repositories/messages.createdAt-ordering.test.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest';
1010
import Database from 'better-sqlite3';
1111
import { BetterSQLite3Database } from 'drizzle-orm/better-sqlite3';
1212
import { MessagesRepository } from './messages.js';
13+
import { ALL_SOURCES } from './base.js';
1314
import * as schema from '../schema/index.js';
1415
import { createTestDb } from '../../server/test-helpers/testDb.js';
1516

@@ -75,7 +76,7 @@ describe('MessagesRepository — createdAt ordering (#3122)', () => {
7576
insert('real-newer-1', { rxTime: 2000, createdAt: 2000 });
7677
insert('real-newer-2', { rxTime: 3000, createdAt: 3000 });
7778

78-
const rows = await repo.getMessages(10);
79+
const rows = await repo.getMessages(10, 0, ALL_SOURCES);
7980
expect(rows.map((r: any) => r.id)).toEqual(['real-newer-2', 'real-newer-1', 'future-but-old']);
8081
});
8182

@@ -84,7 +85,7 @@ describe('MessagesRepository — createdAt ordering (#3122)', () => {
8485
insert('actual-new', { channel: 0, rxTime: 500, createdAt: 200 });
8586
insert('other-channel', { channel: 5, rxTime: 999, createdAt: 999 });
8687

87-
const rows = await repo.getMessagesByChannel(0, 10);
88+
const rows = await repo.getMessagesByChannel(0, 10, 0, ALL_SOURCES);
8889
expect(rows.map((r: any) => r.id)).toEqual(['actual-new', 'skewed']);
8990
});
9091

@@ -98,23 +99,23 @@ describe('MessagesRepository — createdAt ordering (#3122)', () => {
9899
// Cursor "before" = 250 should pull rows whose createdAt < 250.
99100
// Under the old rxTime cursor 'skewed' would be excluded (rxTime > 250).
100101
// Under createdAt 'skewed' is included.
101-
const rows = await repo.getMessagesBeforeInChannel(0, 250, 10);
102+
const rows = await repo.getMessagesBeforeInChannel(0, 250, 10, ALL_SOURCES);
102103
expect(rows.map((r: any) => r.id)).toEqual(['b', 'skewed', 'a']);
103104
});
104105

105106
it('getMessagesSqlite (sync variant) orders by createdAt', () => {
106107
insert('skewed', { rxTime: FAR_FUTURE, createdAt: 100 });
107108
insert('newer', { rxTime: 50, createdAt: 200 });
108109

109-
const rows = repo.getMessagesSqlite(10);
110+
const rows = repo.getMessagesSqlite(10, 0, ALL_SOURCES);
110111
expect(rows.map((r: any) => r.id)).toEqual(['newer', 'skewed']);
111112
});
112113

113114
it('getMessagesByChannelSqlite (sync variant) orders by createdAt', () => {
114115
insert('skewed', { channel: 2, rxTime: FAR_FUTURE, createdAt: 100 });
115116
insert('newer', { channel: 2, rxTime: 50, createdAt: 200 });
116117

117-
const rows = repo.getMessagesByChannelSqlite(2, 10);
118+
const rows = repo.getMessagesByChannelSqlite(2, 10, 0, ALL_SOURCES);
118119
expect(rows.map((r: any) => r.id)).toEqual(['newer', 'skewed']);
119120
});
120121
});

src/db/repositories/messages.exclude-portnums.test.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest';
1111
import Database from 'better-sqlite3';
1212
import { BetterSQLite3Database } from 'drizzle-orm/better-sqlite3';
1313
import { MessagesRepository } from './messages.js';
14+
import { ALL_SOURCES } from './base.js';
1415
import * as schema from '../schema/index.js';
1516
import { createTestDb } from '../../server/test-helpers/testDb.js';
1617

@@ -59,18 +60,18 @@ describe('MessagesRepository.getMessages excludePortnums', () => {
5960
insert('trace-1', 70, 2000);
6061
insert('text-2', 1, 3000);
6162

62-
const all = await repo.getMessages(100);
63+
const all = await repo.getMessages(100, 0, ALL_SOURCES);
6364
expect(all).toHaveLength(3);
6465

65-
const filtered = await repo.getMessages(100, 0, undefined, [70]);
66+
const filtered = await repo.getMessages(100, 0, ALL_SOURCES, [70]);
6667
expect(filtered.map(m => m.id).sort()).toEqual(['text-1', 'text-2']);
6768
});
6869

6970
it('keeps rows whose portnum is NULL (legacy data predates the column)', async () => {
7071
insert('legacy', null, 1000);
7172
insert('trace', 70, 2000);
7273

73-
const filtered = await repo.getMessages(100, 0, undefined, [70]);
74+
const filtered = await repo.getMessages(100, 0, ALL_SOURCES, [70]);
7475
expect(filtered.map(m => m.id)).toEqual(['legacy']);
7576
});
7677

@@ -83,21 +84,21 @@ describe('MessagesRepository.getMessages excludePortnums', () => {
8384

8485
// Without the filter, limit=3 returns the 3 newest — which now includes
8586
// the traceroute and drops dm-old. This is the bug.
86-
const unfiltered = await repo.getMessages(3);
87+
const unfiltered = await repo.getMessages(3, 0, ALL_SOURCES);
8788
expect(unfiltered.map(m => m.id)).toContain('trace');
8889
expect(unfiltered.map(m => m.id)).not.toContain('dm-old');
8990

9091
// With the filter, all 3 DMs survive the same capped window.
91-
const filtered = await repo.getMessages(3, 0, undefined, [70]);
92+
const filtered = await repo.getMessages(3, 0, ALL_SOURCES, [70]);
9293
expect(filtered.map(m => m.id).sort()).toEqual(['dm-mid', 'dm-old', 'dm-recent']);
9394
});
9495

9596
it('is a no-op when excludePortnums is empty or omitted', async () => {
9697
insert('a', 1, 1000);
9798
insert('b', 70, 2000);
9899

99-
const omitted = await repo.getMessages(100);
100-
const empty = await repo.getMessages(100, 0, undefined, []);
100+
const omitted = await repo.getMessages(100, 0, ALL_SOURCES);
101+
const empty = await repo.getMessages(100, 0, ALL_SOURCES, []);
101102

102103
expect(omitted.map(m => m.id).sort()).toEqual(['a', 'b']);
103104
expect(empty.map(m => m.id).sort()).toEqual(['a', 'b']);

0 commit comments

Comments
 (0)