dcrouter/readme.hints.md

# Implementation Hints and Learnings

## SmartProxy Usage

### New Route-Based Architecture (v18+)
- SmartProxy now uses a route-based configuration system
- Routes define match criteria and actions instead of simple port-to-port forwarding
- All traffic types (HTTP, HTTPS, TCP, WebSocket) are configured through routes

```typescript
// NEW: Route-based SmartProxy configuration
const smartProxy = new plugins.smartproxy.SmartProxy({
  routes: [
    {
      name: 'https-traffic',
      match: {
        ports: 443,
        domains: ['example.com', '*.example.com']
      },
      action: {
        type: 'forward',
        target: {
          host: 'backend.server.com',
          port: 8080
        }
      },
      tls: {
        mode: 'terminate',
        certificate: 'auto'
      }
    }
  ],
  defaults: {
    target: {
      host: 'fallback.server.com',
      port: 8080
    }
  },
  acme: {
    accountEmail: 'admin@example.com',
    enabled: true,
    useProduction: true
  }
});
```

### Migration from Old to New
```typescript
// OLD configuration style (deprecated)
{
  fromPort: 443,
  toPort: 8080,
  targetIP: 'backend.server.com',
  domainConfigs: [...]
}

// NEW route-based style
{
  routes: [{
    name: 'main-route',
    match: { ports: 443 },
    action: {
      type: 'forward',
      target: { host: 'backend.server.com', port: 8080 }
    }
  }]
}
```

### Direct Component Usage
- Use SmartProxy components directly instead of creating your own wrappers
- SmartProxy already includes Port80Handler and NetworkProxy functionality
- When using SmartProxy, configure it directly rather than instantiating Port80Handler or NetworkProxy separately

### Certificate Management
- SmartProxy has built-in ACME certificate management
- Configure it in the `acme` property of SmartProxy options
- Use `accountEmail` (not `email`) for the ACME contact email
- SmartProxy handles both HTTP-01 challenges and certificate application automatically

## qenv Usage

### Direct Usage
- Use qenv directly instead of creating environment variable wrappers
- Instantiate qenv with appropriate basePath and nogitPath:

```typescript
const qenv = new plugins.qenv.Qenv('./', '.nogit/');
const value = await qenv.getEnvVarOnDemand('ENV_VAR_NAME');
```

## TypeScript Interfaces

### SmartProxy Interfaces
- Always check the interfaces from the node_modules to ensure correct property names
- Important interfaces for the new architecture:
  - `ISmartProxyOptions`: Main configuration with `routes` array
  - `IRouteConfig`: Individual route configuration
  - `IRouteMatch`: Match criteria for routes
  - `IRouteTarget`: Target configuration for forwarding
  - `IAcmeOptions`: ACME certificate configuration
  - `TTlsMode`: TLS handling modes ('passthrough' | 'terminate' | 'terminate-and-reencrypt')

### New Route Configuration
```typescript
interface IRouteConfig {
  name: string;
  match: {
    ports: number | number[];
    domains?: string | string[];
    path?: string;
    headers?: Record<string, string | RegExp>;
  };
  action: {
    type: 'forward' | 'redirect' | 'block' | 'static';
    target?: {
      host: string | string[] | ((context) => string);
      port: number | 'preserve' | ((context) => number);
    };
  };
  tls?: {
    mode: TTlsMode;
    certificate?: 'auto' | { key: string; cert: string; };
  };
  security?: {
    authentication?: IRouteAuthentication;
    rateLimit?: IRouteRateLimit;
    ipAllowList?: string[];
    ipBlockList?: string[];
  };
}
```

### Required Properties
- For `ISmartProxyOptions`, `routes` array is the main configuration
- For `IAcmeOptions`, use `accountEmail` for the contact email
- Routes must have `name`, `match`, and `action` properties

## Testing

### Test Structure
- Follow the project's test structure, using `@push.rocks/tapbundle`
- Use `expect(value).toEqual(expected)` for equality checks
- Use `expect(value).toBeTruthy()` for boolean assertions

```typescript
tap.test('test description', async () => {
  const result = someFunction();
  expect(result.property).toEqual('expected value');
  expect(result.valid).toBeTruthy();
});
```

### Cleanup
- Include a cleanup test to ensure proper test resource handling
- Add a `stop` test to forcefully end the test when needed:

```typescript
tap.test('stop', async () => {
  await tap.stopForcefully();
});
```

## Architecture Principles

### Simplicity
- Prefer direct usage of libraries instead of creating wrappers
- Don't reinvent functionality that already exists in dependencies
- Keep interfaces clean and focused, avoiding unnecessary abstraction layers

