diff --git a/backend/src/__tests__/action-transform.integration.test.ts b/backend/src/__tests__/action-transform.integration.test.ts index 40ee1c2..f93c4cf 100644 --- a/backend/src/__tests__/action-transform.integration.test.ts +++ b/backend/src/__tests__/action-transform.integration.test.ts @@ -55,7 +55,7 @@ describe('Action transformation integration', () => { afterEach(async () => { await conversationService._clear(); - actionService._clear(); + await actionService._clear(); }); function connectWs(): Promise { @@ -109,7 +109,7 @@ describe('Action transformation integration', () => { it('should transform pendingAction fields in GET /api/actions/pending', async () => { // Create an action via the service directly - actionService.createAction( + await actionService.createAction( 'transform-test-user', 'test-conv-id', 'create_event', @@ -144,7 +144,7 @@ describe('Action transformation integration', () => { }); it('should transform action fields in reject response', async () => { - const created = actionService.createAction( + const created = await actionService.createAction( 'transform-test-user', 'test-conv-id', 'delete_event', @@ -167,7 +167,7 @@ describe('Action transformation integration', () => { it('should reject action for unauthorized user', async () => { // Create an action for a different user - const created = actionService.createAction( + const created = await actionService.createAction( 'other-user-id', 'test-conv-id', 'delete_event', @@ -182,7 +182,7 @@ describe('Action transformation integration', () => { }); it('should transform action fields in approve response', async () => { - const created = actionService.createAction( + const created = await actionService.createAction( 'transform-test-user', 'test-conv-id', 'create_event', diff --git a/backend/src/__tests__/approval-modal-crash.integration.test.ts b/backend/src/__tests__/approval-modal-crash.integration.test.ts index 3b97a13..8c0722b 100644 --- a/backend/src/__tests__/approval-modal-crash.integration.test.ts +++ b/backend/src/__tests__/approval-modal-crash.integration.test.ts @@ -54,11 +54,11 @@ describe('Approval modal crash fix integration', () => { afterEach(async () => { await conversationService._clear(); - actionService._clear(); + await actionService._clear(); }); - it('toFrontendAction should always produce details as an object', () => { - const action = actionService.createAction( + it('toFrontendAction should always produce details as an object', async () => { + const action = await actionService.createAction( 'approval-modal-test-user', 'conv-1', 'create_event', @@ -81,8 +81,8 @@ describe('Approval modal crash fix integration', () => { expect(frontend).not.toHaveProperty('actionType'); }); - it('toFrontendAction should produce valid details for delete actions', () => { - const action = actionService.createAction( + it('toFrontendAction should produce valid details for delete actions', async () => { + const action = await actionService.createAction( 'approval-modal-test-user', 'conv-1', 'delete_event', @@ -98,8 +98,8 @@ describe('Approval modal crash fix integration', () => { expect(frontend.type).toBe('delete_event'); }); - it('toFrontendAction should produce valid details for update actions', () => { - const action = actionService.createAction( + it('toFrontendAction should produce valid details for update actions', async () => { + const action = await actionService.createAction( 'approval-modal-test-user', 'conv-1', 'update_event', @@ -120,7 +120,7 @@ describe('Approval modal crash fix integration', () => { }); it('GET /api/actions/pending returns details field (not params) for the modal', async () => { - actionService.createAction( + await actionService.createAction( 'approval-modal-test-user', 'conv-1', 'create_event', @@ -154,7 +154,7 @@ describe('Approval modal crash fix integration', () => { }); it('GET /api/actions/pending returns details for delete action with calendarId and eventId', async () => { - actionService.createAction( + await actionService.createAction( 'approval-modal-test-user', 'conv-1', 'delete_event', diff --git a/backend/src/__tests__/approval-ux.integration.test.ts b/backend/src/__tests__/approval-ux.integration.test.ts index 989f009..64a8ddc 100644 --- a/backend/src/__tests__/approval-ux.integration.test.ts +++ b/backend/src/__tests__/approval-ux.integration.test.ts @@ -53,7 +53,7 @@ describe('Approval UX Integration', () => { afterEach(async () => { await conversationService._clear(); - actionService._clear(); + await actionService._clear(); }); function connectWs(): Promise { @@ -168,7 +168,7 @@ describe('Approval UX Integration', () => { const conv = await conversationService.createConversation('approval-test-user', 'Test'); // Create a pending action directly - const action = actionService.createAction( + const action = await actionService.createAction( 'approval-test-user', conv.id, 'create_event', diff --git a/backend/src/__tests__/chat.integration.test.ts b/backend/src/__tests__/chat.integration.test.ts index 665abe1..7947bb6 100644 --- a/backend/src/__tests__/chat.integration.test.ts +++ b/backend/src/__tests__/chat.integration.test.ts @@ -52,7 +52,7 @@ describe('Chat Integration (real agent)', () => { afterEach(async () => { await conversationService._clear(); - actionService._clear(); + await actionService._clear(); }); function connectWs(): Promise { diff --git a/backend/src/__tests__/chat.test.ts b/backend/src/__tests__/chat.test.ts index 2b93c36..476bf43 100644 --- a/backend/src/__tests__/chat.test.ts +++ b/backend/src/__tests__/chat.test.ts @@ -65,7 +65,7 @@ describe('Chat & HITL API', () => { afterEach(async () => { await conversationService._clear(); - actionService._clear(); + await actionService._clear(); }); function connectWs(cookie?: string): Promise { @@ -261,7 +261,7 @@ describe('Chat & HITL API', () => { }); it('should reject a pending action', async () => { - const action = actionService.createAction( + const action = await actionService.createAction( 'test-user-1', 'conv-1', 'create_event', @@ -281,7 +281,7 @@ describe('Chat & HITL API', () => { }); it('should return 409 when rejecting an already rejected action', async () => { - const action = actionService.createAction( + const action = await actionService.createAction( 'test-user-1', 'conv-1', 'create_event', diff --git a/backend/src/__tests__/conversationHistory.integration.test.ts b/backend/src/__tests__/conversationHistory.integration.test.ts index 49abbdb..a8bc651 100644 --- a/backend/src/__tests__/conversationHistory.integration.test.ts +++ b/backend/src/__tests__/conversationHistory.integration.test.ts @@ -50,9 +50,9 @@ describe('Conversation History Integration', () => { await new Promise((resolve) => server.close(() => resolve())); }, 15000); - afterEach(() => { + afterEach(async () => { conversationService._clear(); - actionService._clear(); + await actionService._clear(); }); function connectWs(): Promise { diff --git a/backend/src/__tests__/database.integration.test.ts b/backend/src/__tests__/database.integration.test.ts index 5d70afb..26a36be 100644 --- a/backend/src/__tests__/database.integration.test.ts +++ b/backend/src/__tests__/database.integration.test.ts @@ -78,9 +78,11 @@ describe('Database Integration', () => { // 5. Create pending action const action = await pendingActionRepo.create({ + user_id: user.id, conversation_id: conv.id, action_type: 'create_event', action_payload: { summary: 'Meeting', startDateTime: '2026-01-30T10:00:00Z' }, + description: 'Create a meeting', }); expect(action.status).toBe('pending'); @@ -131,9 +133,9 @@ describe('Database Integration', () => { await userRepo.upsert({ id: 'user-1', email: 'u@test.com', display_name: 'U' }); const conv = await conversationRepo.create('user-1', 'Action Test Conv'); - const a1 = await pendingActionRepo.create({ conversation_id: conv.id, action_type: 'create_event', action_payload: { summary: 'Event 1' } }); - const a2 = await pendingActionRepo.create({ conversation_id: conv.id, action_type: 'delete_event', action_payload: { eventId: 'e1' } }); - const a3 = await pendingActionRepo.create({ conversation_id: conv.id, action_type: 'update_event', action_payload: { eventId: 'e2' } }); + const a1 = await pendingActionRepo.create({ user_id: 'user-1', conversation_id: conv.id, action_type: 'create_event', action_payload: { summary: 'Event 1' }, description: 'Create event 1' }); + const a2 = await pendingActionRepo.create({ user_id: 'user-1', conversation_id: conv.id, action_type: 'delete_event', action_payload: { eventId: 'e1' }, description: 'Delete event' }); + const a3 = await pendingActionRepo.create({ user_id: 'user-1', conversation_id: conv.id, action_type: 'update_event', action_payload: { eventId: 'e2' }, description: 'Update event' }); // Approve first, reject second, leave third pending await pendingActionRepo.updateStatus(a1.id, 'approved'); diff --git a/backend/src/__tests__/database.test.ts b/backend/src/__tests__/database.test.ts index 1cd61d8..ac28ef2 100644 --- a/backend/src/__tests__/database.test.ts +++ b/backend/src/__tests__/database.test.ts @@ -158,9 +158,11 @@ describe('Database Layer', () => { it('should create and find by id', async () => { const action = await pendingActionRepo.create({ + user_id: 'user-1', conversation_id: conversationId, action_type: 'create_event', action_payload: { summary: 'Meeting', startDateTime: '2025-01-01T10:00:00Z' }, + description: 'Create a meeting', }); expect(action.status).toBe('pending'); expect(action.action_type).toBe('create_event'); @@ -171,22 +173,22 @@ describe('Database Layer', () => { }); it('should find pending actions by conversation', async () => { - await pendingActionRepo.create({ conversation_id: conversationId, action_type: 'create_event', action_payload: {} }); - await pendingActionRepo.create({ conversation_id: conversationId, action_type: 'delete_event', action_payload: {} }); + await pendingActionRepo.create({ user_id: 'user-1', conversation_id: conversationId, action_type: 'create_event', action_payload: {}, description: 'Create event' }); + await pendingActionRepo.create({ user_id: 'user-1', conversation_id: conversationId, action_type: 'delete_event', action_payload: {}, description: 'Delete event' }); const pending = await pendingActionRepo.findPendingByConversationId(conversationId); expect(pending).toHaveLength(2); }); it('should update status to approved', async () => { - const action = await pendingActionRepo.create({ conversation_id: conversationId, action_type: 'create_event', action_payload: {} }); + const action = await pendingActionRepo.create({ user_id: 'user-1', conversation_id: conversationId, action_type: 'create_event', action_payload: {}, description: 'Create event' }); const updated = await pendingActionRepo.updateStatus(action.id, 'approved'); expect(updated!.status).toBe('approved'); expect(updated!.resolved_at).toBeDefined(); }); it('should update status to rejected', async () => { - const action = await pendingActionRepo.create({ conversation_id: conversationId, action_type: 'create_event', action_payload: {} }); + const action = await pendingActionRepo.create({ user_id: 'user-1', conversation_id: conversationId, action_type: 'create_event', action_payload: {}, description: 'Create event' }); await pendingActionRepo.updateStatus(action.id, 'rejected'); // Rejected actions should not appear in pending query diff --git a/backend/src/__tests__/date-format.integration.test.ts b/backend/src/__tests__/date-format.integration.test.ts index 34aa5ee..b809a8d 100644 --- a/backend/src/__tests__/date-format.integration.test.ts +++ b/backend/src/__tests__/date-format.integration.test.ts @@ -52,9 +52,9 @@ describe('Date Format Integration', () => { await new Promise((resolve) => server.close(() => resolve())); }, 15000); - afterEach(() => { + afterEach(async () => { conversationService._clear(); - actionService._clear(); + await actionService._clear(); }); function connectWs(): Promise { diff --git a/backend/src/__tests__/pending-actions-refresh.integration.test.ts b/backend/src/__tests__/pending-actions-refresh.integration.test.ts index 86154bc..307b992 100644 --- a/backend/src/__tests__/pending-actions-refresh.integration.test.ts +++ b/backend/src/__tests__/pending-actions-refresh.integration.test.ts @@ -55,7 +55,7 @@ describe('Pending actions persist across refresh', () => { afterEach(async () => { await conversationService._clear(); - actionService._clear(); + await actionService._clear(); }); function connectWs(): Promise { @@ -95,7 +95,7 @@ describe('Pending actions persist across refresh', () => { // The LLM may or may not return a pendingAction depending on its response. // Regardless, let's also manually create one to ensure deterministic testing. - const manualAction = actionService.createAction( + const manualAction = await actionService.createAction( 'refresh-test-user', conversationId, 'create_event', @@ -136,7 +136,7 @@ describe('Pending actions persist across refresh', () => { const conversationId = chatRes.data.conversationId; // Create a pending action - const action = actionService.createAction( + const action = await actionService.createAction( 'refresh-test-user', conversationId, 'create_event', @@ -188,12 +188,12 @@ describe('Pending actions persist across refresh', () => { const convId2 = chat2.data.conversationId; ws.close(); - actionService.createAction( + await actionService.createAction( 'refresh-test-user', convId1, 'create_event', { calendarId: 'primary', summary: 'Conv1 Event', startDateTime: new Date().toISOString(), endDateTime: new Date().toISOString() }, 'Conv1 action', ); - actionService.createAction( + await actionService.createAction( 'refresh-test-user', convId2, 'create_event', { calendarId: 'primary', summary: 'Conv2 Event', startDateTime: new Date().toISOString(), endDateTime: new Date().toISOString() }, 'Conv2 action', diff --git a/backend/src/controllers/chat.controller.ts b/backend/src/controllers/chat.controller.ts index 70ce812..acbea7c 100644 --- a/backend/src/controllers/chat.controller.ts +++ b/backend/src/controllers/chat.controller.ts @@ -102,7 +102,7 @@ export const getPendingActions = async (req: AuthenticatedRequest, res: Response return; } - let actions = actionService.getPendingActions(req.user.id); + let actions = await actionService.getPendingActions(req.user.id); // Support optional conversationId filter const conversationId = req.query.conversationId; @@ -157,7 +157,7 @@ export const rejectAction = async (req: AuthenticatedRequest, res: Response): Pr } const actionId = param(req, 'id'); - const result = actionService.rejectAction(actionId, req.user.id); + const result = await actionService.rejectAction(actionId, req.user.id); const rejectionMessage = formatActionMessage(result, false); try { await conversationService.addMessage(result.conversationId, 'assistant', rejectionMessage); diff --git a/backend/src/database/db.ts b/backend/src/database/db.ts index 76e6afc..1ac8f1c 100644 --- a/backend/src/database/db.ts +++ b/backend/src/database/db.ts @@ -44,4 +44,5 @@ async function initializeIndexes(database: Db): Promise { await database.collection('conversations').createIndex({ user_id: 1 }); await database.collection('messages').createIndex({ conversation_id: 1 }); await database.collection('pending_actions').createIndex({ conversation_id: 1, status: 1 }); + await database.collection('pending_actions').createIndex({ user_id: 1, status: 1 }); } diff --git a/backend/src/database/repositories/pendingActionRepository.ts b/backend/src/database/repositories/pendingActionRepository.ts index 51486e5..67509a4 100644 --- a/backend/src/database/repositories/pendingActionRepository.ts +++ b/backend/src/database/repositories/pendingActionRepository.ts @@ -3,20 +3,24 @@ import crypto from 'crypto'; interface PendingActionDocument extends Document { id: string; + user_id: string; conversation_id: string; action_type: string; action_payload: string; - status: 'pending' | 'approved' | 'rejected'; + description: string; + status: 'pending' | 'approved' | 'rejected' | 'executed' | 'failed'; created_at: string; resolved_at: string | null; } export interface PendingActionRow { id: string; + user_id: string; conversation_id: string; action_type: string; action_payload: string; - status: 'pending' | 'approved' | 'rejected'; + description: string; + status: 'pending' | 'approved' | 'rejected' | 'executed' | 'failed'; created_at: string; resolved_at: string | null; } @@ -32,37 +36,40 @@ export class PendingActionRepository { if (!doc) return undefined; return { id: doc.id, + user_id: doc.user_id, conversation_id: doc.conversation_id, action_type: doc.action_type, action_payload: doc.action_payload, + description: doc.description, status: doc.status, created_at: doc.created_at, resolved_at: doc.resolved_at ?? null, }; } - async create(action: { conversation_id: string; action_type: string; action_payload: object }): Promise { + async create(action: { + user_id: string; + conversation_id: string; + action_type: string; + action_payload: object; + description: string; + }): Promise { const id = crypto.randomUUID(); const now = new Date().toISOString(); const payload = JSON.stringify(action.action_payload); - await this.collection.insertOne({ - id, - conversation_id: action.conversation_id, - action_type: action.action_type, - action_payload: payload, - status: 'pending', - created_at: now, - resolved_at: null, - }); - return { + const doc: PendingActionDocument = { id, + user_id: action.user_id, conversation_id: action.conversation_id, action_type: action.action_type, action_payload: payload, + description: action.description, status: 'pending', created_at: now, resolved_at: null, }; + await this.collection.insertOne(doc); + return this.toRow(doc)!; } async findById(id: string): Promise { @@ -70,12 +77,17 @@ export class PendingActionRepository { return this.toRow(doc); } + async findPendingByUserId(userId: string): Promise { + const docs = await this.collection.find({ user_id: userId, status: 'pending' }).sort({ created_at: -1 }).toArray(); + return docs.map(d => this.toRow(d)!); + } + async findPendingByConversationId(conversationId: string): Promise { const docs = await this.collection.find({ conversation_id: conversationId, status: 'pending' }).sort({ created_at: 1 }).toArray(); return docs.map(d => this.toRow(d)!); } - async updateStatus(id: string, status: 'approved' | 'rejected'): Promise { + async updateStatus(id: string, status: 'approved' | 'rejected' | 'executed' | 'failed'): Promise { const doc = await this.collection.findOneAndUpdate( { id }, { $set: { status, resolved_at: new Date().toISOString() } }, diff --git a/backend/src/services/action.service.ts b/backend/src/services/action.service.ts index 0ffef87..6310c27 100644 --- a/backend/src/services/action.service.ts +++ b/backend/src/services/action.service.ts @@ -1,55 +1,85 @@ -import { randomUUID } from 'crypto'; import { Auth } from 'googleapis'; -import { PendingAction, ActionType, ActionStatus, CreateEventParams, UpdateEventParams } from '../types'; +import { PendingAction, ActionType, CreateEventParams, UpdateEventParams } from '../types'; import { calendarService } from './calendar.service'; +import { PendingActionRepository, PendingActionRow } from '../database/repositories/pendingActionRepository'; +import { getDatabase } from '../database/db'; +import type { Db } from 'mongodb'; -// In-memory storage (will be replaced by DB repositories) -const actions = new Map(); +function rowToAction(row: PendingActionRow): PendingAction { + return { + id: row.id, + userId: row.user_id, + conversationId: row.conversation_id, + actionType: row.action_type as ActionType, + params: JSON.parse(row.action_payload), + description: row.description, + status: row.status as PendingAction['status'], + createdAt: row.created_at, + resolvedAt: row.resolved_at ?? undefined, + }; +} export class ActionService { - getPendingActions(userId: string): PendingAction[] { - return Array.from(actions.values()) - .filter((a) => a.userId === userId && a.status === 'pending') - .sort((a, b) => b.createdAt.localeCompare(a.createdAt)); + private repo: PendingActionRepository | null = null; + + private async getRepo(): Promise { + if (!this.repo) { + const db: Db = await getDatabase(); + this.repo = new PendingActionRepository(db); + } + return this.repo; + } + + /** Inject a repository directly (used for testing). */ + _setRepo(repo: PendingActionRepository): void { + this.repo = repo; } - getAction(actionId: string): PendingAction | undefined { - return actions.get(actionId); + async getPendingActions(userId: string): Promise { + const repo = await this.getRepo(); + const rows = await repo.findPendingByUserId(userId); + return rows.map(rowToAction); } - createAction( + async getAction(actionId: string): Promise { + const repo = await this.getRepo(); + const row = await repo.findById(actionId); + return row ? rowToAction(row) : undefined; + } + + async createAction( userId: string, conversationId: string, actionType: ActionType, params: PendingAction['params'], description: string - ): PendingAction { - const action: PendingAction = { - id: randomUUID(), - userId, - conversationId, - actionType, - params, + ): Promise { + const repo = await this.getRepo(); + const row = await repo.create({ + user_id: userId, + conversation_id: conversationId, + action_type: actionType, + action_payload: params, description, - status: 'pending', - createdAt: new Date().toISOString(), - }; - actions.set(action.id, action); - return action; + }); + return rowToAction(row); } async approveAction(actionId: string, auth: Auth.OAuth2Client, userId: string): Promise { - const action = actions.get(actionId); - if (!action) { + const repo = await this.getRepo(); + const row = await repo.findById(actionId); + if (!row) { throw new Error('Action not found'); } - if (action.userId !== userId) { + if (row.user_id !== userId) { throw new Error('Unauthorized'); } - if (action.status !== 'pending') { - throw new Error(`Action already ${action.status}`); + if (row.status !== 'pending') { + throw new Error(`Action already ${row.status}`); } + const action = rowToAction(row); + try { switch (action.actionType) { case 'create_event': @@ -64,35 +94,37 @@ export class ActionService { break; } } - action.status = 'executed'; + const updated = await repo.updateStatus(actionId, 'executed'); + return updated ? rowToAction(updated) : { ...action, status: 'executed', resolvedAt: new Date().toISOString() }; } catch (error) { - action.status = 'failed'; + await repo.updateStatus(actionId, 'failed'); throw error; } - - action.resolvedAt = new Date().toISOString(); - return action; } - rejectAction(actionId: string, userId: string): PendingAction { - const action = actions.get(actionId); - if (!action) { + async rejectAction(actionId: string, userId: string): Promise { + const repo = await this.getRepo(); + const row = await repo.findById(actionId); + if (!row) { throw new Error('Action not found'); } - if (action.userId !== userId) { + if (row.user_id !== userId) { throw new Error('Unauthorized'); } - if (action.status !== 'pending') { - throw new Error(`Action already ${action.status}`); + if (row.status !== 'pending') { + throw new Error(`Action already ${row.status}`); } - action.status = 'rejected'; - action.resolvedAt = new Date().toISOString(); - return action; + + const updated = await repo.updateStatus(actionId, 'rejected'); + return updated ? rowToAction(updated) : rowToAction({ ...row, status: 'rejected', resolved_at: new Date().toISOString() }); } - /** For testing: clear all data */ - _clear(): void { - actions.clear(); + /** For testing: clear all pending actions from DB */ + async _clear(): Promise { + const repo = await this.getRepo(); + // Access the collection directly via a known method — drop all docs + const db = (repo as any).db as Db; + await db.collection('pending_actions').deleteMany({}); } } diff --git a/backend/src/services/chat.service.ts b/backend/src/services/chat.service.ts index 2bf4221..14317d7 100644 --- a/backend/src/services/chat.service.ts +++ b/backend/src/services/chat.service.ts @@ -112,7 +112,7 @@ export class ChatService { for (const tc of agentResult.toolCalls) { const actionType = MUTATING_ACTIONS[tc.name]; if (actionType) { - const action = actionService.createAction( + const action = await actionService.createAction( userId, convId, actionType,