From 642b593884588f239431fe77e48c932955635d19 Mon Sep 17 00:00:00 2001 From: Ho Ngoc Hai Date: Sat, 11 Apr 2026 01:38:14 +0700 Subject: [PATCH] docs: move remaining analysis docs to docs/audits/ Move MCP module exploration, quick reference, and inquiries exploration documents to the centralized audit directory. Co-Authored-By: Paperclip --- docs/audits/MCP_MODULE_EXPLORATION.md | 673 ++++++++++++++++++++ docs/audits/MCP_QUICK_REFERENCE.md | 386 +++++++++++ docs/audits/README_INQUIRIES_EXPLORATION.md | 430 +++++++++++++ 3 files changed, 1489 insertions(+) create mode 100644 docs/audits/MCP_MODULE_EXPLORATION.md create mode 100644 docs/audits/MCP_QUICK_REFERENCE.md create mode 100644 docs/audits/README_INQUIRIES_EXPLORATION.md diff --git a/docs/audits/MCP_MODULE_EXPLORATION.md b/docs/audits/MCP_MODULE_EXPLORATION.md new file mode 100644 index 0000000..d08c94b --- /dev/null +++ b/docs/audits/MCP_MODULE_EXPLORATION.md @@ -0,0 +1,673 @@ +# MCP Module Exploration - GoodGo Platform + +## 1. MODULE STRUCTURE & SOURCE FILES + +### Directory Structure +``` +apps/api/src/modules/mcp/ +├── index.ts +├── mcp.module.ts +└── presentation/ + ├── mcp-transport.controller.ts + └── __tests__/ + └── mcp-transport.controller.spec.ts +``` + +### All Source Files (4 files) + +#### 1. **apps/api/src/modules/mcp/index.ts** +- **Type**: Module entry point (exports) +- **Purpose**: Exports the McpIntegrationModule +- **Exports**: `{ McpIntegrationModule }` + +#### 2. **apps/api/src/modules/mcp/mcp.module.ts** +- **Type**: NestJS Module configuration (22 lines) +- **Key Class**: `McpIntegrationModule implements OnModuleInit` +- **Responsibility**: + - Sets up MCP core module with configuration + - Initializes TypesenseClient for MCP registry + - Logs initialized server names on module init +- **Dependencies Injected**: + - `TypesenseClientService` (from SearchModule) + - `McpRegistryService` (from @goodgo/mcp-servers) + - `LoggerService` (from SharedModule) +- **Imports**: + - `SearchModule` + - `AuthModule` + - `McpCoreModule.forRoot()` with config +- **Controllers**: McpTransportController +- **Lifecycle**: Implements `onModuleInit()` + +#### 3. **apps/api/src/modules/mcp/presentation/mcp-transport.controller.ts** +- **Type**: NestJS Controller (102 lines) +- **Key Class**: `McpTransportController` +- **Responsibility**: HTTP transport layer for MCP SSE connections +- **Decorators Applied**: + - `@ApiTags('mcp')` + - `@ApiBearerAuth('JWT')` + - `@Controller('mcp')` + - `@UseGuards(JwtAuthGuard)` - protects all endpoints +- **Properties**: + - `transports: Map` - active session management +- **Injected Dependencies**: + - `registry: McpRegistryService` + +**Endpoints:** + +1. **GET /mcp/servers** (line 27-34) + - Summary: List available MCP servers + - Throttle: 30 requests per 60s + - Returns: `{ servers: string[] }` + - Status: 200 (success), 401 (unauthorized) + +2. **GET /mcp/:serverName/sse** (line 36-68) + - Summary: Open SSE connection to MCP server + - Throttle: 5 requests per 60s (stricter) + - Params: `serverName` + - Returns: SSE stream + - Response Handling: + - Creates `SSEServerTransport` instance + - Stores in map with `sessionId` + - Connects to server via `server.connect(transport)` + - Cleans up on request close + - Status: 200 (stream), 404 (server not found), 401 (unauthorized) + +3. **POST /mcp/:serverName/messages** (line 70-102) + - Summary: Send message to MCP server session + - Throttle: 30 requests per 60s + - Params: `serverName` + - Query: `sessionId` (required) + - Body: Message data passed to transport + - Response Handling: + - Validates sessionId exists + - Delegates to `transport.handlePostMessage(req, res)` + - Status: 200 (success), 400 (missing sessionId), 404 (session not found), 401 (unauthorized) + +--- + +## 2. TEST FILES + +### Total Test Files: 1 + +#### **apps/api/src/modules/mcp/presentation/__tests__/mcp-transport.controller.spec.ts** (174 lines) +- **Testing Framework**: Vitest +- **Subject**: `McpTransportController` +- **Test Structure**: Describe blocks + beforeEach setup + +**Test Suites (4 describe blocks):** + +1. **security decorators** (4 tests) + - Verifies JwtAuthGuard is applied + - Checks Throttle metadata on endpoints + - Validates throttle limits (30, 5, 30) + +2. **listServers** (2 tests) + - Returns server list from registry + - Handles empty server list + +3. **handleSse** (3 tests) + - Throws NOT_FOUND when server doesn't exist + - Creates transport and connects to server + - Cleans up on connection close (transport removal) + +4. **handleMessage** (2 tests) + - Throws BAD_REQUEST when sessionId missing + - Throws NOT_FOUND when session doesn't exist + +**Mocking Patterns Used:** +- `vi.mock()` for external modules (SSEServerTransport) +- `vi.fn()` for service method mocks +- `mockReturnValue()`, `mockResolvedValue()`, `mockRejectValue()` +- Manual mock objects for requests/responses +- Reflection API for metadata checks (`Reflect.getMetadata()`) + +--- + +## 3. DDD LAYER STRUCTURE + +**Current Status**: MCP module has a **SIMPLIFIED structure** (NOT full DDD yet): + +### What EXISTS: +- **Presentation Layer** ✅ + - `presentation/mcp-transport.controller.ts` + - `presentation/__tests__/mcp-transport.controller.spec.ts` + - HTTP endpoint definitions + - Guard decorators (auth) + - Throttle decorators + - Swagger decorators + +### What DOES NOT EXIST (yet): +- **Domain Layer** ❌ + - No `domain/` directory + - No entities, value objects, or domain events + - No domain business logic + +- **Application Layer** ❌ + - No `application/` directory + - No commands/handlers (CQRS pattern not used here) + - No queries + - No DTOs + +- **Infrastructure Layer** ❌ + - No `infrastructure/` directory + - No repositories + - No external service adapters + - No database access + +### Architecture Notes: +- The MCP module acts as an **integration wrapper** +- Delegates to `@goodgo/mcp-servers` library (external dependency) +- Simple **presentation-only controller** approach +- Focuses on HTTP transport mechanism (SSE connections) +- Session management via in-memory Map + +--- + +## 4. KEY CLASSES/HANDLERS NEEDING TESTS + +### Current Implementation: + +1. **McpIntegrationModule** (mcp.module.ts) + - **Status**: Partially tested (initialization logic) + - **Methods needing tests**: + - `constructor()` - dependency injection + - `onModuleInit()` - initialization flow + - **Test focus**: Module setup, service integration, logging + +2. **McpTransportController** (mcp-transport.controller.ts) + - **Status**: Well tested ✅ (174 lines of tests) + - **Methods tested**: + - `listServers()` - returns available servers + - `handleSse()` - establishes SSE connection + - `handleMessage()` - routes messages to session + - **Test coverage**: Happy path + error cases + security decorators + +--- + +## 5. TESTING PATTERNS FROM OTHER MODULES + +### Pattern 1: Auth Module - Handler Testing (Simple) +**File**: `apps/api/src/modules/auth/application/__tests__/login-user.handler.spec.ts` + +```typescript +// KEY PATTERNS: +// 1. No explicit imports from vitest (globals enabled) +// 2. beforeEach: Create handler with mocked dependencies +// 3. vi.fn() for service mocks +// 4. mockResolvedValue() for async returns +// 5. expect().toHaveBeenCalledWith() for verification +// 6. Simple test structure for handlers + +describe('LoginUserHandler', () => { + let handler: LoginUserHandler; + let mockTokenService: { generateTokenPair: ReturnType }; + + const tokenPair = { + accessToken: 'access-jwt', + refreshToken: 'family.refresh-hex', + expiresIn: 900, + }; + + beforeEach(() => { + mockTokenService = { generateTokenPair: vi.fn().mockResolvedValue(tokenPair) }; + handler = new LoginUserHandler(mockTokenService as any); + }); + + it('generates token pair with correct payload', async () => { + const command = new LoginUserCommand('user-1', '0912345678', 'BUYER'); + const result = await handler.execute(command); + + expect(result).toEqual(tokenPair); + expect(mockTokenService.generateTokenPair).toHaveBeenCalledWith({ + sub: 'user-1', + phone: '0912345678', + role: 'BUYER', + }); + }); +}); +``` + +**Key Takeaways**: +- ✅ Minimal setup, direct handler instantiation +- ✅ Command objects for test input +- ✅ Simple mock with typed object +- ✅ Focus on behavior verification +- ✅ Uses `as any` for type casting + + +### Pattern 2: Payments Module - Complex Handler Testing (Advanced) +**File**: `apps/api/src/modules/payments/application/__tests__/create-payment.handler.spec.ts` + +```typescript +// KEY PATTERNS: +// 1. Multiple mocked dependencies (repo, factory, gateway, event bus) +// 2. Complex mock setup with multiple methods +// 3. Tests for idempotency handling +// 4. Error case validation +// 5. Event publishing verification + +describe('CreatePaymentHandler', () => { + let handler: CreatePaymentHandler; + let mockPaymentRepo: { [K in keyof IPaymentRepository]: ReturnType }; + let mockGatewayFactory: { getGateway: ReturnType }; + let mockGateway: { + createPaymentUrl: ReturnType; + verifyCallback: ReturnType; + refund: ReturnType; + }; + let mockEventBus: { publish: ReturnType }; + + beforeEach(() => { + mockPaymentRepo = { + findById: vi.fn(), + findByProviderTxId: vi.fn(), + findByIdempotencyKey: vi.fn(), + findByUserId: vi.fn(), + save: vi.fn().mockResolvedValue(undefined), + update: vi.fn(), + updateIfStatus: vi.fn(), + }; + + mockGateway = { + createPaymentUrl: vi.fn().mockResolvedValue({ + paymentUrl: 'https://vnpay.vn/pay/123', + providerTxId: 'vnpay-tx-1', + }), + verifyCallback: vi.fn(), + refund: vi.fn(), + }; + + mockGatewayFactory = { + getGateway: vi.fn().mockReturnValue(mockGateway), + }; + + mockEventBus = { publish: vi.fn() }; + const mockLogger = { log: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn(), verbose: vi.fn() }; + + handler = new CreatePaymentHandler( + mockPaymentRepo as any, + mockGatewayFactory as any, + mockEventBus as any, + mockLogger as any, + ); + }); + + it('creates payment successfully', async () => { + mockPaymentRepo.findByIdempotencyKey.mockResolvedValue(null); + + const command = new CreatePaymentCommand( + 'user-1', 'VNPAY', 'SUBSCRIPTION', 500_000n, + 'Thanh toán gói Pro', 'https://goodgo.vn/return', '127.0.0.1', + undefined, 'idem-key-1', + ); + const result = await handler.execute(command); + + expect(result.paymentId).toBeDefined(); + expect(result.paymentUrl).toBe('https://vnpay.vn/pay/123'); + expect(result.providerTxId).toBe('vnpay-tx-1'); + expect(mockPaymentRepo.save).toHaveBeenCalledTimes(1); + expect(mockEventBus.publish).toHaveBeenCalled(); + expect(mockGatewayFactory.getGateway).toHaveBeenCalledWith('VNPAY'); + }); + + it('throws ConflictException for duplicate idempotency key (pending)', async () => { + mockPaymentRepo.findByIdempotencyKey.mockResolvedValue({ status: 'PENDING' }); + + const command = new CreatePaymentCommand( + 'user-1', 'VNPAY', 'SUBSCRIPTION', 500_000n, + 'desc', 'https://goodgo.vn/return', '127.0.0.1', + undefined, 'existing-key', + ); + + await expect(handler.execute(command)).rejects.toThrow(/idempotency/); + }); +}); +``` + +**Key Takeaways**: +- ✅ Complex multi-dependency mocking +- ✅ Mapped type for repository mocks `[K in keyof IPaymentRepository]` +- ✅ Individual method mocks for each dependency +- ✅ Tests for business rules (idempotency) +- ✅ Error case testing with regex matching +- ✅ Logger mock included +- ✅ Verify multiple calls and interactions + + +### Pattern 3: Domain Entity Testing (DDD Style) +**File**: `apps/api/src/modules/payments/domain/__tests__/payment.entity.spec.ts` + +```typescript +// KEY PATTERNS: +// 1. Explicit vitest imports at the top +// 2. Complex entity with state transitions +// 3. Tests for domain events +// 4. Tests for value objects +// 5. Tests for error handling (Result types) +// 6. Helper factory function for test setup + +import { describe, it, expect } from 'vitest'; +import { PaymentEntity } from '../entities/payment.entity'; +import { PaymentCompletedEvent } from '../events/payment-completed.event'; +import { PaymentCreatedEvent } from '../events/payment-created.event'; +import { PaymentFailedEvent } from '../events/payment-failed.event'; +import { PaymentRefundedEvent } from '../events/payment-refunded.event'; +import { Money } from '../value-objects/money.vo'; + +describe('PaymentEntity', () => { + const createPayment = (status?: string) => { + const money = Money.create(500_000n).unwrap(); + const payment = PaymentEntity.createNew( + 'pay-1', + 'user-1', + 'VNPAY', + 'LISTING_FEE', + money, + 'txn-1', + 'idem-key-1', + ); + if (status === 'PROCESSING') { + payment.markProcessing('vnp-txn-123'); + } + if (status === 'COMPLETED') { + payment.markProcessing('vnp-txn-123'); + payment.clearDomainEvents(); + payment.markCompleted({ responseCode: '00' }); + } + return payment; + }; + + it('should create a new payment with domain events', () => { + const money = Money.create(500_000n).unwrap(); + const payment = PaymentEntity.createNew( + 'pay-1', + 'user-1', + 'VNPAY', + 'LISTING_FEE', + money, + 'txn-1', + ); + + expect(payment.id).toBe('pay-1'); + expect(payment.status).toBe('PENDING'); + + const events = payment.domainEvents; + expect(events).toHaveLength(1); + expect(events[0]).toBeInstanceOf(PaymentCreatedEvent); + }); + + it('should mark completed payment as refunded and emit event', () => { + const payment = createPayment('COMPLETED'); + payment.clearDomainEvents(); + const result = payment.markRefunded(); + expect(result.isOk).toBe(true); + expect(payment.status).toBe('REFUNDED'); + + const events = payment.domainEvents; + expect(events).toHaveLength(1); + expect(events[0]).toBeInstanceOf(PaymentRefundedEvent); + }); + + it('should not refund a non-completed payment', () => { + const payment = createPayment(); + const result = payment.markRefunded(); + expect(result.isErr).toBe(true); + expect(result.unwrapErr().message).toContain('hoàn tiền'); + }); +}); +``` + +**Key Takeaways**: +- ✅ Explicit vitest imports (explicit over implicit) +- ✅ Helper factory method for complex setup +- ✅ Tests for entity behavior (state transitions) +- ✅ Tests for domain events +- ✅ Tests for error handling (Result pattern) +- ✅ No mocking - tests real entity logic +- ✅ Value object usage (Money.create().unwrap()) +- ✅ Domain event verification + + +### Pattern 4: Infrastructure Service Testing (Crypto/External) +**File**: `apps/api/src/modules/payments/infrastructure/__tests__/zalopay.service.spec.ts` + +```typescript +// KEY PATTERNS: +// 1. Explicit vitest imports +// 2. External service simulation (payment gateway) +// 3. Crypto/HMAC signature testing +// 4. Helper function to build test data +// 5. Mocking ConfigService +// 6. Tests for security (tamper detection) + +import * as crypto from 'crypto'; +import { type ConfigService } from '@nestjs/config'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { ZalopayService } from '../services/zalopay.service'; + +describe('ZalopayService', () => { + let service: ZalopayService; + const key2 = 'TESTKEY2ABCDEF1234567890ABCDEF12'; + + beforeEach(() => { + const mockConfig = { + get: vi.fn((key: string, defaultValue?: string) => { + const env: Record = { + 'ZALOPAY_APP_ID': '2553', + 'ZALOPAY_KEY1': 'TESTKEY1ABCDEF1234567890ABCDEF12', + 'ZALOPAY_KEY2': 'TESTKEY2ABCDEF1234567890ABCDEF12', + }; + return env[key] ?? defaultValue; + }), + getOrThrow: vi.fn((key: string) => { + // ... + }), + } as unknown as ConfigService; + const mockLogger = { log: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn(), verbose: vi.fn() }; + service = new ZalopayService(mockConfig, mockLogger as any); + }); + + function buildCallbackData( + dataPayload: Record = {}, + tamperMac = false, + ): Record { + const payload = { + app_id: 2553, + app_trans_id: '260408_order-123', + zp_trans_id: 'ZLP_TX_456', + ...dataPayload, + }; + const dataStr = JSON.stringify(payload); + let mac = crypto + .createHmac('sha256', key2) + .update(dataStr) + .digest('hex'); + + if (tamperMac) { + mac = 'a'.repeat(mac.length); + } + + return { data: dataStr, mac }; + } + + it('should verify a valid callback with timingSafeEqual', () => { + const data = buildCallbackData(); + const result = service.verifyCallback(data); + + expect(result.isValid).toBe(true); + expect(result.isSuccess).toBe(true); + expect(result.orderId).toBe('260408_order-123'); + expect(result.providerTxId).toBe('ZLP_TX_456'); + }); + + it('should reject an invalid MAC', () => { + const data = buildCallbackData({}, true); + const result = service.verifyCallback(data); + + expect(result.isValid).toBe(false); + }); + + it('should handle invalid JSON in data field gracefully', () => { + const mac = crypto + .createHmac('sha256', key2) + .update('not-json') + .digest('hex'); + + const result = service.verifyCallback({ data: 'not-json', mac }); + expect(result.isValid).toBe(false); + }); +}); +``` + +**Key Takeaways**: +- ✅ Explicit vitest imports (import from 'vitest') +- ✅ Tests for security-critical functionality (HMAC verification) +- ✅ ConfigService mocking for environment variables +- ✅ Helper function for complex test data generation +- ✅ Tests for error conditions (invalid JSON, wrong MAC) +- ✅ Tests for security edge cases (tampered data) +- ✅ Uses Node.js crypto module directly +- ✅ Real algorithm testing (SHA256 HMAC) + +--- + +## 6. TESTING CONVENTIONS SUMMARY + +### Framework & Setup +- **Framework**: Vitest +- **Environment**: Node.js +- **Globals**: Enabled (`globals: true` in config) +- **File Pattern**: `*.spec.ts` (not `.test.ts`) +- **Test Organization**: `__tests__/` subdirectory + +### Import Styles +```typescript +// Style 1: Using globals (in simple tests) +describe('MyClass', () => { + it('does something', () => { + expect(true).toBe(true); + }); +}); + +// Style 2: Explicit imports (in complex/domain tests) +import { describe, it, expect, beforeEach, vi } from 'vitest'; +``` + +### Mocking Patterns +1. **Service Mocks** + ```typescript + mockService = { + method: vi.fn().mockResolvedValue(value), + method2: vi.fn().mockReturnValue(value), + }; + ``` + +2. **Module Mocks** + ```typescript + vi.mock('@goodgo/mcp-servers', () => ({ + SSEServerTransport: class MockSSE { /* ... */ }, + })); + ``` + +3. **Configuration Mocks** + ```typescript + const mockConfig = { + get: vi.fn((key) => env[key]), + getOrThrow: vi.fn((key) => env[key] || throw), + }; + ``` + +### Test Structure +```typescript +describe('ClassName', () => { + let instance: ClassName; + let mockDep1: Mock; + let mockDep2: Mock; + + beforeEach(() => { + // Setup mocks + mockDep1 = { method: vi.fn() }; + // Instantiate with mocks + instance = new ClassName(mockDep1 as any, mockDep2 as any); + }); + + describe('methodName', () => { + it('happy path scenario', async () => { + // Arrange + mockDep1.method.mockResolvedValue(expected); + + // Act + const result = await instance.method(); + + // Assert + expect(result).toEqual(expected); + expect(mockDep1.method).toHaveBeenCalledWith(params); + }); + + it('error case', async () => { + // Arrange + mockDep1.method.mockRejectedValue(new Error('fail')); + + // Act & Assert + await expect(instance.method()).rejects.toThrow('fail'); + }); + }); +}); +``` + +### Common Assertions Used +- `expect(x).toBe(value)` - strict equality +- `expect(x).toEqual(object)` - deep equality +- `expect(x).toHaveLength(n)` - array/string length +- `expect(x).toBeInstanceOf(Class)` - instance check +- `expect(fn).toHaveBeenCalled()` - called verification +- `expect(fn).toHaveBeenCalledWith(args)` - call arguments +- `expect(fn).toHaveBeenCalledTimes(n)` - call count +- `expect(promise).rejects.toThrow(msg)` - error expectation + +### Vitest-Specific Features Used +- `vi.fn()` - create mock function +- `vi.mock()` - module mocking +- `mockResolvedValue()` - async return +- `mockReturnValue()` - sync return +- `mockRejectedValue()` - error simulation +- `Reflect.getMetadata()` - decorator inspection (NestJS) + +--- + +## 7. RECOMMENDATIONS FOR MCP MODULE TESTING + +### Current Coverage Status +- ✅ Controller (presentation layer): Well tested +- ⚠️ Module initialization: Basic (mocked) +- ❌ Domain layer: Not implemented +- ❌ Application layer: Not implemented +- ❌ Infrastructure layer: Not implemented + +### Recommended Next Steps +1. **Expand Controller Tests** + - Integration tests with mock SSE connections + - Test session lifecycle (open → message → close) + - Test error recovery + +2. **Add Module Tests** + - Test `McpIntegrationModule.onModuleInit()` + - Test registry initialization + - Test service injection + +3. **Consider Adding Domain Tests** (if business logic added) + - Session entity tests + - Connection state management + - Event handling + +### Test Command +```bash +# Run all MCP tests +pnpm test -- src/modules/mcp + +# Run with coverage +pnpm test -- --coverage src/modules/mcp + +# Watch mode +pnpm test -- --watch src/modules/mcp +``` diff --git a/docs/audits/MCP_QUICK_REFERENCE.md b/docs/audits/MCP_QUICK_REFERENCE.md new file mode 100644 index 0000000..4956baa --- /dev/null +++ b/docs/audits/MCP_QUICK_REFERENCE.md @@ -0,0 +1,386 @@ +# MCP Module - Quick Reference & Testing Guide + +## 📋 Module at a Glance + +| Aspect | Details | +|--------|---------| +| **Location** | `apps/api/src/modules/mcp/` | +| **Total Files** | 4 source files (2 TypeScript + 1 module config + 1 export) | +| **Test Files** | 1 test file (174 lines) | +| **Architecture** | Presentation layer only (simplified) | +| **Testing Framework** | Vitest with Globals enabled | +| **Test Coverage** | Controller well tested, Module/Infrastructure untested | + +--- + +## 📁 File Listing (Complete) + +``` +✅ TESTED ✅ SOURCE ❌ NOT TESTED +───────────────────────────────────────────────────────────── +MCP module/ mcp/ +├── mcp.module.ts [Module config - 22 lines] +├── index.ts [Re-export - 1 line] +└── presentation/ + ├── mcp-transport.controller.ts [Controller - 102 lines] + └── __tests__/ + └── mcp-transport.controller.spec.ts ✅ [174 lines] +``` + +### File Details + +| File | Type | Lines | Status | Purpose | +|------|------|-------|--------|---------| +| `index.ts` | Export | 1 | ✅ | Module entry point | +| `mcp.module.ts` | Config | 22 | ⚠️ | NestJS module setup | +| `mcp-transport.controller.ts` | Controller | 102 | ✅ | HTTP/SSE transport | +| `mcp-transport.controller.spec.ts` | Test | 174 | ✅ | Controller tests | + +--- + +## 🏗️ Architecture Diagram + +### Current DDD Structure (Simplified) + +``` +┌─────────────────────────────────────────────────┐ +│ Presentation Layer (Only) ✅ │ +├─────────────────────────────────────────────────┤ +│ • McpTransportController │ +│ - GET /mcp/servers (list) │ +│ - GET /mcp/:serverName/sse (connect) │ +│ - POST /mcp/:serverName/messages (send) │ +├─────────────────────────────────────────────────┤ +│ Dependencies: │ +│ • McpRegistryService (external) │ +│ • JwtAuthGuard (auth layer) │ +│ • SSEServerTransport (external) │ +└─────────────────────────────────────────────────┘ +``` + +### Missing Layers (Not Implemented) + +``` +❌ Domain Layer + - No entities, value objects, events + - No business logic abstractions + +❌ Application Layer + - No CQRS handlers + - No command/query objects + - No use case orchestration + +❌ Infrastructure Layer + - No repositories + - No external service adapters + - No persistence logic +``` + +--- + +## 🧪 Testing Overview + +### Test File Stats + +``` +File: mcp-transport.controller.spec.ts +├── Total Tests: 11 +├── Test Suites: 4 describe blocks +├── Total Lines: 174 +└── Coverage Areas: + ├── Security Decorators: 4 tests + ├── listServers(): 2 tests + ├── handleSse(): 3 tests + └── handleMessage(): 2 tests +``` + +### Test Breakdown by Suite + +``` +1. Security Decorators (4 tests) + ├── JwtAuthGuard applied + ├── listServers throttle (30 req/60s) + ├── handleSse throttle (5 req/60s) ⚡ stricter + └── handleMessage throttle (30 req/60s) + +2. listServers (2 tests) + ├── Returns server list + └── Handles empty list + +3. handleSse (3 tests) + ├── Throws NOT_FOUND for missing server + ├── Creates transport & connects + └── Cleans up on connection close + +4. handleMessage (2 tests) + ├── Throws BAD_REQUEST for missing sessionId + └── Throws NOT_FOUND for expired session +``` + +--- + +## 🎯 Key Classes & Methods + +### McpIntegrationModule +```typescript +class McpIntegrationModule implements OnModuleInit { + constructor( + typesenseClient: TypesenseClientService, + mcpRegistry: McpRegistryService, + logger: LoggerService, + ) {} + + async onModuleInit(): Promise { + // 1. Set typesense client on registry + // 2. Re-initialize servers + // 3. Log initialized servers + } +} +``` + +### McpTransportController +```typescript +@Controller('mcp') +@UseGuards(JwtAuthGuard) +class McpTransportController { + private transports: Map + + constructor(registry: McpRegistryService) {} + + @Get('servers') + @Throttle(30/60s) + listServers(): { servers: string[] } + + @Get(':serverName/sse') + @Throttle(5/60s) // Stricter limit for SSE + async handleSse(serverName, user, req, res): Promise + + @Post(':serverName/messages') + @Throttle(30/60s) + async handleMessage(serverName, user, req, res): Promise +} +``` + +--- + +## 🔧 Testing Patterns Used + +### 1. Mock Pattern (Vitest) +```typescript +// Mocking external module classes +vi.mock('@goodgo/mcp-servers', () => ({ + SSEServerTransport: class MockSSEServerTransport { + sessionId = 'mock-session-id'; + handlePostMessage = vi.fn().mockResolvedValue(undefined); + constructor(path: string, res: unknown) {} + }, +})); +``` + +### 2. Service Mock Pattern +```typescript +const mockRegistry = { + getServerNames: vi.fn(), + getServer: vi.fn(), +}; +``` + +### 3. Decorator Verification Pattern +```typescript +// Using Reflect API for NestJS decorators +const guards = Reflect.getMetadata('__guards__', McpTransportController); +const throttleLimit = Reflect.getMetadata( + 'THROTTLER:LIMITdefault', + McpTransportController.prototype.listServers, +); +``` + +### 4. Error Testing Pattern +```typescript +// Testing HTTP errors +await expect( + controller.handleSse('nonexistent', user, req, res) +).rejects.toThrow(HttpException); + +// Checking status code +try { + await controller.handleSse(...); +} catch (error) { + expect((error as HttpException).getStatus()).toBe(HttpStatus.NOT_FOUND); +} +``` + +--- + +## 📚 Testing Patterns from Other Modules + +### Auth Module - Simple Handler Pattern +```typescript +// Minimal dependencies, focused testing +describe('LoginUserHandler', () => { + let handler: LoginUserHandler; + let mockTokenService = { generateTokenPair: vi.fn() }; + + beforeEach(() => { + handler = new LoginUserHandler(mockTokenService as any); + }); + + it('generates token pair', async () => { + mockTokenService.generateTokenPair.mockResolvedValue(tokenPair); + const result = await handler.execute(command); + expect(result).toEqual(tokenPair); + }); +}); +``` + +### Payments Module - Complex Handler Pattern +```typescript +// Multiple dependencies, rich testing scenarios +describe('CreatePaymentHandler', () => { + let handler: CreatePaymentHandler; + let mockPaymentRepo: { [K in keyof IPaymentRepository]: ReturnType }; + let mockGatewayFactory: { getGateway: ReturnType }; + let mockEventBus: { publish: ReturnType }; + + beforeEach(() => { + mockPaymentRepo = { + findById: vi.fn(), + save: vi.fn().mockResolvedValue(undefined), + // ... more methods + }; + handler = new CreatePaymentHandler(mockPaymentRepo as any, ...); + }); + + it('creates payment successfully', async () => { + // Rich test scenario with multiple assertions + expect(result.paymentId).toBeDefined(); + expect(mockPaymentRepo.save).toHaveBeenCalledTimes(1); + expect(mockEventBus.publish).toHaveBeenCalled(); + }); +}); +``` + +### Payments Domain - DDD Pattern +```typescript +// Explicit imports, entity behavior testing +import { describe, it, expect } from 'vitest'; + +describe('PaymentEntity', () => { + it('should create payment with events', () => { + const payment = PaymentEntity.createNew(...); + + expect(payment.status).toBe('PENDING'); + expect(payment.domainEvents).toHaveLength(1); + expect(payment.domainEvents[0]).toBeInstanceOf(PaymentCreatedEvent); + }); + + it('should not refund non-completed payment', () => { + const payment = createPayment(); // helper + const result = payment.markRefunded(); + expect(result.isErr).toBe(true); + }); +}); +``` + +### Infrastructure Service - Crypto Pattern +```typescript +// Complex external service testing +describe('ZalopayService', () => { + let service: ZalopayService; + + beforeEach(() => { + const mockConfig = { + get: vi.fn((key) => env[key]), + getOrThrow: vi.fn((key) => env[key] || throw), + }; + service = new ZalopayService(mockConfig as any); + }); + + it('should verify valid callback', () => { + const mac = crypto.createHmac('sha256', key2).update(dataStr).digest('hex'); + const result = service.verifyCallback({ data: dataStr, mac }); + + expect(result.isValid).toBe(true); + expect(result.orderId).toBe('expected-order-id'); + }); +}); +``` + +--- + +## 🚀 Running Tests + +### Test Commands +```bash +# Run all MCP tests +pnpm test -- src/modules/mcp + +# Run with watch mode +pnpm test -- --watch src/modules/mcp + +# Run with coverage +pnpm test -- --coverage src/modules/mcp + +# Run specific test file +pnpm test -- src/modules/mcp/presentation/__tests__/mcp-transport.controller.spec.ts +``` + +### Vitest Configuration +```typescript +// apps/api/vitest.config.ts +{ + test: { + globals: true, // vi, describe, it, expect available globally + environment: 'node', + include: ['src/**/*.spec.ts'], // Matches *.spec.ts pattern + exclude: ['**/*.integration.spec.ts', 'node_modules'], + } +} +``` + +--- + +## 💡 Recommendations + +### Immediate (High Priority) +- ✅ Controller is well tested - maintain this +- 📝 Add tests for `McpIntegrationModule.onModuleInit()` +- 📝 Test module dependency injection + +### Future (Lower Priority) +- 🏗️ If domain logic is added, create domain tests +- 🏗️ If application handlers are added, follow payments module pattern +- 🏗️ Add integration tests for full SSE lifecycle + +### Best Practices to Follow +1. **Use globals pattern** for simple tests (like existing controller tests) +2. **Use explicit imports** for complex domain tests +3. **Use helper factories** for complex entity setup +4. **Use Reflect API** for decorator verification +5. **Test both happy path AND error cases** +6. **Use regex matching** for error message assertions +7. **Verify service calls** with `toHaveBeenCalledWith()` + +--- + +## 📖 File Location Reference + +``` +GoodGo Platform Root +├── apps/api/src/modules/mcp/ ← MCP Module +│ ├── index.ts +│ ├── mcp.module.ts +│ └── presentation/ +│ ├── mcp-transport.controller.ts +│ └── __tests__/ +│ └── mcp-transport.controller.spec.ts +│ +├── apps/api/vitest.config.ts ← Test config +├── apps/api/package.json ← Test scripts +│ +└── MCP_MODULE_EXPLORATION.md ← Full documentation +``` + +--- + +Generated: April 11, 2026 diff --git a/docs/audits/README_INQUIRIES_EXPLORATION.md b/docs/audits/README_INQUIRIES_EXPLORATION.md new file mode 100644 index 0000000..8769b24 --- /dev/null +++ b/docs/audits/README_INQUIRIES_EXPLORATION.md @@ -0,0 +1,430 @@ +# 📚 Inquiries Module - Complete Exploration Documentation + +**Generated:** April 11, 2026 +**Module:** `apps/api/src/modules/inquiries/` +**Status:** ✅ Complete & Thorough Analysis + +--- + +## 📖 DOCUMENTATION FILES + +This exploration includes **3 comprehensive documentation files** designed for different use cases: + +### 1. **INQUIRIES_MODULE_EXPLORATION.md** (23 KB) +**Purpose:** In-depth reference guide +**Best for:** Understanding architecture, patterns, and complete file descriptions + +Contains: +- 📁 Complete directory structure +- 📄 Full file listing with descriptions +- 🏗️ Module architecture diagrams +- 🔑 Key classes & handlers breakdown +- 🎯 DDD layer analysis +- 📊 Test file summary +- 🛠️ Dependencies & security +- 📝 API contracts + +**Read this when:** You need complete understanding of the module + +--- + +### 2. **INQUIRIES_MODULE_QUICK_REFERENCE.md** (9.3 KB) +**Purpose:** Quick lookup guide +**Best for:** Getting up to speed quickly, finding specific information + +Contains: +- 📊 Module at a glance +- 📁 Files by layer overview +- 🔄 Request flow diagrams +- 🔑 Key classes table +- 📝 Key interfaces +- 🧪 Test statistics +- 🔐 Authorization matrix +- 🎯 DDD principles +- 📌 Common patterns +- 🔍 Where to look for X guide + +**Read this when:** You need quick answers or are new to the module + +--- + +### 3. **INQUIRIES_COMPLETE_FILE_INDEX.md** (23 KB) +**Purpose:** Exhaustive file reference +**Best for:** Finding specific files, understanding dependencies, planning modifications + +Contains: +- 📋 Complete file listing by path +- 📊 File statistics tables +- 🔍 File discovery guide +- 🔗 Dependency graph +- 📝 File cross-references +- 🎯 Entry points for modifications +- Line count for each file +- Detailed method descriptions + +**Read this when:** You need file-by-file details or planning changes + +--- + +## 🎯 QUICK START BY USE CASE + +### I want to understand the overall architecture +→ Start with **Quick Reference** → Dive into **Exploration** + +### I'm new to this module +→ Start with **Quick Reference** → Browse **File Index** as needed + +### I need to add a new endpoint +→ Check **File Index** → Entry Points section → Follow the guide + +### I need to understand a specific layer +→ Search **Quick Reference** for layer → Read full details in **Exploration** + +### I'm looking for a specific class or method +→ Use **File Index** → File Discovery Guide section + +### I need to understand data flow +→ **Quick Reference** → Request Flows section + +### I'm fixing a bug or refactoring +→ **File Index** → Dependency Graph & Cross-References + +--- + +## 📊 MODULE SNAPSHOT + +``` +Location: apps/api/src/modules/inquiries/ +Files: 25 total (19 source + 6 test files) +LOC: 1,212 lines of code +Pattern: CQRS + DDD (Clean Architecture) +Tests: 24 test cases across 6 suites +Endpoints: 4 HTTP endpoints +``` + +### Layer Breakdown + +| Layer | Files | Purpose | +|-------|-------|---------| +| **Presentation** | 5 | HTTP endpoints + validation | +| **Application** | 8 | Commands/Queries + orchestration | +| **Domain** | 6 | Business logic + contracts | +| **Infrastructure** | 1 | Prisma persistence | +| **Module** | 2 | Configuration + exports | + +--- + +## 🗂️ FILE TREE + +``` +inquiries/ +├── 📄 index.ts [Barrel export] +├── 📄 inquiries.module.ts [NestJS Module] +│ +├── 📁 presentation/ +│ ├── controllers/ +│ │ └── inquiries.controller.ts [4 endpoints] +│ ├── dto/ +│ │ ├── create-inquiry.dto.ts [Input validation] +│ │ └── list-inquiries.dto.ts [Pagination] +│ └── __tests__/ +│ └── inquiries.controller.spec.ts [6 tests] +│ +├── 📁 application/ +│ ├── commands/ +│ │ ├── create-inquiry/ +│ │ │ ├── create-inquiry.command.ts +│ │ │ └── create-inquiry.handler.ts +│ │ └── mark-inquiry-read/ +│ │ ├── mark-inquiry-read.command.ts +│ │ └── mark-inquiry-read.handler.ts +│ ├── queries/ +│ │ ├── get-inquiries-by-agent/ +│ │ │ ├── get-inquiries-by-agent.query.ts +│ │ │ └── get-inquiries-by-agent.handler.ts +│ │ └── get-inquiries-by-listing/ +│ │ ├── get-inquiries-by-listing.query.ts +│ │ └── get-inquiries-by-listing.handler.ts +│ └── __tests__/ +│ ├── create-inquiry.handler.spec.ts +│ ├── mark-inquiry-read.handler.spec.ts +│ ├── get-inquiries-by-listing.handler.spec.ts +│ └── get-inquiries-by-agent.handler.spec.ts +│ +├── 📁 domain/ +│ ├── entities/ +│ │ └── inquiry.entity.ts [Aggregate Root] +│ ├── events/ +│ │ ├── inquiry-created.event.ts +│ │ └── inquiry-read.event.ts +│ ├── repositories/ +│ │ ├── inquiry.repository.ts [Interface] +│ │ └── inquiry-read.dto.ts [Read DTO] +│ └── __tests__/ +│ └── inquiry-domain.spec.ts [5 tests] +│ +└── 📁 infrastructure/ + └── repositories/ + └── prisma-inquiry.repository.ts [Implementation] +``` + +--- + +## 🔄 REQUEST FLOWS + +### Create Inquiry +``` +POST /inquiries +→ Controller validates JWT +→ CreateInquiryCommand dispatched +→ CreateInquiryHandler executes: + • Validate listing exists + • Create InquiryEntity + • Save to repository + • Publish InquiryCreatedEvent +→ Response: { id, listingId, createdAt } +``` + +### Mark as Read +``` +PATCH /inquiries/:id/read (AGENT only) +→ Controller validates JWT + AGENT role +→ MarkInquiryReadCommand dispatched +→ MarkInquiryReadHandler executes: + • Load inquiry entity + • Verify agent owns listing + • Update entity state + • Persist change + • Publish InquiryReadEvent +→ Response: { success: true } +``` + +### List Inquiries +``` +GET /inquiries/listing/:id or /agent/me +→ Controller validates JWT (+ AGENT for /agent/me) +→ Query dispatched +→ Handler resolves pagination +→ Repository executes paginated query +→ Response: PaginatedResult +``` + +--- + +## 🔑 KEY CONCEPTS + +### DDD (Domain-Driven Design) +- **Domain Entity:** `InquiryEntity` encapsulates inquiry logic +- **Domain Events:** `InquiryCreatedEvent`, `InquiryReadEvent` +- **Repository Pattern:** Interface in domain, implementation in infrastructure +- **Aggregate Root:** InquiryEntity manages state transitions + +### CQRS (Command Query Responsibility Segregation) +- **Commands:** CreateInquiryCommand, MarkInquiryReadCommand (write operations) +- **Queries:** GetInquiriesByListingQuery, GetInquiriesByAgentQuery (read operations) +- **Handlers:** Separate classes for each command/query + +### Clean Architecture Layers +1. **Presentation:** HTTP endpoints, DTOs, decorators +2. **Application:** Commands/Queries, handlers, coordination +3. **Domain:** Business logic, entities, events, interfaces +4. **Infrastructure:** Database, persistence implementation + +--- + +## 🧪 TEST COVERAGE + +**Total:** 24 tests across 6 test suites + +| Test Suite | Tests | Focus | +|-----------|-------|-------| +| Domain Entity | 5 | Aggregate behavior, events | +| CreateInquiryHandler | 4 | Happy path, validation, events | +| MarkInquiryReadHandler | 5 | Happy path, auth, errors | +| GetInquiriesByListingHandler | 2 | Pagination, empty results | +| GetInquiriesByAgentHandler | 2 | Results, agent lookup | +| InquiriesController | 6 | All endpoints, defaults | + +--- + +## 🔐 Security + +**Authentication:** All endpoints require JWT token +**Authorization:** RBAC with AGENT role for certain endpoints + +### Endpoint Security Matrix + +| Endpoint | Auth | Role | Extra Checks | +|----------|------|------|--------------| +| POST /inquiries | JWT | Any | Listing exists | +| GET /listing/:id | JWT | Any | - | +| GET /agent/me | JWT | AGENT | - | +| PATCH /:id/read | JWT | AGENT | Agent owns listing | + +--- + +## 📡 API CONTRACTS + +### POST /inquiries +``` +Request: { listingId: string, message: string, phone?: string } +Response: { id: string, listingId: string, createdAt: string } +Status: 201 | 400 | 401 | 404 +``` + +### GET /inquiries/listing/:listingId +``` +Query: { page?: number, limit?: number } +Response: PaginatedResult +Status: 200 | 401 +``` + +### GET /inquiries/agent/me +``` +Query: { page?: number, limit?: number } +Response: PaginatedResult +Status: 200 | 401 | 403 +``` + +### PATCH /inquiries/:id/read +``` +Response: { success: boolean } +Status: 200 | 401 | 403 | 404 +``` + +--- + +## 🚀 GETTING STARTED + +### For Understanding the Code +1. Read **Quick Reference** (5 min) +2. Review **Request Flows** (5 min) +3. Explore specific layers in **Exploration** as needed + +### For Adding Features +1. Check **File Index** → Entry Points (2 min) +2. Follow the modification guide (varies) +3. Check existing tests for patterns + +### For Debugging +1. Identify the layer (presentation/application/domain/infrastructure) +2. Find the file in **File Index** +3. Check tests for expected behavior + +--- + +## 📋 COMMON PATTERNS + +### Adding a Command +1. Create `application/commands/[name]/[name].command.ts` +2. Create `application/commands/[name]/[name].handler.ts` +3. Implement `ICommandHandler` +4. Register in `inquiries.module.ts` CommandHandlers array +5. Add tests in `application/__tests__/` + +### Adding a Query +1. Create `application/queries/[name]/[name].query.ts` +2. Create `application/queries/[name]/[name].handler.ts` +3. Implement `IQueryHandler` +4. Register in `inquiries.module.ts` QueryHandlers array +5. Add tests in `application/__tests__/` + +### Adding an Endpoint +1. Add method to `InquiriesController` +2. Add DTOs if needed to `presentation/dto/` +3. Dispatch command/query via bus +4. Add tests to `presentation/__tests__/` + +--- + +## 🔗 EXTERNAL DEPENDENCIES + +**Core:** +- `@nestjs/common` - Framework +- `@nestjs/cqrs` - CQRS bus +- `@prisma/client` - ORM + +**Validation:** +- `class-validator` - DTO validation +- `class-transformer` - Type transformation + +**Documentation:** +- `@nestjs/swagger` - OpenAPI docs + +**Internal:** +- `@modules/shared` - AggregateRoot, exceptions +- `@modules/auth` - JWT guards, decorators + +--- + +## 📈 QUALITY METRICS + +| Aspect | Rating | Notes | +|--------|--------|-------| +| Architecture | ⭐⭐⭐⭐⭐ | Clean DDD + CQRS | +| Organization | ⭐⭐⭐⭐⭐ | Clear layer separation | +| Type Safety | ⭐⭐⭐⭐⭐ | Full TypeScript | +| Tests | ⭐⭐⭐⭐☆ | 24 tests, good depth | +| Auth | ⭐⭐⭐⭐⭐ | Proper RBAC | +| Maintainability | ⭐⭐⭐⭐⭐ | Easy to extend | + +--- + +## 💡 ARCHITECTURAL HIGHLIGHTS + +✅ **Separation of Concerns** - Each layer has single responsibility +✅ **Dependency Inversion** - Depend on abstractions, not concrete implementations +✅ **Event-Driven** - Domain events for business significance +✅ **CQRS** - Read/write paths separate and optimizable +✅ **Type Safety** - Full TypeScript with no any types +✅ **Testability** - All layers independently testable +✅ **Extensibility** - Easy to add new commands/queries + +--- + +## 🎓 LEARNING PATH + +**Beginner:** Read Quick Reference → Understand layers → Look at controller +**Intermediate:** Study handlers → Review entities → Examine tests +**Advanced:** Deep dive into event sourcing → Pagination → Optimization patterns + +--- + +## 📞 REFERENCE QUICK LINKS + +Within documentation files: + +**Quick Reference:** +- Files by layer +- Request flows +- Authorization matrix +- Entry points for changes + +**File Index:** +- Complete file descriptions +- Dependency graph +- Cross-references +- Modification guide + +**Exploration:** +- Architecture diagrams +- Design patterns +- Security details +- Test coverage analysis + +--- + +## ✨ Next Steps + +1. **Read the Quick Reference** to get oriented +2. **Pick a documentation file** based on your need +3. **Use the discovery guides** to find specific information +4. **Follow the modification guides** when making changes +5. **Check tests** for expected behavior + +--- + +**Happy exploring! 🚀** + +Generated: April 11, 2026 +Documentation Status: ✅ Complete