### Component Integration
- Leverage built-in integrations between components (like SmartProxy's ACME handling)
- Use parallel operations for performance (like in the `stop()` method)
- Separate concerns clearly (HTTP handling vs. SMTP handling)

## Email Integration with SmartProxy

### Architecture
- Email traffic is routed through SmartProxy using automatic route generation
- Email server runs on internal ports and receives forwarded traffic from SmartProxy
- SmartProxy handles external ports (25, 587, 465) and forwards to internal ports

### Email Route Generation
```typescript
// Email configuration automatically generates SmartProxy routes
emailConfig: {
  ports: [25, 587, 465],
  hostname: 'mail.example.com',
  domainRules: [...]
}

// Generates routes like:
{
  name: 'smtp-route',
  match: { ports: [25] },
  action: {
    type: 'forward',
    target: { host: 'localhost', port: 10025 }
  },
  tls: { mode: 'passthrough' } // STARTTLS handled by email server
}
```

### Port Mapping
- External port 25 → Internal port 10025 (SMTP)
- External port 587 → Internal port 10587 (Submission)
- External port 465 → Internal port 10465 (SMTPS)

### TLS Handling
- Ports 25 and 587: Use 'passthrough' mode (STARTTLS handled by email server)
- Port 465: Use 'terminate' mode (SmartProxy handles TLS termination)
- Domain-specific TLS can be configured per email rule

## SMTP Test Migration

### Test Framework
- Tests migrated from custom framework to @push.rocks/tapbundle
- Each test file is self-contained with its own server lifecycle management
- Test files use pattern `test.*.ts` for automatic discovery by tstest

### Server Lifecycle
- SMTP server uses `listen()` method to start (not `start()`)
- SMTP server uses `close()` method to stop (not `stop()` or `destroy()`)
- Server loader module manages server lifecycle for tests

### Test Structure
```typescript
import { expect, tap } from '@push.rocks/tapbundle';
import { startTestServer, stopTestServer } from '../server.loader.js';

const TEST_PORT = 2525;
const TEST_TIMEOUT = 10000;

tap.test('prepare server', async () => {
  await startTestServer();
  await new Promise(resolve => setTimeout(resolve, 100));
});

tap.test('test name', async (tools) => {
  const done = tools.defer();
  // test implementation
  done.resolve();
});

tap.test('cleanup server', async () => {
  await stopTestServer();
});

tap.start();
```

### Common Issues and Solutions
1. **Multi-line SMTP responses**: Handle response buffering carefully, especially for EHLO
2. **Timing issues**: Use proper state management instead of string matching
3. **ES Module imports**: Use `import` statements, not `require()`
4. **Server cleanup**: Always close connections properly to avoid hanging tests
5. **Response buffer management**: Clear the response buffer after processing each state to avoid false matches from previous responses. Use specific response patterns (e.g., '250 OK' instead of just '250') to avoid ambiguity.

### SMTP Protocol Testing
- Server generates self-signed certificates automatically for testing
- Default test port is 2525
- Connection timeout is typically 10 seconds
- Always check for complete SMTP responses (ending with space after code)

## SMTP Implementation Findings (2025-05-25)

### Fixed Issues

1. **AUTH Mechanism Implementation**
   - The server-side AUTH command handler was incomplete
   - Implemented `handleAuthPlain` with proper PLAIN authentication flow
   - Implemented `handleAuthLogin` with state-based LOGIN authentication flow
   - Added `validateUser` function to test server configuration
   - AUTH tests now expect STARTTLS instead of direct TLS (`secure: false` with `requireTLS: true`)

2. **TLS Connection Timeout Handling**
   - For secure connections, the client was waiting for 'connect' event instead of 'secureConnect'
   - Fixed in `ConnectionManager.establishSocket()` to use the appropriate event based on connection type
   - This prevents indefinite hangs during TLS handshake failures

3. **STARTTLS Server Implementation**
   - Removed incorrect `(tlsSocket as any)._start()` call which is client-side only
   - Server-side TLS sockets handle handshake automatically when data arrives
   - The `_start()` method caused Node.js assertion failure: `wrap->is_client()`

4. **Edge Case Test Patterns**
   - Tests using non-existent `smtpClient.connect()` method - use `verify()` instead
   - SMTP servers must handle DATA mode properly by processing lines individually
   - Empty/minimal server responses need to be valid SMTP codes (e.g., "250 OK\r\n")
   - Out-of-order pipelined responses break SMTP protocol - responses must be in order

### Common Test Patterns

1. **Connection Testing**
   ```typescript
   const verified = await smtpClient.verify();
   expect(verified).toBeTrue();
   ```

2. **Server Data Handling**
   ```typescript
   socket.on('data', (data) => {
     const lines = data.toString().split('\r\n');
     lines.forEach(line => {
       if (!line && lines[lines.length - 1] === '') return;
       // Process each line individually
     });
   });
   ```

3. **Authentication Setup**
   ```typescript
   auth: {
     required: true,
     methods: ['PLAIN', 'LOGIN'],
     validateUser: async (username, password) => {
       return username === 'testuser' && password === 'testpass';
     }
   }
   ```

### Progress Tracking
- Fixed 8 tests total (as of 2025-05-25)
- Fixed 8 additional tests (as of 2025-05-26):
  - test.cedge-03.protocol-violations.ts
  - test.cerr-03.network-failures.ts
  - test.cerr-05.quota-exceeded.ts
  - test.cerr-06.invalid-recipients.ts
  - test.crel-01.reconnection-logic.ts
  - test.crel-02.network-interruption.ts
  - test.crel-03.queue-persistence.ts
- 26 error logs remaining in `.nogit/testlogs/00err/`
- Performance, additional reliability, RFC compliance, and security tests still need fixes

## Test Fix Findings (2025-05-26)

### Common Issues in SMTP Client Tests

1. **DATA Phase Handling in Test Servers**
   - Test servers must properly handle DATA mode
   - Need to track when in DATA mode and look for the terminating '.'
   - Multi-line data must be processed line by line
   ```typescript
   let inData = false;
   socket.on('data', (data) => {
     const lines = data.toString().split('\r\n');
     lines.forEach(line => {
       if (inData && line === '.') {
         socket.write('250 OK\r\n');
         inData = false;
       } else if (line === 'DATA') {
         socket.write('354 Send data\r\n');
         inData = true;
       }
     });
   });
   ```

2. **Import Issues**
   - `createSmtpClient` should be imported from `ts/mail/delivery/smtpclient/index.js`
   - Test server functions: use `startTestServer`/`stopTestServer` (not `startTestSmtpServer`)
   - Helper exports `createTestSmtpClient`, not `createSmtpClient`

3. **SmtpClient API Misconceptions**
   - SmtpClient doesn't have methods like `connect()`, `isConnected()`, `getConnectionInfo()`
   - Use `verify()` for connection testing
   - Use `sendMail()` with Email objects for sending
   - Connection management is handled internally

4. **createSmtpClient is Not Async**
   - The factory function returns an SmtpClient directly, not a Promise
   - Remove `await` from `createSmtpClient()` calls

5. **Test Expectations**
   - Multi-line SMTP responses may timeout if server doesn't send final line
   - Mixed valid/invalid recipients might succeed for valid ones (implementation-specific)
   - Network failure tests should use realistic expectations

6. **Test Runner Requirements**
   - Tests using `tap` from '@git.zone/tstest/tapbundle' must call `tap.start()` at the end
   - Without `tap.start()`, no tests will be detected or run
   - Place `tap.start()` after all `tap.test()` definitions

7. **Connection Pooling Effects**
   - SmtpClient uses connection pooling by default
   - Test servers may not receive all messages immediately
   - Messages might be queued and sent through different connections
   - Adjust test expectations to account for pooling behavior

## Test Fixing Progress (2025-05-26 Afternoon)

### Summary
- Total failing tests initially: 34
- Tests fixed: 28
- Tests remaining: 6

### Remaining Tests to Fix:
1. test.ccm-05.connection-reuse.ts - SMTP client connection
2. test.cperf-05.network-efficiency.ts - SMTP client performance  
3. test.cperf-06.caching-strategies.ts - SMTP client performance
4. test.cperf-07.queue-management.ts - SMTP client performance
5. test.cperf-08.dns-caching.ts - SMTP client performance
6. test.crel-07.resource-cleanup.ts - SMTP client reliability

### Fixed Tests (28):
- **Edge Cases (1)**: test.cedge-03.protocol-violations.ts ✓
- **Error Handling (4)**: cerr-03, cerr-05, cerr-06 ✓
- **Reliability (6)**: crel-01 through crel-06 ✓
- **RFC Compliance (7)**: crfc-02 through crfc-08 ✓
- **Security (10)**: csec-01 through csec-10 ✓

### Important Notes:
- Error logs are deleted after tests are fixed (per original instruction)
- Tests taking >1 minute usually indicate hanging issues
- Property names: use 'host' not 'hostname' for SmtpClient options
- Always use helpers: createTestSmtpClient, createTestServer
- Always add tap.start() at the end of test files