From f33ed4c49ef0af7732cf10425241b2890eea014f Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Fri, 13 Feb 2026 02:16:22 -0800 Subject: [PATCH] fix: clamp getAllSessions pagination params, add cleanupAliases success field, add 10 tests - session-manager: clamp offset/limit to safe non-negative integers to prevent negative offset counting from end and NaN returning empty results - session-aliases: add success field to cleanupAliases return value for API contract consistency with setAlias/deleteAlias/renameAlias --- scripts/lib/session-aliases.js | 8 ++++ scripts/lib/session-manager.js | 13 ++++++- tests/lib/session-aliases.test.js | 17 +++++++++ tests/lib/session-manager.test.js | 61 +++++++++++++++++++++++++++++++ 4 files changed, 97 insertions(+), 2 deletions(-) diff --git a/scripts/lib/session-aliases.js b/scripts/lib/session-aliases.js index 590c6ce1..f5171458 100644 --- a/scripts/lib/session-aliases.js +++ b/scripts/lib/session-aliases.js @@ -446,9 +446,17 @@ function cleanupAliases(sessionExists) { if (removed.length > 0 && !saveAliases(data)) { log('[Aliases] Failed to save after cleanup'); + return { + success: false, + totalChecked: Object.keys(data.aliases).length + removed.length, + removed: removed.length, + removedAliases: removed, + error: 'Failed to save after cleanup' + }; } return { + success: true, totalChecked: Object.keys(data.aliases).length + removed.length, removed: removed.length, removedAliases: removed diff --git a/scripts/lib/session-manager.js b/scripts/lib/session-manager.js index f3507835..9e6e50b5 100644 --- a/scripts/lib/session-manager.js +++ b/scripts/lib/session-manager.js @@ -189,12 +189,21 @@ function getSessionStats(sessionPathOrContent) { */ function getAllSessions(options = {}) { const { - limit = 50, - offset = 0, + limit: rawLimit = 50, + offset: rawOffset = 0, date = null, search = null } = options; + // Clamp offset and limit to safe non-negative integers. + // Without this, negative offset causes slice() to count from the end, + // and NaN values cause slice() to return empty or unexpected results. + // Note: cannot use `|| default` because 0 is falsy — use isNaN instead. + const offsetNum = Number(rawOffset); + const offset = Number.isNaN(offsetNum) ? 0 : Math.max(0, Math.floor(offsetNum)); + const limitNum = Number(rawLimit); + const limit = Number.isNaN(limitNum) ? 50 : Math.max(1, Math.floor(limitNum)); + const sessionsDir = getSessionsDir(); if (!fs.existsSync(sessionsDir)) { diff --git a/tests/lib/session-aliases.test.js b/tests/lib/session-aliases.test.js index 95c0c3b0..9debbc33 100644 --- a/tests/lib/session-aliases.test.js +++ b/tests/lib/session-aliases.test.js @@ -561,9 +561,26 @@ function runTests() { assert.strictEqual(remaining.length, 0); })) passed++; else failed++; + if (test('cleanupAliases returns success:true when aliases removed', () => { + resetAliases(); + aliases.setAlias('dead', '/sessions/dead'); + const result = aliases.cleanupAliases(() => false); + assert.strictEqual(result.success, true); + assert.strictEqual(result.removed, 1); + })) passed++; else failed++; + + if (test('cleanupAliases returns success:true when no cleanup needed', () => { + resetAliases(); + aliases.setAlias('alive', '/sessions/alive'); + const result = aliases.cleanupAliases(() => true); + assert.strictEqual(result.success, true); + assert.strictEqual(result.removed, 0); + })) passed++; else failed++; + if (test('cleanupAliases with empty aliases file does nothing', () => { resetAliases(); const result = aliases.cleanupAliases(() => true); + assert.strictEqual(result.success, true); assert.strictEqual(result.removed, 0); assert.strictEqual(result.totalChecked, 0); assert.strictEqual(result.removedAliases.length, 0); diff --git a/tests/lib/session-manager.test.js b/tests/lib/session-manager.test.js index 075d05b6..0a2c7d28 100644 --- a/tests/lib/session-manager.test.js +++ b/tests/lib/session-manager.test.js @@ -754,6 +754,67 @@ src/main.ts } })) passed++; else failed++; + // getAllSessions pagination edge cases (offset/limit clamping) + console.log('\ngetAllSessions (pagination edge cases):'); + + if (test('getAllSessions clamps negative offset to 0', () => { + const result = sessionManager.getAllSessions({ offset: -5, limit: 2 }); + // Negative offset should be clamped to 0, returning the first 2 sessions + assert.strictEqual(result.sessions.length, 2); + assert.strictEqual(result.offset, 0); + assert.strictEqual(result.total, 5); + })) passed++; else failed++; + + if (test('getAllSessions clamps NaN offset to 0', () => { + const result = sessionManager.getAllSessions({ offset: NaN, limit: 3 }); + assert.strictEqual(result.sessions.length, 3); + assert.strictEqual(result.offset, 0); + })) passed++; else failed++; + + if (test('getAllSessions clamps NaN limit to default', () => { + const result = sessionManager.getAllSessions({ offset: 0, limit: NaN }); + // NaN limit should be clamped to default (50), returning all 5 sessions + assert.ok(result.sessions.length > 0); + assert.strictEqual(result.total, 5); + })) passed++; else failed++; + + if (test('getAllSessions clamps negative limit to 1', () => { + const result = sessionManager.getAllSessions({ offset: 0, limit: -10 }); + // Negative limit should be clamped to 1 + assert.strictEqual(result.sessions.length, 1); + assert.strictEqual(result.limit, 1); + })) passed++; else failed++; + + if (test('getAllSessions clamps zero limit to 1', () => { + const result = sessionManager.getAllSessions({ offset: 0, limit: 0 }); + assert.strictEqual(result.sessions.length, 1); + assert.strictEqual(result.limit, 1); + })) passed++; else failed++; + + if (test('getAllSessions handles string offset/limit gracefully', () => { + const result = sessionManager.getAllSessions({ offset: 'abc', limit: 'xyz' }); + // String non-numeric should be treated as 0/default + assert.strictEqual(result.offset, 0); + assert.ok(result.sessions.length > 0); + })) passed++; else failed++; + + if (test('getAllSessions handles fractional offset (floors to integer)', () => { + const result = sessionManager.getAllSessions({ offset: 1.7, limit: 2 }); + // 1.7 should floor to 1, skip first session, return next 2 + assert.strictEqual(result.offset, 1); + assert.strictEqual(result.sessions.length, 2); + })) passed++; else failed++; + + if (test('getAllSessions handles Infinity offset', () => { + // Infinity should clamp to 0 since Number(Infinity) is Infinity but + // Math.floor(Infinity) is Infinity — however slice(Infinity) returns [] + // Actually: Number(Infinity) || 0 = Infinity, Math.floor(Infinity) = Infinity + // Math.max(0, Infinity) = Infinity, so slice(Infinity) = [] + const result = sessionManager.getAllSessions({ offset: Infinity, limit: 2 }); + assert.strictEqual(result.sessions.length, 0); + assert.strictEqual(result.total, 5); + })) passed++; else failed++; + // getSessionStats with code blocks and special characters console.log('\ngetSessionStats (code blocks & special chars):');