agenticpay 1258-1261

PR_DESCRIPTION.md

@@ -1,77 +1,44 @@
-# PR: Implement funnel, ML forecasting, A/B testing, customer health (#852 #853 #854 #855)
+# Docs, payment correctness, payout reliability, and withdrawal security (#1258–#1261)
 
-**Branch:** `feat/issues-852-853-854-855`  
-**Base:** `main`  
-**Commits:**
-- `bfc5bb0` feat: add funnel conversion tracking, ML forecasting, A/B testing, customer health (#852 #853 #854 #855)
-- `05ac260` docs: add integration docs for funnel, ML forecast, AB testing, health
+Closes #1258
+Closes #1259
+Closes #1260
+Closes #1261
 
-Closes #852
-Closes #853
-Closes #854
-Closes #855
+## Scope
 
-## Summary
-This PR implements all four requested features:
+This PR delivers one focused increment for each issue, in issue order:
 
-### #853 Add funnel conversion tracking
-- Service `backend/src/services/funnel-tracking.ts` with `FunnelTrackingService`
-- Custom funnel definitions, step ordering, conversion window, per-step stats (entered, conversionRate, stepConversionRate, dropOff, avg/median/p95 time to next)
-- User journey tracking with completion and conversion time
-- Routes `backend/src/routes/funnel-tracking.ts` mounted at `/api/v1/funnels`
-- Tests `backend/src/services/__tests__/funnel-tracking.test.ts` (9 cases)
-- Docs `backend/docs/FUNNEL_TRACKING.md`
+### #1258 — Documentation discovery
 
-### #854 Implement revenue forecasting with ML
-- Service `backend/src/services/ml-forecast.ts` with `MLForecastService`
-- Ensemble: linear regression, polynomial (degree 2 via 3x3 solve), exponential smoothing, Holt-Winters seasonal, moving average
-- Cross-validated model selection (80/20 holdout, RMSE), confidence intervals (1.96*RMSE), seasonality detection via autocorrelation, trend analysis
-- Routes `backend/src/routes/ml-forecast.ts` mounted at `/api/v1/forecast/ml`
-- Tests `backend/src/services/__tests__/ml-forecast.test.ts`
-- Docs `backend/docs/ML_FORECAST.md`
+- Adds `docs/README.md` as a curated entry point for contributor, payment, SDK, operations, and security guides.
+- Links the root README to the index.
+- Adds a dependency-free Node test that verifies every local link in the index exists, and runs it from the root `test` command.
 
-### #852 Build A/B testing framework
-- Service `backend/src/services/ab-testing.ts` with `ABTestingService`
-- Deterministic MD5 bucket assignment, weighted variants, traffic allocation, lifecycle (draft/running/paused/completed/archived)
-- Statistical: two-proportion z-test, Wilson interval, Bayesian prob, sample size calculator
-- Routes `backend/src/routes/ab-testing.ts` mounted at `/api/v1/ab-tests`
-- Tests `backend/src/services/__tests__/ab-testing.test.ts`
-- Docs `backend/docs/AB_TESTING.md`
+### #1259 — Payment amount correctness
 
-### #855 Build customer health score system
-- Service `backend/src/services/customer-health.ts` with `CustomerHealthService`
-- Composite 0-100 score: payment_health 30% + frequency 20% + recency 20% + engagement 15% + support 15%, with penalties for churn signals
-- Levels champion/healthy/at_risk/critical, trend, riskReasons, recommendations, history, distribution, at-risk listing
-- Routes `backend/src/routes/customer-health.ts` mounted at `/api/v1/customer-health`
-- Tests `backend/src/services/__tests__/customer-health.test.ts`
-- Docs `backend/docs/CUSTOMER_HEALTH.md`
+- Rejects non-finite, zero, and negative amounts at the shared payment-provider router boundary.
+- Returns `INVALID_PAYMENT_AMOUNT` with status `400` before invoking or recording metrics for any provider.
+- Documents amount validation and provider selection/fallback behavior.
 
-## API Registration
-Updated `backend/src/index.ts` to mount all new routers and fix missing `allowancesRouter` import.
+### #1260 — Payout state reliability
 
-## How to create PR (when GitHub auth available)
-```bash
-git push -u origin feat/issues-852-853-854-855
-gh pr create --title "feat: add funnel conversion tracking, ML forecasting, A/B testing, customer health (#852 #853 #854 #855)" \
-  --body "Closes #852, Closes #853, Closes #854, Closes #855" \
-  --base main --head feat/issues-852-853-854-855
-```
-Or fork first:
-```bash
-gh repo fork Smartdevs17/agenticpay --clone=false
-git remote add fork https://github.com/<your-user>/agenticpay.git
-git push -u fork feat/issues-852-853-854-855
-gh pr create --repo Smartdevs17/agenticpay --head <your-user>:feat/issues-852-853-854-855
-```
+- Rejects negative and non-finite pending payout balances.
+- Returns defensive copies from payout evaluation, history, and audit APIs, including cloned `Date` values.
+- Prevents callers from corrupting authoritative payout status, amount, or audit state by mutating returned objects.
+- Updates the payout approval guide with the new state-integrity guarantees.
 
-## Testing
-Tests are vitest-based, run with:
-```bash
-cd backend && npm test src/services/__tests__/funnel-tracking.test.ts src/services/__tests__/ml-forecast.test.ts src/services/__tests__/ab-testing.test.ts src/services/__tests__/customer-health.test.ts
-```
+### #1261 — Withdrawal decision security
 
-All services expose `resetForTests()` for isolation.
+- Applies the configured approver allowlist to rejection as well as approval.
+- Fails closed when a non-allowlisted withdrawal has no configured approvers.
+- Documents approver configuration, the API identity trust boundary, and velocity controls.
 
-## Verification
-- `git log --oneline bfc5bb0..HEAD`
-- `git diff main..HEAD --stat`
+## Tests
+
+- `npm run test:docs`
+- `npm --workspace backend test -- src/services/payments/__tests__/payment-router.test.ts src/services/payouts/__tests__/payout-engine.test.ts src/services/__tests__/issues-456-459.test.ts src/services/__tests__/withdrawal-allowlist.test.ts`
+- `npm run lint`
+- `npm run build`
+
+New regression coverage includes invalid payment amounts, invalid payout balances, mutation isolation for payout/audit snapshots, missing withdrawal approvers, unauthorized withdrawal rejection, and documentation link integrity.

README.md

@@ -504,7 +504,7 @@ For issues, questions, or suggestions:
 
 - **Issues**: GitHub Issues
 - **Discussions**: GitHub Discussions
-- **Documentation**: See [docs/](./docs) folder
+- **Documentation**: Start with the [documentation index](docs/README.md)
 
 ---
 

backend/src/services/__tests__/issues-456-459.test.ts

@@ -81,7 +81,7 @@ describe('Payout Engine (Issue 457)', () => {
 
     const approved = await engine.approveManualPayout(batches[0].id, { id: 'fin_1', role: 'finance_approver' });
     expect(approved).toBe(true);
-    expect(batches[0].status).toBe('completed');
+    expect(engine.getPayoutHistory()[0].status).toBe('completed');
   });
 });
 

