diff --git a/.github/workflows/build-check.yml b/.github/workflows/build-check.yml index 4ba46668..23295728 100644 --- a/.github/workflows/build-check.yml +++ b/.github/workflows/build-check.yml @@ -25,6 +25,9 @@ jobs: - name: Install dependencies run: npm ci + - name: Check circular module dependencies + run: npm run check:circular + - name: Generate GraphQL TypeScript types run: | npm run graphql:codegen diff --git a/docs/CIRCULAR_DEPENDENCY_CHECK.md b/docs/CIRCULAR_DEPENDENCY_CHECK.md new file mode 100644 index 00000000..a66c2f9f --- /dev/null +++ b/docs/CIRCULAR_DEPENDENCY_CHECK.md @@ -0,0 +1,5 @@ +# Circular dependency check + +`npm run check:circular` scans production TypeScript modules under `src/`, resolves relative imports, and fails when a module is reachable from itself. Test files are excluded so test-only fixtures do not affect the application graph. TypeORM entity files are excluded because bidirectional relation metadata intentionally references both entity classes; application services and shared policy contracts remain in the checked graph. + +The check runs before `npm run build` and in the build workflow. Keep module boundaries directional; when a cycle is found, move shared contracts into a lower-level module or inject the dependency through an interface rather than suppressing the check. diff --git a/package.json b/package.json index 4707339e..ec84bd68 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,8 @@ "private": true, "license": "MIT", "scripts": { - "prebuild": "rimraf dist", + "prebuild": "rimraf dist && npm run check:circular", + "check:circular": "node scripts/check-circular-dependencies.js", "build:tsc": "tsc -p tsconfig.build.json", "build:plain-tsc": "tsc --project tsconfig.json --pretty --listEmittedFiles", "build": "nest build", diff --git a/scripts/check-circular-dependencies.js b/scripts/check-circular-dependencies.js new file mode 100644 index 00000000..8a28c5f3 --- /dev/null +++ b/scripts/check-circular-dependencies.js @@ -0,0 +1,80 @@ +const fs = require("node:fs"); +const path = require("node:path"); + +function sourceFiles(directory) { + return fs.readdirSync(directory, { withFileTypes: true }).flatMap((entry) => { + const fullPath = path.join(directory, entry.name); + if (entry.isDirectory()) return sourceFiles(fullPath); + if (!entry.isFile() || !entry.name.endsWith(".ts")) return []; + // TypeORM entity relation callbacks intentionally reference each other; + // those metadata links do not form an application module cycle. + if (directory.split(path.sep).includes("entities")) return []; + if (entry.name.endsWith(".spec.ts") || entry.name.endsWith(".test.ts")) return []; + return [fullPath]; + }); +} + +function resolveImport(fromFile, specifier) { + if (!specifier.startsWith(".")) return undefined; + const base = path.resolve(path.dirname(fromFile), specifier); + const candidates = [base, `${base}.ts`, path.join(base, "index.ts")]; + return candidates.find((candidate) => fs.existsSync(candidate) && fs.statSync(candidate).isFile()); +} + +function dependencyGraph(root) { + const files = sourceFiles(root); + const known = new Set(files.map((file) => path.normalize(file))); + const graph = new Map(); + const importPattern = /(?:import(?:[\s\S]*?from\s*|\s*)|export[\s\S]*?from\s*)["']([^"']+)["']/g; + + for (const file of files) { + const dependencies = new Set(); + const contents = fs.readFileSync(file, "utf8"); + for (const match of contents.matchAll(importPattern)) { + const dependency = resolveImport(file, match[1]); + if (dependency && known.has(path.normalize(dependency))) dependencies.add(path.normalize(dependency)); + } + graph.set(path.normalize(file), [...dependencies]); + } + return graph; +} + +function findCycles(graph) { + const visited = new Set(); + const active = new Set(); + const stack = []; + const cycles = []; + + function visit(file) { + if (active.has(file)) { + const start = stack.indexOf(file); + cycles.push([...stack.slice(start), file]); + return; + } + if (visited.has(file)) return; + visited.add(file); + active.add(file); + stack.push(file); + for (const dependency of graph.get(file) || []) visit(dependency); + stack.pop(); + active.delete(file); + } + + for (const file of graph.keys()) visit(file); + return cycles; +} + +function run(root = path.resolve(__dirname, "..", "src")) { + const cycles = findCycles(dependencyGraph(root)); + if (cycles.length) { + console.error("Circular TypeScript dependencies detected:"); + for (const cycle of cycles) console.error(` ${cycle.map((file) => path.relative(process.cwd(), file)).join(" -> ")}`); + return 1; + } + console.log("No circular TypeScript dependencies found."); + return 0; +} + +if (require.main === module) process.exitCode = run(process.argv[2] ? path.resolve(process.argv[2]) : undefined); + +module.exports = { dependencyGraph, findCycles, run }; diff --git a/scripts/check-circular-dependencies.spec.js b/scripts/check-circular-dependencies.spec.js new file mode 100644 index 00000000..ca635603 --- /dev/null +++ b/scripts/check-circular-dependencies.spec.js @@ -0,0 +1,14 @@ +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); +const { findCycles, dependencyGraph } = require("./check-circular-dependencies"); + +describe("circular dependency checker", () => { + it("detects a cycle in relative imports", () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "trellis-cycle-")); + fs.writeFileSync(path.join(root, "a.ts"), 'import "./b";'); + fs.writeFileSync(path.join(root, "b.ts"), 'import "./a";'); + expect(findCycles(dependencyGraph(root))).toHaveLength(1); + fs.rmSync(root, { recursive: true, force: true }); + }); +}); diff --git a/src/config/env.validation.ts b/src/config/env.validation.ts index c4285610..2ff6fa31 100644 --- a/src/config/env.validation.ts +++ b/src/config/env.validation.ts @@ -269,6 +269,16 @@ export class EnvironmentVariables { @Transform(({ value }) => value === "true") RECONCILIATION_HORIZON_POLLING?: boolean = false; + /** Absolute drift that emits reconciliation.variance.alert. */ + @IsOptional() + @IsString() + RECONCILIATION_VARIANCE_ALERT_THRESHOLD?: string = "0.0100000"; + + /** Maximum absolute drift eligible for an authorized auto-reimbursement worker. */ + @IsOptional() + @IsString() + AUTO_REIMBURSEMENT_MAX_LIMIT?: string = "1.0000000"; + // Blockchain configuration @IsNumber() @Transform(({ value }) => parseInt(value, 10) || 1) diff --git a/src/policy/policy-shared.types.ts b/src/policy/policy-shared.types.ts new file mode 100644 index 00000000..481b2749 --- /dev/null +++ b/src/policy/policy-shared.types.ts @@ -0,0 +1,8 @@ +export type TradeSide = "buy" | "sell"; + +export interface TradingPolicy { + enabled: boolean; + maxOrderAmount: number; + allowedAssets?: string[]; + allowedSides: TradeSide[]; +} diff --git a/src/policy/policy-simulation.types.ts b/src/policy/policy-simulation.types.ts index 4f55f608..e939c3b3 100644 --- a/src/policy/policy-simulation.types.ts +++ b/src/policy/policy-simulation.types.ts @@ -1,4 +1,4 @@ -import type { TradeSide, TradingPolicy } from "./policy.types"; +import type { TradeSide, TradingPolicy } from "./policy-shared.types"; /** * Policy domains supported by the policy simulation preview engine. diff --git a/src/policy/policy.types.ts b/src/policy/policy.types.ts index 56a62c84..79faafbc 100644 --- a/src/policy/policy.types.ts +++ b/src/policy/policy.types.ts @@ -1,4 +1,5 @@ -export type TradeSide = "buy" | "sell"; +export type { TradeSide, TradingPolicy } from "./policy-shared.types"; +import type { TradeSide, TradingPolicy } from "./policy-shared.types"; /** The input required to make a business-policy decision about a trade. */ export interface TradePolicyInput { @@ -26,12 +27,5 @@ export type PolicyDecision = | { allowed: true } | { allowed: false; violation: PolicyViolation }; -export interface TradingPolicy { - enabled: boolean; - maxOrderAmount: number; - allowedAssets?: string[]; - allowedSides: TradeSide[]; -} - export * from "./policy-simulation.types"; diff --git a/src/reconciliation/reconciliation.module.ts b/src/reconciliation/reconciliation.module.ts index bb689cd3..79048c10 100644 --- a/src/reconciliation/reconciliation.module.ts +++ b/src/reconciliation/reconciliation.module.ts @@ -15,9 +15,7 @@ import { FeatureFlagRolloutGuard } from "../config/feature-flag-rollout.guard"; import { AuthModule } from "../core/auth/auth.module"; import { UserModule } from "../core/user/user.module"; - -import { AuthModule } from "../core/auth/auth.module"; -import { UserModule } from "../core/user/user.module"; +import { VarianceAnomalyService } from "./variance-anomaly.service"; @Module({ imports: [ @@ -39,12 +37,14 @@ import { UserModule } from "../core/user/user.module"; TransactionDetailsService, FeatureFlagsService, FeatureFlagRolloutGuard, + VarianceAnomalyService, ], exports: [ ReconciliationService, DryRunReconciliationService, TransactionDetailsService, FeatureFlagRolloutGuard, + VarianceAnomalyService, ], }) export class ReconciliationModule {} diff --git a/src/reconciliation/reconciliation.service.ts b/src/reconciliation/reconciliation.service.ts index 18a5e8b9..a81fef94 100644 --- a/src/reconciliation/reconciliation.service.ts +++ b/src/reconciliation/reconciliation.service.ts @@ -1,4 +1,4 @@ -import { Injectable, Logger, NotFoundException } from "@nestjs/common"; +import { Injectable, Logger, NotFoundException, Optional } from "@nestjs/common"; import { ConfigService } from "@nestjs/config"; import { InjectRepository } from "@nestjs/typeorm"; import { In, Repository } from "typeorm"; @@ -20,6 +20,7 @@ import { StellarTransactionStatus, } from "./entities/stellar-transaction.entity"; import { telemetryService } from "../observability/telemetry.service"; +import { VarianceAnomalyService } from "./variance-anomaly.service"; const SCALE = 7n; const SCALE_FACTOR = 10n ** SCALE; @@ -36,7 +37,8 @@ export class ReconciliationService { private readonly transactionRepo: Repository, @InjectRepository(ReconciliationAudit) private readonly auditRepo: Repository, - private readonly configService: ConfigService + private readonly configService: ConfigService, + @Optional() private readonly varianceAnomalyService?: VarianceAnomalyService, ) {} async createInvoice( @@ -344,6 +346,15 @@ export class ReconciliationService { assetCode: invoice.assetCode, }, }); + // A partial payment is an expected intermediate state, not balance drift. + // Evaluate only once the invoice is fully settled (including overpayment). + if (isPaid) { + this.varianceAnomalyService?.evaluate( + invoice.invoiceId, + invoice.expectedAmount, + invoice.paidAmount, + ); + } endTelemetry("success", undefined, { invoiceId: invoice.invoiceId, diff --git a/src/reconciliation/variance-anomaly.service.spec.ts b/src/reconciliation/variance-anomaly.service.spec.ts new file mode 100644 index 00000000..93e7e0c6 --- /dev/null +++ b/src/reconciliation/variance-anomaly.service.spec.ts @@ -0,0 +1,41 @@ +import { ConfigService } from "@nestjs/config"; +import { EventEmitter2 } from "@nestjs/event-emitter"; +import { VarianceAnomalyService } from "./variance-anomaly.service"; + +describe("VarianceAnomalyService", () => { + function makeService(values: Record = {}) { + const config = new ConfigService({ + RECONCILIATION_VARIANCE_ALERT_THRESHOLD: "0.0100000", + AUTO_REIMBURSEMENT_MAX_LIMIT: "1.0000000", + ...values, + }); + return { service: new VarianceAnomalyService(config, new EventEmitter2()), config }; + } + + it("queues micro discrepancies below both safety thresholds", () => { + const { service } = makeService(); + expect(service.evaluate("invoice-1", "10.0000000", "9.9950000")).toMatchObject({ + variance: "0.0050000", + disposition: "auto_reimburse", + alertRequired: false, + auditRequired: true, + }); + }); + + it("requires manual review and an alert for material drift", () => { + const { service } = makeService(); + expect(service.evaluate("invoice-2", "10.0000000", "9.9700000")).toMatchObject({ + variance: "0.0300000", + disposition: "manual_review", + alertRequired: true, + }); + }); + + it("never auto reimburses a variance at or above the configured cap", () => { + const { service } = makeService({ + RECONCILIATION_VARIANCE_ALERT_THRESHOLD: "2.0000000", + AUTO_REIMBURSEMENT_MAX_LIMIT: "0.0100000", + }); + expect(service.evaluate("invoice-3", "1.0000000", "0.9800000").disposition).toBe("manual_review"); + }); +}); diff --git a/src/reconciliation/variance-anomaly.service.ts b/src/reconciliation/variance-anomaly.service.ts new file mode 100644 index 00000000..605917fb --- /dev/null +++ b/src/reconciliation/variance-anomaly.service.ts @@ -0,0 +1,86 @@ +import { Injectable, Logger } from "@nestjs/common"; +import { ConfigService } from "@nestjs/config"; +import { EventEmitter2 } from "@nestjs/event-emitter"; + +const SCALE = 7n; +const SCALE_FACTOR = 10n ** SCALE; + +export type VarianceDisposition = "within_tolerance" | "auto_reimburse" | "manual_review"; + +export interface VarianceEvaluation { + referenceId: string; + expectedAmount: string; + actualAmount: string; + variance: string; + disposition: VarianceDisposition; + alertRequired: boolean; + auditRequired: boolean; +} + +/** + * Applies one deterministic policy to every reconciliation variance. The + * service emits an event for the notification adapters (Slack, PagerDuty, or + * email) and never performs a reimbursement itself; a separately authorized + * worker can consume the auto-reimbursement event and leave an audit trail. + */ +@Injectable() +export class VarianceAnomalyService { + private readonly logger = new Logger(VarianceAnomalyService.name); + + constructor( + private readonly config: ConfigService, + private readonly events: EventEmitter2, + ) {} + + evaluate(referenceId: string, expectedAmount: string, actualAmount: string): VarianceEvaluation { + const expected = this.toUnits(expectedAmount); + const actual = this.toUnits(actualAmount); + const absoluteVariance = expected >= actual ? expected - actual : actual - expected; + const alertThreshold = this.toUnits( + this.config.get("RECONCILIATION_VARIANCE_ALERT_THRESHOLD", "0.0100000"), + ); + const reimbursementCap = this.toUnits( + this.config.get("AUTO_REIMBURSEMENT_MAX_LIMIT", "1.0000000"), + ); + + const disposition: VarianceDisposition = + absoluteVariance === 0n + ? "within_tolerance" + : absoluteVariance < reimbursementCap && absoluteVariance < alertThreshold + ? "auto_reimburse" + : "manual_review"; + const evaluation: VarianceEvaluation = { + referenceId, + expectedAmount: this.fromUnits(expected), + actualAmount: this.fromUnits(actual), + variance: this.fromUnits(absoluteVariance), + disposition, + alertRequired: absoluteVariance >= alertThreshold, + auditRequired: disposition !== "within_tolerance", + }; + + if (evaluation.alertRequired) { + this.events.emit("reconciliation.variance.alert", evaluation); + this.logger.error( + `Reconciliation variance ${evaluation.variance} for ${referenceId} requires ${disposition}`, + ); + } else if (disposition === "auto_reimburse") { + this.events.emit("reconciliation.variance.auto_reimburse", evaluation); + this.logger.warn(`Micro-variance ${evaluation.variance} queued for ${referenceId}`); + } + return evaluation; + } + + private toUnits(value: string): bigint { + const normalized = String(value).trim(); + if (!/^\d+(\.\d+)?$/.test(normalized)) throw new Error(`Invalid reconciliation amount: ${value}`); + const [whole, fraction = ""] = normalized.split("."); + return BigInt(whole) * SCALE_FACTOR + BigInt(fraction.padEnd(Number(SCALE), "0").slice(0, Number(SCALE))); + } + + private fromUnits(units: bigint): string { + const whole = units / SCALE_FACTOR; + const fraction = (units % SCALE_FACTOR).toString().padStart(Number(SCALE), "0"); + return `${whole}.${fraction}`; + } +} diff --git a/test/TRANSACTION_ISOLATION.md b/test/TRANSACTION_ISOLATION.md new file mode 100644 index 00000000..a58be133 --- /dev/null +++ b/test/TRANSACTION_ISOLATION.md @@ -0,0 +1,13 @@ +# Database isolation for E2E specs + +Use `withDatabaseTransaction(dataSource, body)` for one-off database-backed +flows, or `installTransactionIsolation(dataSource, { beforeEach, afterEach })` +for a suite. The harness creates a dedicated TypeORM query runner, starts a +`READ COMMITTED` transaction before the spec, passes its manager to the test, +and always rolls it back and releases the runner afterward. + +Tests must obtain repositories from the provided manager (or pass that manager +through the service under test). This keeps inserted rows invisible to later +specs, permits parallel workers to use separate connections, and removes the +need for destructive table truncation. Rollback also runs when the spec throws, +so a failed test cannot pollute the next run. diff --git a/test/transactional-database-harness.spec.ts b/test/transactional-database-harness.spec.ts new file mode 100644 index 00000000..704b06c1 --- /dev/null +++ b/test/transactional-database-harness.spec.ts @@ -0,0 +1,45 @@ +import { withDatabaseTransaction } from "./transactional-database-harness"; + +describe("transactional database harness", () => { + it("rolls back and releases after a successful spec body", async () => { + const queryRunner = { + isTransactionActive: true, + manager: { query: jest.fn().mockResolvedValue([]) }, + connect: jest.fn().mockResolvedValue(undefined), + startTransaction: jest.fn().mockResolvedValue(undefined), + rollbackTransaction: jest.fn().mockImplementation(async () => { + queryRunner.isTransactionActive = false; + }), + release: jest.fn().mockResolvedValue(undefined), + } as any; + const dataSource = { createQueryRunner: () => queryRunner } as any; + + await withDatabaseTransaction(dataSource, async (manager) => { + await manager.query("INSERT INTO fixture VALUES (1)"); + }); + + expect(queryRunner.startTransaction).toHaveBeenCalledWith("READ COMMITTED"); + expect(queryRunner.rollbackTransaction).toHaveBeenCalledTimes(1); + expect(queryRunner.release).toHaveBeenCalledTimes(1); + }); + + it("rolls back before releasing when the spec body fails", async () => { + const queryRunner = { + isTransactionActive: true, + manager: {}, + connect: jest.fn().mockResolvedValue(undefined), + startTransaction: jest.fn().mockResolvedValue(undefined), + rollbackTransaction: jest.fn().mockImplementation(async () => { + queryRunner.isTransactionActive = false; + }), + release: jest.fn().mockResolvedValue(undefined), + } as any; + await expect( + withDatabaseTransaction({ createQueryRunner: () => queryRunner } as any, async () => { + throw new Error("spec failed"); + }), + ).rejects.toThrow("spec failed"); + expect(queryRunner.rollbackTransaction).toHaveBeenCalledTimes(1); + expect(queryRunner.release).toHaveBeenCalledTimes(1); + }); +}); diff --git a/test/transactional-database-harness.ts b/test/transactional-database-harness.ts new file mode 100644 index 00000000..14265fd7 --- /dev/null +++ b/test/transactional-database-harness.ts @@ -0,0 +1,51 @@ +import type { DataSource, EntityManager, QueryRunner } from "typeorm"; + +export interface TransactionHooks { + beforeEach: (callback: () => Promise) => void; + afterEach: (callback: () => Promise) => void; +} + +/** Run a spec body in a transaction that is always rolled back. */ +export async function withDatabaseTransaction( + dataSource: DataSource, + body: (manager: EntityManager) => Promise, +): Promise { + const queryRunner = dataSource.createQueryRunner(); + await queryRunner.connect(); + await queryRunner.startTransaction("READ COMMITTED"); + try { + return await body(queryRunner.manager); + } finally { + if (queryRunner.isTransactionActive) await queryRunner.rollbackTransaction(); + await queryRunner.release(); + } +} + +/** + * Install per-spec isolation for E2E suites. Repositories used by the spec + * must be obtained from `manager()` so writes stay inside this query runner. + */ +export function installTransactionIsolation( + dataSource: DataSource, + hooks: TransactionHooks, +): () => EntityManager { + let queryRunner: QueryRunner | undefined; + hooks.beforeEach(async () => { + queryRunner = dataSource.createQueryRunner(); + await queryRunner.connect(); + await queryRunner.startTransaction("READ COMMITTED"); + }); + hooks.afterEach(async () => { + if (!queryRunner) return; + try { + if (queryRunner.isTransactionActive) await queryRunner.rollbackTransaction(); + } finally { + await queryRunner.release(); + queryRunner = undefined; + } + }); + return () => { + if (!queryRunner) throw new Error("Database transaction has not been started"); + return queryRunner.manager; + }; +}