NetForge/.planning/research/PITFALLS.md

# Domain Pitfalls: ISP Management SaaS

**Domain:** Multi-tenant ISP operations management (billing, accounting, inventory, ticketing)
**Researched:** 2026-03-04
**Confidence note:** Based on domain analysis of the PRD and known engineering patterns for billing systems, double-entry accounting, multi-tenant SaaS, and ISP-specific operational software. WebSearch was unavailable during this session; claims draw on well-established engineering knowledge. Flag LOW-confidence items for validation before implementation.

---

## Critical Pitfalls

Mistakes that cause rewrites or fundamental architectural breakage.

---

### Pitfall 1: Leaking Tenant Data Due to Missing `tenant_id` on Every Query

**What goes wrong:** A developer writes a query without scoping it to the current tenant. One ISP's staff sees another ISP's clients, invoices, or payments. This is not just a bug — it is a compliance and trust catastrophe that can destroy the product.

**Why it happens:** Early in development, the app has only one tenant (the owner's own ISP), so queries work correctly without `tenant_id` filters. The pattern only breaks when a second tenant onboards. By then, dozens of queries may be unscoped.

**Consequences:**
- Cross-tenant data exposure (data breach)
- Regulatory liability
- Loss of all tenants if discovered
- Complete rewrite of the data access layer

**Prevention:**
- Implement a Row-Level Security (RLS) policy at the database level (Postgres RLS is ideal) so queries that forget `tenant_id` return zero rows rather than wrong rows
- Create a base repository/service class that automatically appends `tenant_id` to every query; all data-access code must extend this class
- Never allow raw queries to bypass the scoping layer
- Write integration tests that spin up two tenants with overlapping data and assert zero cross-contamination

**Warning signs:**
- Any raw SQL query without a `WHERE tenant_id = ?` clause
- Queries using global IDs (e.g., `/invoices/42`) instead of tenant-scoped IDs
- No automated test that verifies cross-tenant isolation

**Phase:** Address in Phase 1 before any feature work. Multi-tenant scoping is infrastructure, not a feature.

---

### Pitfall 2: Treating Billing as Simple Invoice Generation

**What goes wrong:** The team builds a billing engine that generates invoices on a schedule — and stops there. The hard billing problems (proration, plan changes mid-cycle, backdated payments, credit memos, partial payments, grace periods, suspension/reinstatement timing) are discovered only when the first real ISP uses it and finds edge cases daily.

**Why it happens:** "Generate an invoice" looks simple. The complexity is in the state machine: what happens when a client pays half? What if they pay late and service was already suspended? What if a plan changes on day 15 of a 30-day cycle? These scenarios are invisible until production.

**Consequences:**
- Incorrect account balances (ISP loses or double-charges money)
- Manual reconciliation work for every edge case
- Client disputes
- Accounting ledger entries become inconsistent (billing and accounting diverge)

**Prevention:**
- Model billing as a state machine from day one: `pending → active → overdue → suspended → reinstated | cancelled`
- Define explicit business rules for every transition before writing code (who designed this? write it down)
- Support partial payment application and carry-forward balances
- Support credit memos for overpayments
- Proration: define whether you support it in v1 or not — if not, lock plan changes to billing cycle boundaries and enforce this in the UI
- Keep billing events (invoice generated, payment applied, credit issued) as immutable ledger records; never mutate them

**Warning signs:**
- Billing logic that directly updates an "amount_due" column rather than computing it from an event log
- No concept of a payment application record (just marking an invoice "paid")
- No test coverage for partial payment scenarios

**Phase:** Phase 1. The state machine and ledger approach must be established before the first invoice is generated. Retrofitting this is a rewrite.

---

### Pitfall 3: Building a Fake Accounting System (Cash Tracking Disguised as Double-Entry)

**What goes wrong:** The developer implements revenue and expense tracking as simple balance fields or single-entry logs, then wraps it in accounting-sounding labels. The system cannot produce a valid Balance Sheet, Income Statement, or Trial Balance. When the owner tries to close the books or show financials to an accountant, the numbers are wrong or cannot be reconciled.

**Why it happens:** Double-entry bookkeeping is counterintuitive to developers who think in terms of CRUD. It is easier to build `payments` and `expenses` tables than a proper Journal → Ledger → Trial Balance → Financial Statements pipeline.

**Consequences:**
- Financial reports that look right but fail when cross-checked
- Cannot reconcile cash collected by collectors against accounting records
- Tax/audit failures
- Complete rewrite of the financial layer — which also breaks all existing reports

**Prevention:**
- Implement a proper Journal Entry table as the single source of truth: every financial event creates one or more balanced journal entries (debits = credits, always)
- Chart of Accounts must be established before any financial data is recorded
- Every billing event, payment, expense, and payroll transaction must post to the journal; reports are derived from the journal, never from separate tables
- Validate that debits = credits on every journal entry at the database constraint level, not just application level
- Hire or consult with an accountant to validate the COA and journal entry logic before Phase 1 ships

**Warning signs:**
- A "revenue" column somewhere that gets incremented directly
- Separate `income` and `expense` tables that are never cross-validated
- No `journal_entries` table with debit/credit columns
- Reports that query `payments` directly rather than the ledger

**Phase:** Phase 1. The journal/ledger foundation must exist before any money moves in the system. You cannot add real accounting to an existing fake-accounting system without a full rewrite.

---

### Pitfall 4: Collector Cash Reconciliation Has No Audit Trail

**What goes wrong:** Collectors collect cash door-to-door. The system records that a payment was made, but there is no chain-of-custody record: who collected it, when, how much cash they were holding, when they remitted it to the office, and who verified receipt. Disputes between collectors and management cannot be resolved. Cash goes missing with no accountability.

**Why it happens:** Developers model this as "payment → done." The collector workflow (collect → hold → remit → verify) is an ISP-specific operational pattern that most billing system guides do not cover.

**Consequences:**
- Cash theft with no detection mechanism
- Collector disputes with no resolution path
- Accounting entries that don't match physical cash
- ISP owner cannot trust their own collection data

**Prevention:**
- Model collector sessions: a collector "opens" a collection run, logs each payment during the run, then "closes" the run with a total remittance
- Record the collector ID, timestamp, amount, and client on each payment
- Require an office staff member to verify and sign off on each cash remittance; this creates a two-party audit trail
- The accounting entry for cash receipt should only post when the remittance is verified, not when the collector logs it
- Generate a daily collector summary report: payments collected vs. remitted vs. variance

**Warning signs:**
- Payments table has no `collector_id` column
- No concept of a "collection run" or "remittance session"
- Payments post to accounting immediately without a verification step
- No variance report between collected and remitted amounts

**Phase:** Phase 1 (billing and payment collection). This is core to the cash flow visibility goal, not an enhancement.

---

### Pitfall 5: Inventory Tracking That Doesn't Survive Real-World Operations

**What goes wrong:** Inventory is modeled as a simple stock count. When technicians deploy equipment, swap units, return defective hardware, or take items from the warehouse without logging it, the inventory count diverges from reality within weeks. The system becomes "what we think we have" rather than "what we actually have."

**Why it happens:** Simple inventory is easy to build. The hard part is the movement model: equipment moves from warehouse → technician vehicle → client premises → (possibly back to warehouse or to another client). Each movement needs to be logged as an event, not just a count adjustment.

**Consequences:**
- ISP cannot find hardware they own
- Cannot calculate accurate asset value for the balance sheet
- Cannot determine if technicians are losing or stealing equipment
- Inventory reports are ignored because they're known to be wrong

**Prevention:**
- Model inventory as an event ledger: every movement is an immutable `inventory_movement` record (from location, to location, quantity, actor, timestamp)
- Stock counts are computed from the movement log, never stored as a mutable field
- Locations are typed: Warehouse, TechnicianVehicle, ClientPremises, Repair, Written-off
- Require technicians to log item assignments before a job order can be marked complete
- Periodic physical count reconciliation workflow: compare physical count to system count, log variance as an adjustment event

**Warning signs:**
- An `inventory_items` table with a `quantity` column that gets directly incremented/decremented
- No movement log or audit trail
- No concept of item location beyond "in stock" or "deployed"
- Job orders completable without logging what hardware was used

**Phase:** Phase 2 or 3 (after billing is stable), but the event-ledger model must be decided in Phase 1 architecture even if implementation is deferred.

---

### Pitfall 6: Role-Based Access Without Row-Level Enforcement

**What goes wrong:** The app checks roles at the UI level ("hide this button for technicians") but not at the API level. A Technician who discovers the API endpoint can access or modify billing records, financial data, or other clients' information.

**Why it happens:** RBAC is implemented as frontend permission guards. Backend enforcement is added as an afterthought, often incompletely.

**Consequences:**
- Security vulnerability exploitable by any technically savvy user
- Collectors can see all clients' financial data, not just their assigned routes
- Technicians can mark their own job orders complete without office verification
- Clients can access other clients' accounts

**Prevention:**
- Enforce permissions at the API/service layer, not the UI layer — every endpoint checks the caller's role before returning data
- For row-level access (e.g., a Collector can only see their own payment records), enforce this in the query scope, not a post-fetch filter
- Define permission matrix explicitly before coding: for each role × action × resource, is it allowed?
- Write API-level tests that call endpoints as each role and assert correct 403 responses

**Warning signs:**
- Permission checks only in frontend components or middleware that can be bypassed
- No API test suite that tests unauthorized access scenarios
- "We'll tighten security later" as a deferred task

**Phase:** Phase 1. Every API endpoint written from day one must have role enforcement. Retrofitting security is incomplete by definition.

---

## Moderate Pitfalls

Mistakes that cause delays, technical debt, or operational pain.

---

### Pitfall 7: Prepaid vs. Postpaid Billing Not Modeled as Distinct State Machines

**What goes wrong:** Prepaid and postpaid are implemented as a single billing flow with an `if` statement. The logic becomes a maze of conditionals. Edge cases in one model break the other. Adding postpaid features breaks prepaid behavior.

**Prevention:**
- Treat prepaid and postpaid as two separate billing strategies that share a common interface
- Prepaid: service activation requires payment first; no invoice generated until funds are available or committed
- Postpaid: service activates on cycle start; invoice generated at cycle end; grace period before suspension
- Define each strategy's state machine separately; compose shared logic (payment application, journal entries) rather than duplicating it

**Phase:** Phase 1. The model must be chosen during billing engine design.

---

### Pitfall 8: Soft Deletes Without Considering Accounting Immutability

**What goes wrong:** The team implements soft deletes (`deleted_at` column) on clients, invoices, and payments. An accountant or auditor needs records that existed in a prior period. Soft-deleted records either pollute current reports or disappear from historical reports.

**Prevention:**
- Financial records (invoices, payments, journal entries) must never be deleted — only voided or reversed via a compensating journal entry
- Client records: soft delete is acceptable, but all financial history must remain linked and accessible
- "Void" an invoice by creating a credit memo journal entry; never set `deleted_at` on a posted journal entry

**Phase:** Phase 1. Define archival strategy before any financial data is created.

---

### Pitfall 9: Ticket → Job Order Workflow Without Status Synchronization

**What goes wrong:** Tickets and job orders are separate tables with no status sync. A technician marks a job order complete, but the originating ticket remains "open." Staff sees open tickets for resolved issues. Clients see their issue as unresolved.

**Prevention:**
- Define the canonical status flow: Ticket (open → assigned → resolved → closed) maps to Job Order (created → assigned → in-progress → completed → verified)
- Job Order completion should trigger a Ticket status update (or require explicit ticket closure)
- One ticket can spawn one or more job orders (e.g., initial visit + follow-up); the ticket closes only when all job orders are verified
- Avoid duplicating status fields; derive ticket status from its job orders where possible

**Phase:** Phase 2 (ticketing module).

---

### Pitfall 10: MikroTik Integration Built Directly Into Business Logic

**What goes wrong:** Router commands are called directly from billing code. When the MikroTik is offline, invoices cannot be generated. When the billing logic changes, router integration code is tangled throughout. Testing becomes impossible without a real router.

**Prevention:**
- Isolate MikroTik integration behind an adapter interface: `RouterAdapter.suspend(clientId)`, `RouterAdapter.activate(clientId)`
- Billing engine calls the adapter; the adapter handles the actual RouterOS API call
- Make the adapter async and queue-based: if the router is offline, the command is queued and retried, not blocking the billing operation
- Mock the adapter in tests — billing logic tests should never require a live router

**Phase:** Phase 1 (if MikroTik integration is in scope). Adapter pattern must be established from first integration.

---

### Pitfall 11: Dashboard Metrics Computed at Query Time on Large Tables

**What goes wrong:** The executive dashboard runs `SUM(payments)`, `COUNT(overdue_clients)`, and similar aggregations live against full tables. With 500–2,000 subscribers per ISP and multiple ISPs, dashboard load times become unacceptable. Worse, some ISPs experience degraded billing performance because dashboard queries lock rows.

**Prevention:**
- Pre-aggregate key metrics on a schedule (daily totals, monthly summaries) into a `metrics_snapshots` table
- The dashboard queries snapshots, not raw tables
- Real-time counts (e.g., "currently overdue") use indexed, lightweight queries — not full-table scans
- Add database indexes on `(tenant_id, status)`, `(tenant_id, due_date)`, `(tenant_id, created_at)` from day one

**Phase:** Phase 1 architecture (indexes), Phase 2 or 3 (metrics snapshots if performance degrades).

---

### Pitfall 12: Technician Compensation Calculation Embedded in UI Layer

**What goes wrong:** Technician pay (per-job rates, monthly salary, deductions) is computed in the frontend or a report query rather than a dedicated payroll service. When business rules change (new job types, bonus structures), it requires UI changes and report rewrites rather than a single rule update.

**Prevention:**
- Build a `CompensationService` that encapsulates all pay calculation logic
- Job order completion triggers a compensation event record (not a direct pay update)
- Payroll period closing is a deliberate action that aggregates compensation events into a payroll record and posts journal entries
- Support both per-job and salary models as configurable per technician

**Phase:** Phase 2 (technician management).

---

## Minor Pitfalls

Mistakes that cause annoyance or rework but are recoverable.

---

### Pitfall 13: Plan Tier Configuration Hardcoded in Application Code

**What goes wrong:** ISP plan tiers (e.g., "5Mbps - ₱500/month") are defined as constants in code. When an ISP wants to add a plan or change pricing, a code deployment is required.

**Prevention:** Plans are tenant-configurable records in the database with name, price, billing cycle, and data limits. Admins manage plans via UI. No code changes needed for plan management.

**Phase:** Phase 1.

---

### Pitfall 14: Super-Admin Access Using Same Auth System as Tenant Users

**What goes wrong:** The platform owner (Super-Admin) authenticates through the same login system as ISP staff. A misconfiguration can accidentally scope the Super-Admin to a specific tenant, or a tenant admin can escalate privileges to Super-Admin.

**Prevention:**
- Super-Admin is a separate authentication context, ideally a separate login URL/subdomain
- Super-Admin role is not stored in the same `users` table as tenant users — separate table or a flag with stricter guards
- Super-Admin access bypasses tenant scoping explicitly and auditably, not by coincidence of missing `tenant_id` checks

**Phase:** Phase 1 (SaaS infrastructure).

---

### Pitfall 15: No Idempotency on Payment Logging

**What goes wrong:** A collector logs a payment and the request times out. They resubmit. The payment is recorded twice. The client's balance is double-credited. This is very hard to detect without an audit trail.

**Prevention:**
- Require an idempotency key on all payment creation requests (client-generated UUID)
- Server rejects duplicate idempotency keys within a time window
- Payment creation is a two-step operation: "draft" then "confirm" — timeout on confirm does not re-create

**Phase:** Phase 1.

---

### Pitfall 16: Assuming One Router = One ISP

**What goes wrong:** Data model is built with one router per tenant. ISPs frequently have multiple MikroTik routers across different geographic zones. Hardcoding a single-router assumption requires schema migration when this is discovered.

**Prevention:** Router is a `network_nodes` table with `(tenant_id, router_id, zone_name, ip_address)`. Clients are assigned to a router, not to the tenant globally. Build multi-router from the start even if the first ISP has only one.

**Phase:** Phase 1 (data model).

---

## Phase-Specific Warnings

| Phase Topic | Likely Pitfall | Mitigation |
|-------------|----------------|------------|
| Multi-tenant foundation | Cross-tenant data leakage | Postgres RLS + scoped repository pattern from day one |
| Billing engine design | State machine complexity, fake accounting | Define state machine and journal entry pattern before coding |
| Collector workflow | No audit trail, cash reconciliation gaps | Model collection sessions with two-party verification |
| Chart of Accounts | Built after features, causing retrofit | Design COA in Phase 1 even if reports come in Phase 2 |
| Inventory module | Mutable quantity counts | Event-ledger model; decide architecture even if deferred |
| Ticketing/job orders | Status desync between ticket and job order | One-to-many mapping with explicit sync rules |
| MikroTik integration | Tight coupling to billing logic | Adapter + async queue pattern from first integration |
| Dashboard | Live aggregation on large tables | Indexes in Phase 1; snapshots if needed later |
| Role enforcement | UI-only permission guards | API-layer enforcement tested for every role from day one |
| Technician payroll | Logic in reports/UI | CompensationService with compensation event records |
| Super-Admin | Privilege escalation or accidental tenant scoping | Separate auth context for platform owner |
| Payment logging | Duplicate payments on retry | Idempotency keys on all payment endpoints |

---

## Sources

- Domain analysis based on PRD (`isp_system_prd.md`) and PROJECT.md, verified against well-established patterns in:
  - Double-entry accounting system design (immutable journal, COA, ledger derivation)
  - Multi-tenant SaaS security (Postgres RLS, tenant-scoped repositories)
  - ISP billing state machines (prepaid/postpaid lifecycle, suspension/reinstatement)
  - Inventory event-ledger patterns (movement log vs. mutable quantity)
  - Collector cash-in-transit operational workflows (two-party reconciliation)
- Confidence: MEDIUM for architectural patterns (well-established), LOW for ISP-specific market research (WebSearch unavailable this session — recommend validating collector workflow and MikroTik integration specifics against real ISP operator feedback)