diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f3c54db1387..64ef4acda82 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -282,6 +282,12 @@ jobs: if: "${{ needs.classify_pr.outputs.skip_ci != 'true' && steps.ci_profile.outputs.ci_profile == 'full' }}" run: 'npm run check:desktop-isolation' + - name: 'Run desktop session-tools-core tests' + if: "${{ needs.classify_pr.outputs.skip_ci != 'true' && steps.ci_profile.outputs.ci_profile == 'full' }}" + run: |- + npm exec --yes bun@1.3.14 -- test \ + packages/desktop/packages/session-tools-core/src/handlers/list-sessions.test.ts + - name: 'Install linters' if: "${{ needs.classify_pr.outputs.skip_ci != 'true' && steps.ci_profile.outputs.ci_profile == 'full' }}" run: 'node scripts/lint.js --setup' diff --git a/packages/desktop/packages/session-tools-core/src/handlers/list-sessions.test.ts b/packages/desktop/packages/session-tools-core/src/handlers/list-sessions.test.ts new file mode 100644 index 00000000000..92250c91904 --- /dev/null +++ b/packages/desktop/packages/session-tools-core/src/handlers/list-sessions.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from 'bun:test'; +import { handleListSessions } from './list-sessions.ts'; +import type { ListSessionsOptions, SessionToolContext } from '../context.ts'; + +function createCtx( + onListSessions: (options?: ListSessionsOptions) => void, +): SessionToolContext { + return { + listSessions: (options?: ListSessionsOptions) => { + onListSessions(options); + return { + total: 1, + returned: 1, + sessions: [ + { + id: 'session-123', + name: 'Example', + labels: [], + status: 'todo', + createdAt: 1, + }, + ], + }; + }, + } as unknown as SessionToolContext; +} + +describe('handleListSessions', () => { + it('rejects malformed pagination values before listing sessions', async () => { + const cases: Array<{ args: ListSessionsOptions; message: string }> = [ + { args: { limit: 0 }, message: 'limit must be a positive integer.' }, + { args: { limit: -1 }, message: 'limit must be a positive integer.' }, + { args: { limit: 1.5 }, message: 'limit must be a positive integer.' }, + { + args: { offset: -1 }, + message: 'offset must be a non-negative integer.', + }, + { + args: { offset: 1.5 }, + message: 'offset must be a non-negative integer.', + }, + ]; + + for (const { args, message } of cases) { + const calls: Array = []; + const ctx = createCtx((options) => calls.push(options)); + + const result = await handleListSessions(ctx, args); + + expect(result.isError).toBe(true); + expect(result.content[0]?.text).toContain(message); + expect(calls).toEqual([]); + } + }); + + it('accepts minimum valid pagination boundaries', async () => { + const calls: Array = []; + const ctx = createCtx((options) => calls.push(options)); + + const result = await handleListSessions(ctx, { limit: 1, offset: 0 }); + + expect(result.isError).toBe(false); + expect(calls).toEqual([{ limit: 1, offset: 0 }]); + }); + + it('passes valid pagination values through to the session lister', async () => { + const calls: Array = []; + const ctx = createCtx((options) => calls.push(options)); + + const result = await handleListSessions(ctx, { + status: 'todo', + limit: 2, + offset: 1, + }); + + expect(result.isError).toBe(false); + expect(calls).toEqual([{ status: 'todo', limit: 2, offset: 1 }]); + }); + + it('preserves high integer limits for the backend clamp', async () => { + const calls: Array = []; + const ctx = createCtx((options) => calls.push(options)); + + const result = await handleListSessions(ctx, { limit: 101 }); + + expect(result.isError).toBe(false); + expect(calls).toEqual([{ limit: 101 }]); + }); +}); diff --git a/packages/desktop/packages/session-tools-core/src/handlers/list-sessions.ts b/packages/desktop/packages/session-tools-core/src/handlers/list-sessions.ts index 7eebd1567d5..a0e7675589c 100644 --- a/packages/desktop/packages/session-tools-core/src/handlers/list-sessions.ts +++ b/packages/desktop/packages/session-tools-core/src/handlers/list-sessions.ts @@ -13,12 +13,26 @@ export interface ListSessionsArgs { export async function handleListSessions( ctx: SessionToolContext, - args: ListSessionsArgs + args: ListSessionsArgs, ): Promise { if (!ctx.listSessions) { return errorResponse('list_sessions is not available in this context.'); } + if ( + args.limit !== undefined && + (!Number.isInteger(args.limit) || args.limit < 1) + ) { + return errorResponse('limit must be a positive integer.'); + } + + if ( + args.offset !== undefined && + (!Number.isInteger(args.offset) || args.offset < 0) + ) { + return errorResponse('offset must be a non-negative integer.'); + } + try { const result = ctx.listSessions({ status: args.status, diff --git a/packages/desktop/packages/session-tools-core/src/tool-defs-filtering.test.ts b/packages/desktop/packages/session-tools-core/src/tool-defs-filtering.test.ts index d4101d506f7..aae9cae30e8 100644 --- a/packages/desktop/packages/session-tools-core/src/tool-defs-filtering.test.ts +++ b/packages/desktop/packages/session-tools-core/src/tool-defs-filtering.test.ts @@ -43,6 +43,19 @@ describe('session tool filtering helpers', () => { expect(names.includes('send_developer_feedback')).toBe(false); }); + it('json schema exposes list_sessions pagination controls as integers', () => { + const defs = getToolDefsAsJsonSchema({ includeDeveloperFeedback: false }); + const listSessions = defs.find(d => d.name === 'list_sessions'); + const properties = ( + listSessions?.inputSchema as + | { properties?: Record } + | undefined + )?.properties; + + expect(properties?.limit?.type).toBe('integer'); + expect(properties?.offset?.type).toBe('integer'); + }); + it('all canonical session tools declare safeMode metadata', () => { for (const def of SESSION_TOOL_DEFS) { expect(def.safeMode === 'allow' || def.safeMode === 'block').toBe(true); diff --git a/packages/desktop/packages/session-tools-core/src/tool-defs.ts b/packages/desktop/packages/session-tools-core/src/tool-defs.ts index 5ee64ea56de..78aa699e530 100644 --- a/packages/desktop/packages/session-tools-core/src/tool-defs.ts +++ b/packages/desktop/packages/session-tools-core/src/tool-defs.ts @@ -199,8 +199,8 @@ export const ListSessionsSchema = z.object({ label: z.string().optional().describe('Filter by label'), search: z.string().optional().describe('Substring match on session name'), sortBy: z.enum(['recent', 'name', 'status']).optional().describe('Sort order (default: recent)'), - limit: z.number().optional().describe('Max sessions to return (default 20, max 100)'), - offset: z.number().optional().describe('Skip first N results (for pagination)'), + limit: z.number().int().min(1).optional().describe('Max sessions to return (default 20, max 100)'), + offset: z.number().int().min(0).optional().describe('Skip first N results (for pagination)'), }); // Inter-session messaging