Skip to content

Refactor post-v1 architecture hotspots without changing payment behavior #25

Description

@Phloraxx

Context

The post-v1 correctness/security review fixed the release-blocking defects in PR #23. The remaining findings are maintainability and architecture debt rather than known production failures. They should be handled incrementally after the bugfix PR is merged and proven in production.

The goal is to reduce the chance that future changes introduce state-transition, retry, evidence-matching or startup-wiring regressions without changing the current external API or payment semantics.

Scope

1. Split the API router by domain

internal/api/api.go has become a large registration and handler surface covering payments, SMS, reviews, reconciliation, refunds, backups and Google Messages.

  • keep one composition/root registration point
  • move handlers into domain-focused files or small handler types
  • centralize repeated authentication, JSON decoding and domain-error response behavior
  • preserve all route paths and response formats

2. Reduce branching in financial state machines

The highest-complexity paths are currently:

  • SMS ingestion
  • automatic and manual payment matching
  • reconciliation import/classification
  • review resolution
  • refund request/update transitions

Refactor these into explicit, testable stages such as validation, lookup, classification, mutation and side-effect scheduling. Do not replace readable domain decisions with generic abstraction merely to reduce line count.

3. Extract shared durable-delivery infrastructure

Outgoing payment/refund webhooks and operator-alert notifications independently implement:

  • leasing/claiming
  • stale lease recovery
  • retries/backoff
  • redirect refusal
  • signing
  • response draining
  • terminal exhaustion

Extract only the common delivery mechanics while keeping their payload construction and persistence models domain-specific. The result must retain durable at-least-once delivery and event IDs.

4. Consolidate small shared helpers

Remove repeated implementations where semantics are truly identical, especially:

  • PocketBase date/filter formatting
  • safe string/rune truncation
  • reference normalization where the accepted formats are identical
  • JSON metadata normalization/size validation

Avoid a broad utils package; place helpers in narrowly named packages.

5. Strengthen lifecycle/integration coverage

Add tests around wiring that unit tests can currently miss:

  • all background runners started from main
  • shutdown cancellation
  • Google Messages start/reconnect/reauth/unpair lifecycle
  • durable alert and webhook workers recovering after restart
  • reconciliation evidence reviewed after quarantine but occurring inside the original safe window
  • same-second bank timestamps with lower precision than payment creation

Prefer a small composition/integration harness over attempting to unit-test all of main().

6. Simplify the operator UI structure

The operator UI has accumulated large page components, especially Operations and Settings.

  • split by operational domain
  • keep data loading and mutations near each feature
  • extract repeated record-table/action/dialog behavior only where it is genuinely shared
  • retain the strict CSP and avoid inline styles

Non-goals

  • no database or framework replacement
  • no microservices
  • no event bus
  • no generic workflow engine
  • no public API changes
  • no changes to exact-amount fingerprint allocation or bank-SMS source of truth
  • no redesign of the checkout UI

Suggested sequence

  1. Add composition/lifecycle integration tests.
  2. Split API handlers without behavioral changes.
  3. Refactor refund and payment matching state machines.
  4. Extract durable-delivery mechanics.
  5. Refactor SMS/reconciliation/reviews.
  6. Split operator UI pages.
  7. Consolidate narrowly shared helpers as duplication becomes obvious during the above work.

Each step should be a separate PR with unchanged acceptance behavior.

Acceptance criteria

  • full Go tests, race detector, vet and staticcheck pass
  • production image and fresh-volume acceptance pass
  • no route, response or webhook payload compatibility changes
  • no weakening of idempotency, RRN uniqueness, fingerprint reuse boundaries, redirect refusal, CSP or retention behavior
  • measurable reduction in function complexity and file size without introducing broad generic abstractions
  • operational documentation updated when lifecycle or ownership boundaries move

Priority

Non-blocking architecture debt. Begin after PR #23 is merged and stable in production.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions