> For clean Markdown of any page, append .md to the page URL.
> For a complete documentation index, see https://cla-demo.postmansolutions.com/llms.txt.
> For AI client integration (Claude Code, Cursor, etc.), connect to the MCP server at https://cla-demo.postmansolutions.com/_mcp/server.

# Postal Service Evaluation

## Table of Contents

1. [Executive Summary](https://dev.azure.com/CLADevOps/Development/_wiki/wikis/AutoExtract/10841/Postal-Service-Evaluation?anchor=1.-executive-summary)
2. [Architecture Overview](https://dev.azure.com/CLADevOps/Development/_wiki/wikis/AutoExtract/10841/Postal-Service-Evaluation?anchor=2.-architecture-overview)
3. [Key Findings](https://dev.azure.com/CLADevOps/Development/_wiki/wikis/AutoExtract/10841/Postal-Service-Evaluation?anchor=3.-key-findings)
4. [Dependency and Layer Analysis](https://dev.azure.com/CLADevOps/Development/_wiki/wikis/AutoExtract/10841/Postal-Service-Evaluation?anchor=4.-dependency-and-layer-analysis)
5. [Testability Assessment](https://dev.azure.com/CLADevOps/Development/_wiki/wikis/AutoExtract/10841/Postal-Service-Evaluation?anchor=5.-testability-assessment)

---

## 1. Executive Summary

The Postal Service is a **message-driven microservice** responsible for delivering platform messages to registered client applications via HTTP webhooks. The service subscribes to Azure Service Bus topics, attempts delivery to client endpoints, and implements exponential backoff retry logic for failed deliveries.

### Strengths

* **Clear service responsibility**: The service has a well-defined, focused purpose
* **Message-driven design**: Properly uses Azure Service Bus for decoupled message processing
* **TypeScript strict mode enabled**: Ensures strong type safety
* **Good test structure**: Separation of unit, contract, and service tests with TEST\_SCOPE filtering
* **CLA DevCloud2 framework integration**: Proper use of `framework-nodejs-*` packages for logging, configuration, and web support
* **Graceful shutdown handling**: Proper SIGINT handling for receiver cleanup

### Critical Concerns

* **Awilix dependency unused**: Both `awilix` and `@fastify/awilix` are listed as dependencies but are not used in production code (only in test scaffolding)
* **In-memory scheduling store**: The `createSchedulingStore()` creates module-level in-memory state that will be lost on pod restart
* **HTTP is secondary to messaging**: Fastify routes are minimal (only a test endpoint), yet the architecture appears structured for HTTP-primary services
* **Global module-level caching**: Multiple global `Map` instances (`cached.ts`, `cachedForKey.ts`) lack invalidation and could cause memory issues

---

## 2. Architecture Overview

### 2.1 Technology Baseline

| Component      | Technology           | Version                       |
| -------------- | -------------------- | ----------------------------- |
| Runtime        | Node.js              | 20.x (Docker image)           |
| Language       | TypeScript           | 5.9.3                         |
| Module System  | ESM                  | `"type": "module"`            |
| HTTP Framework | Fastify              | 5.6.1                         |
| DI Container   | Awilix               | 12.0.5 (listed, **not used**) |
| Messaging      | Azure Service Bus    | 7.9.5                         |
| Storage        | Azure Blob Storage   | 12.28.0                       |
| Testing        | Vitest               | 3.2.4                         |
| Logging        | Pino + CLA framework | 9.14.0                        |

### 2.2 Service Architecture Type

This is a **message-primary service** with minimal HTTP exposure. The primary flow is:

```
Azure Service Bus (to-send topic)
        ↓
    subscribe()
        ↓
  attemptDelivery()
        ↓ (success)        ↓ (failure)
    complete()         republishRetry()
                            ↓
                    Azure Service Bus (to-retry topic)
                            ↓
                    attemptRetryDelivery()
```

### 2.3 Project Structure

Project structure deviates from other Fluxx services and should be refactored to align application architecture. Particularly the `prototype/` folder should be addressed (see `3.3 Medium Findings`)

```
src/
├── index.ts                    # Application entry point
├── constants/                  # Configuration keys, problem types, error codes
├── internal/                   # Framework adapters, caching, app singleton
│   ├── frameworkAdapter/       # CLA DevCloud2 integration
│   └── pubsub/                 # Message publisher factories
├── middleware/                 # HTTP error handling (limited use)
├── prototype/                  # Core message handling logic (22 files)
├── routes/                     # HTTP routes (minimal)
├── services/                   # Service Bus publisher factory
├── types/                      # TypeScript type definitions
└── utils/                      # Service Bus message utilities
```

### 2.4 Application Entry and Bootstrap

The entry point (`src/index.ts`) follows this sequence:

1. Load dotenv configuration
2. Create DevFramework instance
3. Initialize Azure App Configuration
4. Prepare configuration lookup function
5. Register logger from framework
6. Start Fastify server
7. **Start message subscribers** (primary purpose)
8. Register shutdown hooks

**Observation:** The service properly starts subscribers after the HTTP server, but the HTTP server serves almost no purpose (only a test endpoint exists)

---

## 3. Key Findings

### 3.1 Critical Findings

#### Awilix Declared But Not Used

**Location:** `package.json` lines 53, 56\
**Impact:** Architectural debt; misleading dependency list\
**Description:** Both `@fastify/awilix` (7.0.0) and `awilix` (12.0.5) are listed as production dependencies but are only imported in test files (`test/unit/routes/postal.test.ts`, `test/contract/sql.test.ts`). The production code does not use Awilix for dependency injection.

**Evidence:**

* No `createContainer()` or `asClass/asFunction` calls in `src/`
* The `app` object in `src/internal/app.ts` is a manual singleton pattern, not an Awilix container

---

#### In-Memory Retry Scheduling State

**Location:** `src/prototype/attemptRetryDelivery.ts` line 9\
**Impact:** Data loss on pod restart; incorrect retry behavior in multi-replica deployments\
**Description:** The scheduling store is created at module scope:

```typescript
const schedulingStore = createSchedulingStore();
```

This creates an in-memory `Record<string, RetryScheduling>` that:

* Is lost when the pod restarts
* Is not shared across replicas
* Could cause duplicate or missed deliveries in scaled environments

---

#### Global Module-Level Caches Without Bounds or Invalidation

**Location:**

* `src/internal/cached.ts` (line 1: `const cache = new Map()`)
* `src/internal/cachedForKey.ts` (line 1: `const cache = new Map<string, unknown>()`)

**Impact:** Memory leaks; stale data\
**Description:** These caches:

* Have no TTL or expiration
* Have no maximum size
* Cannot be invalidated
* Will grow indefinitely in long-running processes

The `cachedForKey` cache stores deliverers keyed by `clientApplicationId`, which could grow unbounded if many clients are served.

---

### 3.2 High Findings

#### Swallowed Error Context in Message Handlers

**Location:** `src/prototype/subscribeToSendTopic.ts` lines 11-24

```typescript
try {
    await attemptDelivery(message);
} catch (_ignored) {  // ← Error context discarded
    const nextRetryScheduling = calculateNextRetryScheduling({...});
    await republishRetry({...});
}
```

**Impact:** Loss of debugging information; inability to classify error types\
**Description:** The error is caught but discarded as `_ignored`. There is no:

* Logging of the original error
* Classification (retryable vs. permanent)
* Error context passed to retry scheduling

---

#### Hardcoded Topic Name in republishRetry

**Location:** `src/prototype/republishRetry.ts` line 12

```typescript
await publish({ message: { ...message, retryScheduling: scheduling }, topicName: 'to-retry' });
```

**Impact:** Configuration inconsistency\
**Description:** The topic name `'to-retry'` is hardcoded rather than sourced from configuration constants, unlike the subscription names which use `app.lookup()`.

---

#### Missing Message Validation at Subscription Boundary

**Location:** `src/prototype/subscribe.ts` lines 16-17

```typescript
const postalMessage = message.body;
await messageHandler(postalMessage);
```

**Impact:** Runtime errors from malformed messages; no dead-letter handling for validation failures\
**Description:** Messages are passed directly from Service Bus without validation. There is no:

* Schema validation
* Type guard verification
* Dead-letter routing for invalid payloads

---

#### Service Missing Standard onClose Shutdown Hook

**Location:** `src/index.ts`\
**Impact:** Implementation deviates from standard shutdown hooks.

**Description:** The service registers shutdown hooks via `registerShutdownHook()` for SIGINT/SIGTERM signals, but does not implement the Framework/Fluxx's standard `onClose` hook

**Expected Implementation:**

```typescript
frameworkApp.server.addHook('onClose', async () => {
    const messageReceiver: MessageReceiver = frameworkApp.server.diContainer.resolve('messageReceiver') as MessageReceiver;
    await messageReceiver.stopListening();
});
```

**Recommendation:** Implement the `onClose` hook to ensure proper cleanup during all shutdown scenarios, not just signal-based termination

---

### 3.3 Medium Findings

#### Subscription Handlers Should Be Lightly Refactored

**Location:** `src/prototype/`\
**Impact:** Minor maintainability issues; small risk of hidden errors

**Description:** Both files follow a strong, consistent pattern, but:

* Duplicate setup logic for app.lookup + subscribe exists in both files → Extract a small helper (e.g., createTopicSubscriber) to reduce repetition
* Misleading comment in subscribeToRetryTopic.ts claims the handler is "intentionally empty," but it isn't → Remove or correct it
* Swallowed errors in subscribeToSendTopic.ts (catch (\_ignored)) hide real failures → At minimum, log the error
* Redundant dateNow() calls in retry scheduling reduce clarity → Capture once and reuse

These are non-breaking refactors that will improve clarity and maintainability without changing behavior

#### Prototype Folder is Overloaded

**Location:** `src/prototype/` (22 files)

**Impact:** Reduced maintainability; unclear architectural boundaries\
**Description:** The `prototype/` folder contains:

* Core domain logic (retry scheduling)
* Infrastructure (Service Bus client)
* Application orchestration (subscribe, publish)
* HTTP delivery (postMessage)
* Time utilities (now\.ts)

This mixes multiple architectural concerns in a single "prototype" folder name.

---

#### Inconsistent Configuration Key Usage

**Location:** Multiple files

**Description:** Configuration keys are inconsistently decleared/used:

* `ENV_KEYS` in `configuration.ts`
  * `NEW_INTERNAL_FILE_TOPIC_NAME` defined, not used.
  * `TOPIC_COMPLETED_RESULT` defined, only used in contract test.
  * `SUBSCRIPTION_FILE_PROCESSING_SERVICE` defined, only used in contract test.
* Environment variables defined in `env.example` and missing from `configuration.ts`
  * `SUBSCRIPTION_TO_RETRY`
  * `SUBSCRIPTION_TO_SEND`
  * `TOPIC_TO_RETRY`
  * `TOPIC_TO_SEND`
* Environment variables accessed as string variables, rather than `ENV_KEYS`
  * `'TOPIC_TO_SEND'` and `'SUBSCRIPTION_TO_SEND'` used in `subscribeToSendTopic.ts`
  * `'TOPIC_TO_RETRY'` and `'SUBSCRIPTION_TO_RETRY'` used in `subscribeToRetryTopic.ts`
  * `'REGISTRATION_BASE_URL'` used in `fetchDeliveryUrl.ts`

---

#### Fastify Routes Unused

**Location:** `src/routes/v1/postal.ts`

**Impact:** Architecture mismatch\
**Description:** The only route is a test endpoint:

```typescript
frameworkServer.get(`${basePath}/test/`, async (_request, reply) => {
    reply.send('Test route is working!');
});
```

The service exposes no production HTTP endpoints, yet Fastify is fully bootstrapped.

---

#### Port Conflicts with Extractor Service

**Location:** Environment configuration (`.env`, `.env.example`)\
**Impact:** Deployment conflicts; inability to run services simultaneously in local development

**Description:** The Postal service has port conflicts with the Extractor service:

* `APPLICATION_PORT` (3400) conflicts with Extractor `APPLICATION_PORT` (3400)
* `DEBUG_PORT` (9234) conflicts with Extractor `DEBUG_PORT` (9234)

**Recommendation:**

* Assign unique ports to Postal service (e.g., APPLICATION\_PORT=3500, DEBUG\_PORT=9235)
* Update all environment files (`.env.example`, `.env.local.example`)
* Update documentation and Docker Compose configurations

---

#### Direct NodeJS Process Usage

**Location:** Multiple files\
**Impact:** Testability issues; tight coupling to Node.js runtime

**Description:** Direct usage of the Node.js `process` object appears in multiple locations:

* `createAppConfigOptions.ts` - Uses `process.env` directly
* `index.ts` - Uses `process.exitCode` and `process` references
* `registerShutdownHook.ts` - Uses `process.on()`, `process.exitCode`

**Issues:**

* Makes unit testing more difficult (requires mocking global `process`)
* Tight coupling to Node.js runtime environment
* Inconsistent with framework's configuration abstraction layer
* Signal handling via `process.on()` bypasses Fastify's lifecycle hooks

**Recommendation:**

* Inject environment configuration through the CLA framework's config lookup mechanism
* Use Fastify lifecycle hooks (`onClose`, `onReady`) instead of raw process signals
* If direct process access is necessary, wrap in testable abstractions
* Consider using framework-provided exit code management

---

### 3.4 Low Findings

#### Comment Inaccuracy in attemptRetryDelivery

**Location:** `src/prototype/subscribeToRetryTopic.ts` lines 8-10

```typescript
// This function is intentionally left empty.
// It is used to satisfy the type requirements of the subscribe function.
// The actual message handling is done in the attemptRetryDelivery function.
```

The function is not empty—it calls `attemptRetryDelivery`. This comment is misleading.

---

#### Duplicate Service Bus Client Management

**Location:**

* `src/prototype/provideServiceBusClient.ts`
* `src/utils/serviceBusMessageUtils.ts`

**Description:** Two different patterns for Service Bus clients:

1. `provideServiceBusClient()` creates a cached singleton
2. `sendMessages()` in utils creates a new client per call

This inconsistency could lead to connection pool exhaustion.

---

## 4. Dependency and Layer Analysis

### 4.1 Logical Layer Mapping

| Layer          | Expected Contents  | Actual Folders                      | Assessment                       |
| -------------- | ------------------ | ----------------------------------- | -------------------------------- |
| Presentation   | Routes, schemas    | `routes/`, `middleware/`            | **Mixed** - minimal HTTP usage   |
| Application    | Orchestration      | `prototype/` (partial)              | **Mixed** - combined with domain |
| Domain         | Business logic     | `prototype/` (partial), `types/`    | **Mixed** - no clear separation  |
| Infrastructure | Azure SDK adapters | `prototype/`, `utils/`, `services/` | **Poor** - scattered             |
| Cross-cutting  | Logging, config    | `internal/`, `constants/`           | **Good**                         |

### 4.2 Dependency Direction Issues

| Issue                    | From                      | To                              | Violation                         |
| ------------------------ | ------------------------- | ------------------------------- | --------------------------------- |
| Infrastructure in domain | `attemptRetryDelivery.ts` | `republishRetry.ts` → Azure SDK | Domain knows about infrastructure |
| Mixed orchestration      | `subscribeToSendTopic.ts` | Both domain logic and infra     | Application layer undefined       |

### 4.3 Coupling Assessment

| Component              | Coupling Level | Risk                                                |
| ---------------------- | -------------- | --------------------------------------------------- |
| Azure Service Bus SDK  | **Risky**      | Used directly in multiple files without abstraction |
| Axios                  | **Acceptable** | Wrapped in `postMessage.ts`                         |
| CLA Framework packages | **Acceptable** | Abstracted through `internal/frameworkAdapter/`     |
| Fastify                | **Acceptable** | Minimal coupling due to minimal HTTP usage          |

---

## 5. Testability Assessment

### 5.1 Test Organization

| Test Type | Location         | Scope Filtering       | Count      |
| --------- | ---------------- | --------------------- | ---------- |
| Unit      | `test/unit/`     | `TEST_SCOPE=unit`     | \~15 files |
| Contract  | `test/contract/` | `TEST_SCOPE=contract` | 3 files    |
| Service   | `test/service/`  | `TEST_SCOPE=service`  | 4 files    |

**Strengths:**

* Clear separation of test scopes
* Vitest configuration properly filters by `TEST_SCOPE`
* Unit tests use mocking (`vi.mock`) appropriately

### 5.2 Testability Barriers

| Barrier                       | Location                       | Impact                                               |
| ----------------------------- | ------------------------------ | ---------------------------------------------------- |
| Module-level cache instances  | `cached.ts`, `cachedForKey.ts` | Caches persist across tests; requires module mocking |
| Module-level scheduling store | `attemptRetryDelivery.ts`      | State persists; requires mock factory                |
| Global `app` singleton        | `internal/app.ts`              | Must mock/stub for unit tests                        |

### 5.3 Mocking Strategy

The codebase uses `vi.mock()` and `vi.hoisted()` patterns effectively:

```typescript
// Example from attemptRetryDelivery.test.ts
const { mockSchedulingStore } = vi.hoisted(() => ({
    mockSchedulingStore: {
        getRetryScheduling: vi.fn(),
        setRetryScheduling: vi.fn(),
    },
}));
vi.mock('../../../src/prototype/createSchedulingStore.js', () => ({
    createSchedulingStore: () => mockSchedulingStore,
}));
```

### 5.4 Test Coverage Recommendation

> % Coverage report from v8
>
> | File      | % Stmts | % Branch | % Funcs | % Lines |
> | --------- | ------- | -------- | ------- | ------- |
> | All files | 78.81   | 92.25    | 67.74   | 78.81   |

Overall coverage shows strong branch coverage at 92.25%, but lower statement and line coverage at 78.81% and function coverage at 67.74%. This indicates that while most conditional paths are evaluated, many executable lines and functions are not fully exercised.

The primary gaps are in untested or partially tested functions, likely including error handling, edge cases, fallback logic, and less frequently used utilities. The difference between branch and line coverage suggests that some code within covered branches is not being executed.

Priority should be given to:

* Uncovered or partially covered functions.
* Error and exception paths.
* Boundary and edge cases.
* Optional or configuration-driven logic.

Improving these areas will increase confidence in code reliability and reduce the risk of defects in untested paths.

While overall coverage is decent (78.81% statements/lines, 92.25% branches), function coverage remains low at 67.74%, and several critical areas have significant gaps. Notably, src/internal and src/prototype contain files with 0–50% coverage (e.g., createAppConfigOptions.ts, cachedForKey.ts, postMessage.ts, registerShutdownHook.ts, subscribe.ts). These modules likely represent important runtime behavior and integration logic.

We should prioritize adding unit tests for these low-coverage files and uncovered line ranges to improve function coverage, reduce risk in core logic, and strengthen confidence in refactors and future feature work.