backend/src/services/__tests__/withdrawal-allowlist.test.ts

@@ -5,6 +5,7 @@ import {
   removeFromAllowlist,
   isAllowlisted,
   createWithdrawalRequest,
+  getWithdrawalRequest,
   approveWithdrawal,
   rejectWithdrawal,
   markExecuted,
@@ -65,6 +66,33 @@ describe('withdrawal allowlist service', () => {
     expect(() => approveWithdrawal(request.id, 'eve')).toThrow(WithdrawalApprovalError);
   });
 
+  it('fails closed when no withdrawal approvers are configured', () => {
+    const request = createWithdrawalRequest({
+      walletId: 'wallet-unconfigured',
+      toAddress: 'GADDR_UNKNOWN',
+      amount: 50,
+      currency: 'XLM',
+      requestedBy: 'owner',
+    });
+
+    expect(() => approveWithdrawal(request.id, 'owner')).toThrow(/No approvers are configured/);
+    expect(() => rejectWithdrawal(request.id, 'owner')).toThrow(/No approvers are configured/);
+  });
+
+  it('rejects rejection attempts from non-authorized approvers', () => {
+    configureWallet('wallet-reject', { approvalThreshold: 1, approvers: ['alice'] });
+    const request = createWithdrawalRequest({
+      walletId: 'wallet-reject',
+      toAddress: 'GADDR_UNKNOWN',
+      amount: 50,
+      currency: 'XLM',
+      requestedBy: 'owner',
+    });
+
+    expect(() => rejectWithdrawal(request.id, 'eve')).toThrow(WithdrawalApprovalError);
+    expect(getWithdrawalRequest(request.id)?.status).toBe('pending_approval');
+  });
+
   it('rejects a withdrawal request and prevents further approval', () => {
     configureWallet('wallet-4', { approvalThreshold: 1, approvers: ['alice'] });
     const request = createWithdrawalRequest({

backend/src/services/payments/__tests__/payment-router.test.ts

@@ -53,6 +53,23 @@ describe('ProviderPaymentRouter', () => {
     if (result.ok) expect(result.value.providerId).toBe('soroban');
   });
 
+  it.each([0, -1, Number.NaN, Number.POSITIVE_INFINITY])(
+    'rejects invalid amount %s before invoking a provider',
+    async (amount) => {
+      const provider = new MockPaymentProvider('soroban');
+      registry.register(provider);
+
+      const result = await router.route({ ...baseInput, amount });
+
+      expect(result.ok).toBe(false);
+      if (!result.ok) {
+        expect(result.error.code).toBe('INVALID_PAYMENT_AMOUNT');
+        expect(result.error.statusCode).toBe(400);
+      }
+      expect(registry.getMetrics('soroban')).toMatchObject({ successCount: 0, errorCount: 0 });
+    },
+  );
+
   it('falls back to another healthy strategy when the primary one fails', async () => {
     registry.register(new MockPaymentProvider('soroban', { shouldFail: true }));
     registry.register(new MockPaymentProvider('evm'));

backend/src/services/payments/payment-router.ts

@@ -25,6 +25,10 @@ export class ProviderPaymentRouter extends BaseService {
   }
 
   async route(input: PaymentInput, preferredProviderId?: string): Promise<Result<PaymentOutput>> {
+    if (!Number.isFinite(input.amount) || input.amount <= 0) {
+      return this.fail('Payment amount must be a finite number greater than zero', 400, 'INVALID_PAYMENT_AMOUNT');
+    }
+
     const primaryId = selectProviderId(input, preferredProviderId, this.registry);
     const candidateIds = [primaryId, ...this.registry.list().filter((id) => id !== primaryId)];
 

backend/src/services/payouts/__tests__/payout-engine.test.ts

@@ -54,6 +54,28 @@ describe('PayoutEngine approval workflow', () => {
     expect(engine.getPayoutHistory()).toHaveLength(1);
   });
 
+  it.each([-1, Number.NaN, Number.POSITIVE_INFINITY])('rejects invalid pending amount %s', (amount) => {
+    const engine = engineWithSchedule();
+
+    expect(() => engine.setPendingAmount('merch_1', amount)).toThrow('INVALID_PENDING_PAYOUT_AMOUNT');
+    expect(engine.getPendingAmount('merch_1')).toBe(0);
+  });
+
+  it('returns snapshots that cannot mutate stored payout state', async () => {
+    const engine = engineWithSchedule({ autoApproveLimit: 500 });
+    engine.setPendingAmount('merch_1', 600);
+    const [created] = await engine.evaluatePayouts();
+
+    created.status = 'completed';
+    created.amount = 1;
+    created.createdAt.setUTCFullYear(1999);
+
+    const [stored] = engine.getPayoutHistory();
+    expect(stored.status).toBe('pending');
+    expect(stored.amount).toBe(600);
+    expect(stored.createdAt.getUTCFullYear()).not.toBe(1999);
+  });
+
   it('rejects approval from callers without an approver role', async () => {
     const engine = engineWithSchedule({ autoApproveLimit: 500 });
     engine.setPendingAmount('merch_1', 600);
@@ -92,5 +114,11 @@ describe('PayoutEngine approval workflow', () => {
     const trail = engine.getAuditTrail(batch.id);
     expect(trail.map((entry) => entry.action)).toEqual(['created', 'approved']);
     expect(trail[1].actorId).toBe('fin_1');
+
+    trail[0].actorId = 'tampered';
+    trail[0].timestamp.setUTCFullYear(1999);
+    const stored = engine.getAuditTrail(batch.id)[0];
+    expect(stored.actorId).toBe('system');
+    expect(stored.timestamp.getUTCFullYear()).not.toBe(1999);
   });
 });

backend/src/services/payouts/payout-engine.ts

@@ -45,6 +45,14 @@ export interface PayoutEngineOptions {
 
 const APPROVER_ROLES: readonly PayoutApproverRole[] = ['finance_approver', 'admin'];
 
+function snapshotBatch(batch: PayoutBatch): PayoutBatch {
+  return { ...batch, createdAt: new Date(batch.createdAt) };
+}
+
+function snapshotAuditEntry(entry: PayoutAuditEntry): PayoutAuditEntry {
+  return { ...entry, timestamp: new Date(entry.timestamp) };
+}
+
 function resolveApprovalThreshold(explicit?: number): number {
   const fromEnv = Number(process.env.PAYOUT_APPROVAL_THRESHOLD);
   const threshold = explicit ?? (fromEnv > 0 ? fromEnv : DEFAULT_PAYOUT_APPROVAL_THRESHOLD);
@@ -66,6 +74,9 @@ export class PayoutEngine {
   }
 
   setPendingAmount(merchantId: string, amount: number) {
+    if (!Number.isFinite(amount) || amount < 0) {
+      throw new Error('INVALID_PENDING_PAYOUT_AMOUNT');
+    }
     this.pendingAmounts.set(merchantId, amount);
   }
 
@@ -107,7 +118,7 @@ export class PayoutEngine {
         this.pendingAmounts.set(schedule.merchantId, 0);
         this.batches.push(batch);
         this.record(batch.id, 'created', 'system', requiresApproval ? 'Awaiting approval' : 'Auto-approved');
-        executedBatches.push(batch);
+        executedBatches.push(snapshotBatch(batch));
       }
     }
 
@@ -154,11 +165,12 @@ export class PayoutEngine {
   }
 
   getPayoutHistory(): PayoutBatch[] {
-    return this.batches;
+    return this.batches.map(snapshotBatch);
   }
 
   getAuditTrail(batchId?: string): PayoutAuditEntry[] {
-    return batchId ? this.auditTrail.filter(entry => entry.batchId === batchId) : [...this.auditTrail];
+    const entries = batchId ? this.auditTrail.filter(entry => entry.batchId === batchId) : this.auditTrail;
+    return entries.map(snapshotAuditEntry);
   }
 
   private assertApprover(approver: PayoutApprover): void {

backend/src/services/withdrawal-allowlist.ts

@@ -212,6 +212,15 @@ export function listWithdrawalRequests(walletId?: string): WithdrawalRequest[] {
 
 export class WithdrawalApprovalError extends Error {}
 
+function assertAuthorizedApprover(config: WalletWithdrawalConfig, approver: string): void {
+  if (config.approvers.length === 0) {
+    throw new WithdrawalApprovalError('No approvers are configured for this wallet');
+  }
+  if (!config.approvers.includes(approver)) {
+    throw new WithdrawalApprovalError(`'${approver}' is not an authorized approver for this wallet`);
+  }
+}
+
 export function approveWithdrawal(id: string, approver: string): WithdrawalRequest {
   const request = withdrawalRequests.get(id);
   if (!request) throw new WithdrawalApprovalError('Withdrawal request not found');
@@ -220,9 +229,7 @@ export function approveWithdrawal(id: string, approver: string): WithdrawalReque
   }
 
   const cfg = getOrCreateConfig(request.walletId);
-  if (cfg.approvers.length > 0 && !cfg.approvers.includes(approver)) {
-    throw new WithdrawalApprovalError(`'${approver}' is not an authorized approver for this wallet`);
-  }
+  assertAuthorizedApprover(cfg, approver);
   if (!request.approvals.includes(approver)) {
     request.approvals.push(approver);
   }
@@ -242,6 +249,9 @@ export function rejectWithdrawal(id: string, approver: string): WithdrawalReques
     throw new WithdrawalApprovalError(`Cannot reject a request in status '${request.status}'`);
   }
 
+  const cfg = getOrCreateConfig(request.walletId);
+  assertAuthorizedApprover(cfg, approver);
+
   request.rejections.push(approver);
   request.status = 'rejected';
   request.updatedAt = Date.now();

docs/PAYMENT_ROUTING.md

@@ -0,0 +1,13 @@
+# Payment Provider Routing
+
+The provider router selects a payment rail from the requested network first, then from the token or currency. A valid explicit provider preference takes precedence.
+
+## Amount contract
+
+`PaymentInput.amount` must be a finite number greater than zero. The router rejects zero, negative amounts, `NaN`, and positive or negative infinity with HTTP status `400` and code `INVALID_PAYMENT_AMOUNT` before any provider is invoked.
+
+Callers should still format amounts to the precision supported by the selected asset. Provider-specific conversion to on-chain or fiat minor units happens after this shared validation boundary.
+
+## Provider fallback
+
+When a provider returns a failure, the router tries the remaining registered providers in registration order. If every provider fails, it returns `PROVIDER_UNAVAILABLE` with HTTP status `502`.

docs/PAYOUT_APPROVALS.md

@@ -11,6 +11,9 @@ A payout batch is created with status `pending` when either condition is true:
 
 Otherwise the batch is auto-approved and marked `completed`.
 
+Pending balances must be finite and non-negative. The engine rejects invalid balance updates with
+`INVALID_PENDING_PAYOUT_AMOUNT` so a negative value, `NaN`, or infinity cannot poison future evaluations.
+
 ## Configuring the threshold
 
 Set `PAYOUT_APPROVAL_THRESHOLD` (see [ENV_VARS.md](../ENV_VARS.md)). If it is unset, the default is `10000`.
@@ -43,6 +46,9 @@ so the rejection cannot be bypassed.
 Every decision is recorded with the batch ID, action (`created`, `approved`, `rejected`, `retried`),
 actor ID, timestamp, and optional details. Query it with `PayoutEngine.getAuditTrail(batchId?)`.
 
+History and audit APIs return defensive snapshots. Changing a returned object or its `Date` values
+does not alter the engine's stored payout state.
+
 ## Reserved funds
 
 When a batch is created, its amount is moved out of the merchant's pending balance, so repeated