Add offline AMPS e2e scenario and manual review transfers#390
Merged
Connorbelez merged 4 commits intomainfrom Apr 10, 2026
Merged
Add offline AMPS e2e scenario and manual review transfers#390Connorbelez merged 4 commits intomainfrom
Connorbelez merged 4 commits intomainfrom
Conversation
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 25 minutes and 55 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (120)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
This was referenced Apr 9, 2026
Merged
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @Connorbelez, your pull request is larger than the review limit of 150000 diff characters
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Pull request overview
This PR extends the payments rail + AMPS demo ecosystem by adding an offline AMPS lifecycle harness (with Playwright coverage), introducing a manual_review transfer provider, and refactoring several transfer/reversal/healing paths to be transfer-owned and more idempotent. It also tightens auth-gated Convex chains and updates docs/tests/generated artifacts to reflect the canonical TransferProvider + transferRequests model.
Changes:
- Add an AMPS offline lifecycle “E2E harness” route + Playwright scenario, plus Convex demo seed/execute/confirm/cleanup support.
- Introduce
manual_reviewtransfer provider and propagate it through validators, registries, dispersal hold periods, and tests. - Refactor reversal/healing flows to be transfer-scoped (postingGroupId + effects), and consolidate/strengthen auth-gated query/mutation chains (payments/cash-ledger/docs/CRM).
Reviewed changes
Copilot reviewed 90 out of 92 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/convex/testKit.ts | New shared convex-test bootstrap. |
| src/test/convex/runtime.ts | Shared scheduled-work draining helper. |
| src/test/convex/payments/reliabilityHarness.ts | Use shared drain helper. |
| src/test/convex/payments/mortgageLifecycleChaos.test.ts | Update reversal IDs to transfer-scoped. |
| src/test/convex/payments/helpers.ts | Reuse scheduled-work drain helper. |
| src/test/convex/onboarding/helpers.ts | Reuse scheduled-work drain helper. |
| src/test/auth/permissions.ts | Add payment/cash-ledger permissions. |
| src/test/auth/helpers.ts | Swap to shared Convex test kit. |
| src/test/auth/chains/role-chains.test.ts | Expand auth chain matrix. |
| src/test/admin/admin-shell.test.ts | Update canAccessAdminPath context usage. |
| src/routeTree.gen.ts | Add AMPS e2e-payments route. |
| src/routes/demo/document-engine/route.tsx | Add staff+permission route guard. |
| src/routes/demo/crm/route.tsx | Guard CRM demo route by permission. |
| src/routes/demo/amps/route.tsx | Guard AMPS demo routes for staff admin. |
| src/routes/demo/amps/e2e-payments.tsx | New offline lifecycle harness UI. |
| src/routes/demo/amps/-rules.tsx | Refactor selection/mortgage options hooks. |
| src/routes/demo/amps/-route.tsx | Add nav link for E2E harness. |
| src/routes/demo/amps/-collection-plan.tsx | Refactor selection/label map hooks. |
| src/routes/demo/amps/-collection-attempts.tsx | Refactor selection/label map hooks. |
| src/routes/admin/route.tsx | Pass full auth context to path gate. |
| src/lib/rbac-display-metadata.ts | Add display metadata for new perms. |
| src/lib/auth.ts | Add staff-admin guards + context-based admin path gate. |
| src/components/demo/amps/hooks.ts | Add reusable selection + mortgage helpers. |
| src/components/demo/amps/dialogs.tsx | Introduce shared dialog submit hook. |
| e2e/helpers/auth-storage.ts | Extract reusable Playwright auth storage helper. |
| e2e/auth.setup.ts | Reuse extracted auth storage helper. |
| e2e/amps/offline-payments.spec.ts | New Playwright offline lifecycle spec. |
| e2e/amps/auth.setup.ts | Reuse extracted auth storage helper. |
| docs/technical-design/unified-payment-rails.md | Update docs to TransferProvider canon. |
| docs/reviews/2026-04-08-maintainability-dry-yagni-review.md | Add maintainability review doc. |
| docs/reviews/2026-04-08-fintech-architecture-review.md | Add architecture review doc. |
| docs/architecture/unified-payment-rails-technical-design.md | Update docs to TransferProvider canon. |
| convex/test/moduleMaps.ts | Register new demo/payment modules. |
| convex/test/authTestEndpoints.ts | Add payment/cash-ledger/document endpoints. |
| convex/schema.ts | Remove legacy transfer statuses. |
| convex/payments/webhooks/stripe.ts | Persist + schedule legacy reversal processing. |
| convex/payments/webhooks/rotessa.ts | Persist + schedule legacy reversal processing. |
| convex/payments/webhooks/legacyReversal.ts | New persisted legacy reversal handler. |
| convex/payments/webhooks/tests/reversalIntegration.test.ts | Update to transfer-owned reversal flow. |
| convex/payments/webhooks/tests/legacyReversalWebhook.test.ts | Add persistence/scheduling tests. |
| convex/payments/transfers/validators.ts | Add manual_review + prune statuses. |
| convex/payments/transfers/types.ts | Add manual_review provider code. |
| convex/payments/transfers/providers/registry.ts | Register ManualReview provider. |
| convex/payments/transfers/providers/manualReview.ts | New ManualReviewTransferProvider. |
| convex/payments/transfers/providers/tests/registry.test.ts | Test provider registry resolution. |
| convex/payments/transfers/interface.ts | Remove legacy PaymentMethod mention. |
| convex/payments/transfers/collectionAttemptReconciliation.ts | Transfer-scoped reversal postingGroupId + timestamps. |
| convex/payments/transfers/tests/mutations.test.ts | Add ManualReview provider unit tests. |
| convex/payments/transfers/tests/mutations.integration.test.ts | Update permissions for integration identity. |
| convex/payments/transfers/tests/handlers.integration.test.ts | Add manual_review inbound/outbound behaviors. |
| convex/payments/transfers/tests/collectionAttemptReconciliation.integration.test.ts | Assert transfer-owned reversal postings. |
| convex/payments/collectionPlan/workout.ts | Refactor actor plumbing + reduce duplication. |
| convex/payments/collectionPlan/ruleRecords.ts | New rule upsert helper by code. |
| convex/payments/collectionPlan/readModels.ts | New shared admin/read-model builders. |
| convex/payments/collectionPlan/execution.ts | Make attempt→transfer linkage more atomic. |
| convex/payments/collectionPlan/admin.ts | Move row-building logic to readModels. |
| convex/payments/collectionPlan/tests/workout.test.ts | Disable GT hashchain in tests. |
| convex/payments/collectionPlan/tests/runner.test.ts | Expect transfer record even on failure. |
| convex/payments/collectionPlan/tests/execution.test.ts | Align staging/replay tests to new flow. |
| convex/payments/cashLedger/transferReconciliationCron.ts | Replace placeholder retry with real retrigger scheduling. |
| convex/payments/cashLedger/transferReconciliation.ts | Use transfer-scoped reversal postingGroupId. |
| convex/payments/cashLedger/queries.ts | Remove legacy approved from in-flight set. |
| convex/payments/cashLedger/integrations.ts | Simplify source normalization + transfer-linked reversal lookup. |
| convex/payments/cashLedger/tests/transferReconciliation.test.ts | Assert retrigger creates CASH_RECEIVED. |
| convex/payments/cashLedger/tests/paymentReversalIntegration.test.ts | Update reversal invocation to cascade step. |
| convex/payments/cashLedger/tests/disbursementGate.test.ts | Update in-flight status expectations. |
| convex/fluent.ts | Add payment/cash-ledger/doc chains; tighten CRM data-plane. |
| convex/engine/hashChain.ts | Warn when GT hashchain disabled. |
| convex/engine/effects/transfer.ts | Stamp confirmedAt; transfer-owned reversal cascade for attempt-linked. |
| convex/engine/effects/collectionAttempt.ts | Extract reversal cascade helper; timestamp stamping; skip duplicate reversal. |
| convex/engine/effects/tests/transfer.test.ts | Assert transfer-owned reversal entries for bridged transfers. |
| convex/documentEngine/templateVersions.ts | Gate reads via documentQuery. |
| convex/documentEngine/templates.ts | Gate reads via documentQuery. |
| convex/documentEngine/templateGroups.ts | Gate reads via documentQuery. |
| convex/documentEngine/systemVariables.ts | Gate reads via documentQuery. |
| convex/documentEngine/generation.ts | Gate actions/mutations via document* chains. |
| convex/documentEngine/dataModelEntities.ts | Gate reads via documentQuery. |
| convex/documentEngine/basePdfs.ts | Gate reads/uploads via document* chains. |
| convex/dispersal/holdPeriod.ts | Add manual_review hold period. |
| convex/dispersal/disbursementBridge.ts | Allow manual_review provider code. |
| convex/dispersal/tests/holdPeriod.test.ts | Test manual_review hold/eligibility date. |
| convex/demo/tests/ampsE2e.test.ts | Add offline AMPS lifecycle demo tests. |
| convex/crm/recordQueries.ts | Replace inline link loading with shared helper. |
| convex/crm/tests/records.test.ts | Seed native borrower record for reference test. |
| convex/crm/tests/helpers.ts | Minor modules constant refactor. |
| convex/admin/queries.ts | Switch admin shell query chain to adminQuery. |
| convex/_generated/api.d.ts | Regenerate API typings for new modules. |
| .gitignore | Stop ignoring reviews/ directory. |
Comments suppressed due to low confidence (1)
convex/payments/cashLedger/transferReconciliationCron.ts:388
retriggerTransferConfirmationnow returnsaction: "retriggered", but the cron still branches on"pending_no_effect"and reportspendingNoEffectin logs/results. This makes the healing metrics/logs misleading and leaves dead code. Suggest removing thepending_no_effectbranch/counter (and updating the sharedTransferHealingResulttype if it no longer applies).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+185
to
+193
| function useDialogSubmitAction(options?: { onClosed?: () => void }) { | ||
| const [open, setOpen] = useState(false); | ||
| const [busy, setBusy] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| if (!open) { | ||
| options?.onClosed?.(); | ||
| } | ||
| }, [open, options]); |
There was a problem hiding this comment.
useDialogSubmitAction calls options.onClosed in an effect that depends on the entire options object. Because callers pass an object literal, options changes every render, so onClosed will run repeatedly whenever open is false (including on initial mount), causing unnecessary state resets and potentially render churn. Prefer depending on open + options?.onClosed (or memoizing options / storing the callback in a ref) so the callback only fires on the open→closed transition.
Comment on lines
+25
to
+28
| if (args.orgId) { | ||
| await args.page.goto(`/e2e/switch-org?orgId=${args.orgId}`); | ||
| await args.page.waitForURL("**/", { timeout: 15_000 }); | ||
| } |
There was a problem hiding this comment.
orgId is interpolated directly into the /e2e/switch-org?orgId=... URL. If the org id ever contains characters that need escaping (e.g. /, +, =), the navigation can break or produce the wrong query string. Use encodeURIComponent(args.orgId) when building the URL.
Connorbelez
force-pushed
the
04-04-active_mortgage_payment_system_08-15
branch
from
a44e778 to
981238b
Compare
Connorbelez
force-pushed
the
04-08-major_refac
branch
from
bbc9dc9 to
f32a31d
Compare
Connorbelez
force-pushed
the
04-04-active_mortgage_payment_system_08-15
branch
from
981238b to
c23c793
Compare
Owner
Author
|
@coderabbitai please review |
Contributor
|
🧠 Learnings used✅ Actions performedFull review triggered. |
Connorbelez
force-pushed
the
04-04-active_mortgage_payment_system_08-15
branch
from
8b9899a to
09d58f1
Compare
Connorbelez
force-pushed
the
04-08-major_refac
branch
from
f32a31d to
1e999c7
Compare
Owner
Author
Merge activity
|
Connorbelez
changed the base branch from
04-04-active_mortgage_payment_system_08-15
to
graphite-base/390
- Add seeded offline lifecycle harness and Playwright coverage - Support `manual_review` in dispersal and transfer flows - Update tests and generated API types
- Route attempt-linked reversals through transfer-owned cascades - Replace confirmation retry placeholder with real retriggering - Update balance, reconciliation, and integration tests for new flow
- Consolidate document/admin query and mutation gates - Extract shared collection-plan read models and rule helpers - Rework AMPS demo workspace seeding and webhook handling
- Normalize money and idempotency-key test fixtures - Remove workflow harness dependencies from integration tests - Update offline deal closing to use transition events
Connorbelez
force-pushed
the
04-08-major_refac
branch
from
1e999c7 to
98e5252
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add offline AMPS e2e scenario and manual review transfers
manual_reviewin dispersal and transfer flowsRefactor transfer reversal and healing flows
Refactor auth-gated convex flows and demo seeding