Make refunds safe to retry, recover, and review - #2065
Merged
Conversation
Contributor
|
Important Review skippedToo many files! This PR contains 666 files, which is 366 over the limit of 300. To get a review, reduce the PR to 300 files or fewer by splitting it into smaller PRs or changing its base branch. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (666)
You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR replaces boolean refund checks with structured payment evidence. It adds refund admission, durable claims, indexed payment references, protected payment state, partial ledger reconciliation, and safeguards for attendee deletion, merging, pruning, and settlement. ChangesPayment refund architecture
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant RefundBatch
participant RowClaims
participant PaymentProvider
participant PaymentReferences
participant RefundLedger
RefundBatch->>RowClaims: claim attendee payment rows
RowClaims-->>RefundBatch: claimed references and sessions
RefundBatch->>PaymentProvider: read charge money
PaymentProvider-->>RefundBatch: refund evidence
RefundBatch->>PaymentProvider: send admitted refunds
RefundBatch->>PaymentReferences: mark returned references
RefundBatch->>RefundLedger: record returned groups
RefundBatch->>RowClaims: release or retain claims
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3561bbf610
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Member
Author
|
@coderabbitai review |
Contributor
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/shared/payment/diagnose.ts`:
- Around line 126-127: Update providerCouldNotRefund to accept RefundResolution,
adding that type to the existing type-only import from
`#shared/payment/resources.ts`, and explicitly annotate the predicate’s boolean
return type. Preserve the existing failed-status and provider_failed-reason
checks while relying on the union’s reason vocabulary for compile-time
validation.
In `@src/shared/payment/resources.ts`:
- Around line 177-186: Update the documentation above refundMoneyThatIs to
briefly describe that it sums refund amounts matching the requested status,
including completed and pending refunds; remove the separate pending-only and
aggregate-calculation descriptions while leaving the helper implementation
unchanged.
In `@src/shared/payment/words.ts`:
- Around line 3-10: Replace milestone and historical narration with concise
current-behavior comments: in src/shared/payment/words.ts lines 3-10, document
only the payment vocabulary used by validation; in
src/shared/payment/resources.ts lines 92-96, state that completed and pending
refunds require positive amounts while failed refunds may be zero; in
src/shared/payment/conflict.ts lines 1-9, describe only the current conflict
union without M5/M7 or reference-kind history.
In `@test/shared/payment/diagnose.test.ts`:
- Around line 162-170: Expand the payment diagnosis tests around the
duplicate-charge case to use distinct charge resource IDs when asserting the
multiple-charges conflict, then add cases covering refund_exceeds_capture,
partial_charge, capture_total_mismatch, and per-charge currency_mismatch in
diagnose.ts. Ensure each fixture reaches its intended conflict path without
being intercepted by duplicate IDs or another earlier validation.
In `@test/shared/payment/refund.test.ts`:
- Around line 68-82: Update the refund expectations in the tests at the
referenced cases, including the pending-refund test and the corresponding
assertions around lines 78, 93, and 113, to use toStrictEqual instead of
toEqual. Preserve the expected objects without a refund key so the assertions
verify that named omits the property rather than returning it as undefined.
In `@test/shared/payment/resources.test.ts`:
- Around line 258-269: Update the comparison cases in the sameProviderResource
test to use valid ProviderResource variants, including the required parentId for
the stripe_payment_intent kind, and remove the `as typeof refundResource` cast
when passing changed. Preserve coverage for provider, kind, and id differences.
In `@test/test-utils/payment-state.ts`:
- Around line 109-115: Remove the unused refundedObservation and
partlyRefundedObservation helper definitions from the payment-state test
utilities, leaving all referenced payment observation helpers unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 083b4e12-9235-4d60-a36b-9a1bc9c73aef
📒 Files selected for processing (16)
src/shared/payment/conflict.tssrc/shared/payment/diagnose.tssrc/shared/payment/money.tssrc/shared/payment/observation.tssrc/shared/payment/refund.tssrc/shared/payment/resource-id.tssrc/shared/payment/resources.tssrc/shared/payment/words.tssrc/shared/validation/kind.tstest/shared/payment/conflict.test.tstest/shared/payment/diagnose.test.tstest/shared/payment/observation.test.tstest/shared/payment/refund.test.tstest/shared/payment/resources.test.tstest/shared/validation/kind.test.tstest/test-utils/payment-state.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d13677b699
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86b3612155
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3cb76b3f40
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
PR4_PLAN.md (1)
1192-1196: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDefine an explicit anchor-row boundary before using
processed_paymentsfor claims.Lines 1192-1196 store the claim anchor in
processed_paymentswith an attendee ID and encrypted payment reference. The schema documents admin roster, export, and refund-all reads that select rows byattendee_idandpayment_reference != ''(src/shared/db/migrations/schema/tables-attendees.ts, Lines 210-218). Those reads can treat the anchor as a real payment. Add a record discriminator and exclude anchors from every payment read, or use a dedicated claims table. Add regression tests before implementing theINSERT-OR-IGNOREflow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@PR4_PLAN.md` around lines 1192 - 1196, Define an explicit discriminator for claim anchor rows before implementing the INSERT-OR-IGNORE flow described in the plan. Update processed_payments payment reads—including admin roster, export, and refund-all queries—to exclude anchor records, or use a dedicated claims table instead; add regression tests covering the boundary before changing the claim insertion behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@PR4_PLAN.md`:
- Around line 122-124: Update the “Not yet built” section in PR4_PLAN.md to
remove the completed admission pre-flight and administrative/API refund wiring,
including src/shared/payment/admit-refund.ts, and add them to the “As built”
inventory. Keep only the remaining claim, discovery and tagging, sweep,
recorder, and owner-action work listed as unfinished.
In `@src/shared/db/migrations/schema/tables-attendees.ts`:
- Around line 199-208: Update every processed_payments write
path—buildFinalizeStatements, finalizeSessionIfUnresolved, and
legacyMergePaymentStatement—to derive and persist payment_reference_index
whenever payment_reference is written, and clear both columns together when a
stale reservation is reused or the reference is removed. Ensure the updates are
atomic and add coverage for finalization, recovery, merge, and stale-reservation
reuse.
In `@src/shared/payment/admit-refund.ts`:
- Around line 101-103: Make refund admission and sending atomic in
src/shared/payment/admit-refund.ts lines 101-103 by acquiring a durable
per-payment-reference claim before provider state is read or refundPayment is
called; non-owners must withhold or reconcile without sending. Add a
deterministic two-caller concurrency regression test in
test/shared/payment/admit-refund.test.ts lines 56-93 asserting only one provider
refund call occurs for the same payment reference.
In `@src/shared/payments.ts`:
- Around line 288-295: Update all payment provider test doubles implementing
PaymentProvider to replace isPaymentRefunded stubs with readChargeMoneyOrNull
implementations. Return a ChargeMoney | null value matching each test scenario,
and remove the obsolete method so the doubles satisfy the current contract.
---
Outside diff comments:
In `@PR4_PLAN.md`:
- Around line 1192-1196: Define an explicit discriminator for claim anchor rows
before implementing the INSERT-OR-IGNORE flow described in the plan. Update
processed_payments payment reads—including admin roster, export, and refund-all
queries—to exclude anchor records, or use a dedicated claims table instead; add
regression tests covering the boundary before changing the claim insertion
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: af444e5c-adab-438f-8955-f2510865a318
📒 Files selected for processing (29)
AGENTS.mdPR4_PLAN.mdsrc/features/admin/attendees-edit.tssrc/features/admin/refunds/provider.tssrc/features/api/payment-processing/refunds.tssrc/shared/db/migrations/2026-08-10_payment_state_columns.tssrc/shared/db/migrations/registry.tssrc/shared/db/migrations/schema/tables-attendees.tssrc/shared/db/migrations/schema/version.tssrc/shared/payment/admit-refund.tssrc/shared/payment/conflict.tssrc/shared/payment/diagnose.tssrc/shared/payment/money.tssrc/shared/payment/refund.tssrc/shared/payment/resources.tssrc/shared/payment/words.tssrc/shared/payments.tssrc/shared/square-provider.tssrc/shared/stripe-provider.tssrc/shared/stripe/schemas.tssrc/shared/sumup-provider.tssrc/shared/sumup.tstest/shared/db/migrations/schema/index.test.tstest/shared/db/migrations/schema/version/guard.test.tstest/shared/payment/admit-refund.test.tstest/shared/payment/diagnose.test.tstest/shared/payment/refund.test.tstest/shared/payment/resources.test.tstest/test-utils/payment-state.ts
💤 Files with no reviewable changes (2)
- src/shared/payment/words.ts
- test/test-utils/payment-state.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 463859c1ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 232c450f60
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffdd053711
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@PR4_PLAN.md`:
- Line 120: Update the “three state columns” entry in PR4_PLAN.md to state that
payment-claim.ts writes payment_reference_index during finalize, while noting
that only pre-M4 legacy rows may remain empty. Remove the contradictory “NOTHING
WRITES IT YET” wording and keep the migration/schema references unchanged.
In `@src/features/admin/attendee-refunds.ts`:
- Around line 135-141: Remove the duplicate three-line comment in the attendee
refund flow, keeping a single copy directly above the underAttendeeClaim call it
explains; do not retain it above const candidate.
In `@src/features/admin/refunds/provider.ts`:
- Around line 143-150: Update the release handling around the refund workflow’s
try/catch so failures from release do not replace the original refund error or
discard a successful result. Preserve and rethrow the caught error after
attempting release(true), and preserve result after attempting
release(run.lost(result)); report any release failure separately through the
existing logError mechanism, threading listingId via underAttendeeClaim or the
run object as appropriate.
In `@src/shared/db/payment-claim.ts`:
- Around line 84-88: Update the SELECT statements in readRows and the other
processed_payments query to alias the table as processed payment using AS, and
qualify table references as needed while preserving callers’ bare where column
names. Leave the UPDATE statements unchanged because SQLite does not support
aliases there.
- Around line 99-119: Update readClaimableRows so rows with a non-empty
payment_reference but an empty payment_reference_index cannot proceed as
claimable without shared-reference validation. Confirm the payment_state
migration backfills payment_reference_index; if it does not, make
readClaimableRows block or fail such claims instead of returning sharing: []
while preserving normal indexed-reference behavior.
In `@src/shared/db/prune.ts`:
- Around line 57-61: Update the prune flow around the protected_state predicate
and mayReleaseClaim/decideClaim behavior so lost claims without keyed capability
do not remain permanently unprunable. Add the claim timestamp to the plaintext
mirror and apply the existing staleBefore cutoff when deciding whether to skip a
claim, or ensure a sweep settles such claims before pruning; preserve skipping
genuinely live claims while allowing stale ones to be pruned.
In `@test/shared/db/payment-claim.test.ts`:
- Around line 161-163: Add assertions to the empty-release test around
releaseAttendeeRows to verify the durable implementation makes zero database
calls, using countDatabaseCalls and an exact count assertion. Add a separate
test for claimAttendeeRows([]) that likewise verifies no database calls occur,
covering its empty-session early return while keeping each test focused on one
behavior.
- Around line 113-117: Update the “is written by the same statement as the
reference” test to import and compare against the exact deterministic value
returned by paymentReferenceIndex("pi_h"), replacing the non-empty regex
assertion while preserving the existing stored.v lookup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fcc613ff-df4b-499d-afb6-ead6e723a7e7
📒 Files selected for processing (44)
PR4_PLAN.mdTODO.mdsrc/features/admin/attendee-refunds.tssrc/features/admin/refunds/provider.tssrc/shared/db/payment-claim.tssrc/shared/db/payment-finalize.tssrc/shared/db/payment-references.tssrc/shared/db/processed-payments.tssrc/shared/db/prune.tssrc/shared/payment/claim.tssrc/shared/payment/row-state.tssrc/shared/payments.tssrc/shared/square-provider.tssrc/shared/square.tssrc/shared/stripe-provider.tssrc/shared/sumup-provider.tstest/features/admin/attendee-refunds/single.test.tstest/features/admin/refunds/provider.test.tstest/features/admin/refunds/provider/batch.test.tstest/features/api/payment-processing/index/refunds.test.tstest/features/api/payment-processing/refunds.test.tstest/features/api/payment-processing/refunds/rejected-charge.test.tstest/features/api/webhooks/helpers.tstest/integration/server/payments-success-refunds.test.tstest/integration/server/payments/replay.test.tstest/integration/server/webhooks/price-signature-package-overrides.test.tstest/integration/stripe/core.test.tstest/integration/webhook-price-signature/helpers.tstest/shared/db/payment-claim.test.tstest/shared/db/processed-payments.test.tstest/shared/db/prune/helpers.tstest/shared/db/prune/payments.test.tstest/shared/payment/claim.test.tstest/shared/payment/row-state.test.tstest/shared/square-provider.test.tstest/shared/square/retrieve-refund.test.tstest/shared/stripe-provider/operations.test.tstest/shared/stripe/request.test.tstest/shared/sumup-provider.test.tstest/shared/sumup/transactions.test.tstest/test-utils/attendees/helpers.tstest/test-utils/payment-state.tstest/test-utils/refund-routes.tstest/test-utils/stripe/responses.ts
There was a problem hiding this comment.
💡 Codex Review
Keep the keyless claim when marker persistence fails
When a SumUp refund succeeds but markPaymentReferencesProviderRefunded throws, this branch preserves the refunded outcome, so underAttendeeClaim classifies the result as validated and releases the keyless claim. Fresh evidence beyond the earlier lost-provider-response thread is that the single-admin route records its ledger entry only after that release; if the process dies or the ledger write also fails while SumUp's events still lag, an immediate retry can issue a second refund. Treat the failed completion marker as an errored/lost result, or retain the claim until another durable completion record lands.
AGENTS.md reference: AGENTS.md:L73-L79
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d06920535f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a3a5d2ffa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/features/admin/refunds/provider.test.ts`:
- Around line 415-446: Extend the existing provider factory to accept a refund
capability option, defaulting to its current keyed behavior, and use that
factory for the keyless provider doubles in the refund tests. Replace the inline
keyless, keylessSaysNo, and alreadyBack objects with factory calls using the
appropriate refunded or alreadyRefunded values and keyless capability,
preserving their current refund outcomes.
- Around line 485-505: Add a test alongside the existing keyless hold tests that
uses a row claim whose release operation rejects. Invoke underAttendeeClaim with
a successful work result, then assert the returned result is unchanged and the
release failure is reported through the established error-reporting mechanism.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5807909d-48ae-4971-99f4-5c6cea26d0ce
📒 Files selected for processing (11)
PR4_PLAN.mdTODO.mdscripts/mutation/equivalent-mutants/shared-a-l.txtsrc/features/admin/attendee-refunds.tssrc/features/admin/refunds/provider.tssrc/shared/db/payment-claim.tssrc/shared/db/prune.tstest/features/admin/refunds/provider.test.tstest/shared/db/payment-claim.test.tstest/shared/db/prune/helpers.tstest/shared/db/prune/payments.test.ts
💤 Files with no reviewable changes (1)
- scripts/mutation/equivalent-mutants/shared-a-l.txt
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 468bc45b88
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5b36068e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82889d2f34
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/features/admin/refunds/provider.test.ts`:
- Around line 663-678: Update the assertions after processRefundBatch to verify
counts.refundedCount and counts.errorCount separately, requiring two refunds and
zero errors. Ensure the test setup for attendees 11 and 12 provides
deterministic database rows or a recording double so the exact expectations
remain valid.
In `@test/shared/db/payment-claim.test.ts`:
- Around line 260-284: Add a test alongside the existing returned-money cases
that books two attendees with the same payment reference, marks the second
attendee’s unclaimed row as refunded, then claims the first attendee via
claimAttendeeRows. Assert held.returned contains the paymentReferenceIndex for
that shared reference, confirming stored.sharing contributes to the result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 576e19c3-38af-4268-877e-7707b404e317
📒 Files selected for processing (9)
TODO.mdsrc/features/admin/attendee-refunds.tssrc/features/admin/refunds/provider.tssrc/shared/db/payment-claim.tssrc/shared/db/payment-references.tssrc/shared/payment/admit-refund.tstest/features/admin/refunds/provider.test.tstest/shared/db/payment-claim.test.tstest/test-utils/refund-routes.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 743c456774
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Member
Author
|
Round 26 — four findings, all verified against the code first, all real, all fixed. Codex P1, Fixed in 562366d: a row with work on it is never pruned, however long ago the claim was taken. Nothing is stranded — a stale claim is resumable, so the next run for that attendee picks the row up and settles it. The contract already required this (the prune "never deletes a row with live work on it"; merge and delete fail closed "fresh or stale"), so the code had drifted from it. The mirror no longer carries a written-at time, which existed only so the prune could expire a claim. Two regressions pin it, one through CodeRabbit, CodeRabbit, CodeRabbit,
Also closed from CI is Generated by Claude Code |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
PR4_PLAN.md (1)
138-143: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the claim-coverage status.
Line 138 states that only the admin bulk route claims. The single-attendee route also calls
underAttendeeClaiminsrc/features/admin/attendee-refunds.tsLines 140-165. State that both admin routes claim, while the callback and refresh routes remain unclaimed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@PR4_PLAN.md` around lines 138 - 143, Update the claim-coverage statement in PR4_PLAN.md to say both admin routes claim via underAttendeeClaim, including the single-attendee path in src/features/admin/attendee-refunds.ts, while the callback flow (tryRefund/refundReferenceAtProvider) and refresh-payment route remain unclaimed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/shared/db/processed-payments.ts`:
- Around line 183-193: Add payment_reference_index to the processed_payments
fixture table definition in the healing test setup so it matches the production
schema and allows the update path using refClause and refParams to execute
successfully.
---
Outside diff comments:
In `@PR4_PLAN.md`:
- Around line 138-143: Update the claim-coverage statement in PR4_PLAN.md to say
both admin routes claim via underAttendeeClaim, including the single-attendee
path in src/features/admin/attendee-refunds.ts, while the callback flow
(tryRefund/refundReferenceAtProvider) and refresh-payment route remain
unclaimed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6d7fa51b-3aa4-4e7b-9a62-d2f3ea5b0feb
📒 Files selected for processing (30)
PR4_PLAN.mdTODO.mdsrc/features/admin/attendee-refunds.tssrc/features/admin/attendees-edit.tssrc/features/admin/refunds/provider.tssrc/features/api/payment-processing/refunds.tssrc/shared/db/payment-anchor/mint.tssrc/shared/db/payment-anchor/session.tssrc/shared/db/payment-claim.tssrc/shared/db/payment-references.tssrc/shared/db/processed-payments.tssrc/shared/db/prune.tssrc/shared/payment-review.tstest/features/admin/attendees-edit.test.tstest/features/admin/refunds/provider.test.tstest/features/admin/refunds/provider/batch.test.tstest/features/api/payment-processing/refunds.test.tstest/shared/db/payment-anchor/mint.test.tstest/shared/db/payment-claim.test.tstest/shared/db/payment-references.test.tstest/shared/db/processed-payments.test.tstest/shared/db/processed-payments/staleness.test.tstest/shared/db/prune/helpers.tstest/shared/db/prune/payments.test.tstest/shared/merge/attendee-merge/repoint.test.tstest/shared/payment-review.test.tstest/shared/refund-ledger/helpers.tstest/test-utils/payment-claim.tstest/test-utils/processed-payments.tstest/test-utils/refund-routes.ts
💤 Files with no reviewable changes (1)
- TODO.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fbceefb98
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Member
Author
|
Round 27 — three findings. CodeRabbit, Rather than add the one column back, it now rebuilds from the schema the app itself uses ( CodeRabbit, Codex P1, remove the test-only It is not left unguarded: the PR description names it, the contract names it as a live fault with the sweeps that will call it, and the check stays red until those land — so the judge cannot quietly become a second authority, because nothing can merge while it has no caller. CI on 9fbceef: Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a0d5286cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Member
Author
|
Codex P1 on The mechanism is exactly as described. A merged attendee's references are read and sent concurrently inside Two corrections to the framing, neither of which rescues it:
Where the suggested fix needs adjusting. "Read and admit the complete reference set first" is the right instinct, but the reason is stronger than stated, and it rules out the alternative I reached for first. I checked whether the ledger could simply record the part that moved: it cannot. That leaves two coherent fixes, and they differ in what a buyer actually experiences:
I am not choosing between those unattended. It changes when real refunds reach real buyers, and this repo's workflow puts a behaviour contract in front of a human before implementation for exactly this kind of change. It is now recorded in Thanks — this one was worth the round. Generated by Claude Code |
- payOnHostedPage was a function(this: LiveWorld) stored in an array and called as pay(this) — losing the this binding in Cucumber's step runner. Make it an arrow (world: LiveWorld) => like the other pay handlers. - testStripeConnection used button.click() which stalls on CI's frame-idle renderer. The #stripe-test-btn is type=button (not a submit), so actOnControl's requestSubmit fallback doesn't apply. Use a scripted DOM click via evaluate to fire the button's JS handler without needing Playwright's stability wait.
Upgrading a released site to this branch could not finish. applySchemaChanges reconciles the whole declared schema, and for a table that already exists it can only ADD COLUMN. This release redefines payment_charges — the released table has no `id`, and SQLite refuses to add a PRIMARY KEY column — so the first migration that swept the schema stopped the upgrade, three entries before the one that would have rebuilt the table. None of the five migrations this branch added had run anywhere, so rather than reorder around the hazard they collapse into one migration that cannot get the order wrong. It drops the six dormant payment-record tables first, which are empty on every site and which nothing has ever written, so the apply then creates each one with a single CREATE carrying every column and never reaches for an ALTER. A guard refuses to drop a table holding rows. No declared trigger reads those tables, so dropping them cannot drop one and the migration does not sync triggers. The new migration test starts from a released-shape payment_charges and proves the upgrade completes and the table ends up rebuilt. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AqLNrpvmNdGSSqUtSafiMd
…er, SumUp currency - holdFirstAppReturn.capturedUrl now polls: the route handler can fire a microtask after awaitReturnToApp sees the intercepted URL, so an instant check raced the capture and threw. - bookComplexOrder waits for the browser to actually leave for the hosted checkout before the provider driver settles it. The booking POST lands on an app reserved page first; reading the Stripe session id from the URL or the Square orderId from the log before the redirect had committed failed. - classifySubmittedRefund now classifies by the LOCAL page state — the app honestly reports what was recorded at submission time. A provider that completes a refund moments after the app accepted it as observing does not contradict the app; it only makes the observation Refresh-eligible. The provider's returned amount is still checked exactly, separately. - SumUp refund currency falls back to the transaction's own currency: SumUp does not reliably repeat it on every REFUND event, and production reads it the same way (src/shared/sumup/transaction.ts).
- The complex-order Scenario Outline never configured the payment provider, so the whole multi-listing cart priced as a provider-less free reservation and completed at /ticket/reserved instead of reaching any checkout. The catalog Given now configures the target provider first on paid legs. - The held return route was page-scoped; a Stripe cross-origin redirect can slip past it as a main-frame navigation. Register the hold on the visitor's whole browser context (first /payment/success wins, later ones pass through) and unregister it once captured so the replay reaches the app. - The complex-order booking step now carries the hosted-payment step allowance; over a fresh tunnel the settle alone can outlast the 120s default and killed the Square leg. - SumUp's transaction history reports money in major units; the shared observation compares minor units. Convert per event (the harness only runs 2-decimal currencies, so a fixed hundredfold is exact).
- classifySubmittedRefund keyed the observing branch on the one-shot flash warning, which can be gone by gathering time; SumUp's settling refund then fell into the recorded branch and failed on its claims. The Refunded status is now the sole positive proof of recorded; everything else is the observing state, whose durable safety claims (no Refunded, no send, no delete, Refresh reachable) must hold — the flash warning stays a claim of the fault scenario only, where the landing page is read directly. - The held return's Playwright route interception never fired for Stripe's cross-origin redirect in CI (both page- and context-scoped). Capture the return URL from the visitor's URL bar instead: the moment it lands on /payment/success, that URL is the checkout's own return binding. Holding is unnecessary for correctness — the processed-payment reservation makes the replay idempotent whichever of webhook or return books first.
- 'When the visitor retries the exact return' (the invalidated-checkout scenario) had no step definition — the scenario never reached it until this run. All three replay wordings now share one table-driven When. - A provider-side-completed refund whose retirement has not caught up safely blocks Delete while the Refunded badge is already up; SumUp hit exactly that and the recorded claims failed on Delete availability. Delete's return is a retirement property, not a recording property — the recorded branch now proves Refunded + no send + no warning, and only the Stripe recovery scenario (where retirement completed) asserts Delete reachability. The observing branch still demands Delete be absent.
The 'the visitor retries the exact return' step had no definition and nothing caught it before a live dispatch — the scenario reached it only after the earlier failures were fixed, then died as undefined. The new direct test loads the same support code the runner loads (loadSupport) and matches every step text the Feature carries against the registered Cucumber expressions, so a missing or renamed step fails in CI, not in the sandbox. The live-harness files that only the nightly workflow can execute (browser sessions, servers, tunnels, provider drivers, step bodies) join the coverage exclusions — the step-coverage test imports them, so a coverage run sees them loaded but untested. Pure helpers with deterministic behaviour stay under coverage and are directly tested under test/e2e-payments/.
Two review findings on the live harness: - SumUp's refund events were counted by event_type alone, so a PENDING or FAILED refund's money read as returned. Count only REFUNDED/SUCCESSFUL events — the same mapping production applies (src/shared/sumup/money.ts) — and throw on FAILED: a rejected refund is a failure, never a pending or completed observation. - observeViaRead converted any read failure (outage, rejected credentials, malformed body) into a pending observation, so a nightly run could pass green without ever learning what the provider returned. Read errors now propagate; pending is reserved for a successful read that honestly reports no settled refund yet.
The harness passes its whole environment to each scenario's app server, including NTFY_URL — so the scenarios' deliberate server errors (the Money fault, the SumUp refusal probes, the price-change refund) pinged ntfy as if they were production incidents, making green runs look like failures. The harness's own failure-only notification is now the only one a run sends.
A partial provider return (some money back, some not) parked the refund as
needs_provider_check whose only offered exit was 'check again' — and that
exit was provably a no-op: answerProviderConflict writes nothing when the
provider money is unchanged, so nothing but returning the rest of the money
at the provider could ever clear the state, permanently blocking the
privacy delete.
A partial return is a settled provider fact, not a pending one, so it now
parks as needs_owner_choice with reason provider_conflict: the owner
confirms the money that came back (the only admitted choice — 'not sent'
would re-arm a send that pays the returned part twice) and the case
completes with the exact partial money, unblocking the delete once
recorded. Only genuinely inconclusive ('wait') evidence remains a provider
check. Partial evidence also stays protected: later provider reads cannot
overwrite the stored decision, and the not-sent choice is refused.
The two deterministic refund-safety scenarios that pinned the old
check-again exit follow the fix: the partial-return case now asks for the
owner's decision, offers only the returned choice, and still contacts no
provider. The attempt outcome for a wait-conflict provider check gains a
direct test, and the authority-architecture heuristic now counts 'wait'
among the authority-result kinds, bringing genuine authority tests into
the reviewed boundary list.
Verified against the reviewing agent's independent finding at head 2140651:
all three legs (dead-end routing, no-op early return, UI offering only the
no-op) stood as described.
The refund case page showed only the captured amount, so an owner closing a partial return (£4 back of a £25 charge) saw £25 as the only figure, confirmed 'the provider returned this money', then attested the Money entry — and posted £25 where the authority had stored £4. Bookkeeping went wrong by the difference and nothing flagged it afterwards, because recording the case is exactly what that attestation finishes. The case now carries the row's returned money alongside the capture, and the page shows 'Provider returned' and 'Not returned' before both money surfaces: the returned choice and the Money-recorded confirmation. The two numbers are the same money the resolution applies — the conflict decision for a settled return, the stored refunded amount for the Money attestation — pulled from the same payment_charges columns, so the page cannot show a number the decision disagrees with. Regression coverage pins the wiring at both layers: the template test asserts a partial-return page contains the returned and missing amounts (and that nothing-extra is claimed when £0 came back), and the recovery integration test asserts the served page carries £4 returned / £21 missing before the choice is submitted.
Stripe requests already aborted on a 20s timer, but the SumUp and Square transports passed no signal to their fetches, so a provider that accepted the connection and then stopped answering held the call open with no answer. That is sharpest on a refund send, which arms the authority before the POST: a stuck call leaves the case armed and unknown, holding the request and its isolate until the platform kills it, and the eventual error is not one the failure classifiers recognize. The timeout now lives in one shared constant all three providers use, so the budget cannot drift between them. A timeout abort reaches the existing classifiers as a connection timeout: a refund send becomes 'uncertain' — the honest answer, since the POST may have landed — and the authority observes later, which is the recovery path this branch built. Both transports gain a regression test that hangs the provider fetch and fast-forwards the clock: SumUp must answer 'uncertain, timeout' and Square 'unavailable, timeout' exactly when the shared budget expires.
Threads fixed in 97efb72 / 7bda1f6 / ed3a823 / f86d152 get resolution replies on the PR. Everything verified-real-but-unfixed is recorded here so the threads can be answered without losing the work: two upgrade-blocking P1s (migration apply ordering, Square missing-order webhook), two parked-outcome P1/P2s (refunded rejection with no Money target, un-retried final placeholder outcome), and the live-harness hardening batch.
The refund system's safety lives in a declared state machine, and the only way to see it has been to read the code. This adds an owner-only System map page that draws every state a money record can sit in and every way it can move — 'if the payment is in state X, its next options are Y and Z', for the whole machine. The map is derived, never drawn by hand. Each machine module builds its states with the real production constructors and discovers its edges by running the real transition functions — a transition that throws is not an option, so the page updates itself whenever the rules change and cannot drift from them. Two machines ship: the refund authority (nine nodes, including the three distinct owner decisions and the one-exit partial return this PR made safe) and the payment review lifecycle (reasons and the exact evidence that retires each). A new machine joins by deriving one module and adding it to the list; the page needs nothing else. The page is complete without JavaScript: every state, its meaning, what the lifecycle says clears it, and its ways forward are rendered as lists. With JavaScript, the same data (labels resolved server-side, embedded as JSON) becomes an interactive SVG diagram — click a state and its own arrows light up, colour-coded by who moves the record: the site, the provider, or you. Wiring follows the house patterns: the settingsArticlePage shell (also adopted by the debug page) and the shared JsonScript component (also adopted by duplicate-preview), both extracted where cpd found the same shape written twice. Tests pin the derived maps exactly — edge sets, actors, key strings, layouts — and each module holds a 100% mutation kill rate, with the genuinely unkillable mutants (refusal text, a defined-vs-undefined stamp) recorded as proven equivalents. The concurrent-rows fixture's poll branch gets the direct test its timing-dependent coverage was flaking on.
Main now holds comments to twenty lines, and this docblock ran twenty-two. Same facts, two paragraphs merged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AqLNrpvmNdGSSqUtSafiMd
stefan-burke
force-pushed
the
claude/m4-pr-a
branch
from
August 17, 2026 10:34
0e91d2b to
0c4b128
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The rebase merged TODO.md by union, and each historical rename of the Refund All entry's heading came back as its own bodiless line above the real one. The one heading old m4 actually carried stays; a sweep against both pre-rebase sources confirms no other heading was added or lost. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AqLNrpvmNdGSSqUtSafiMd
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This was referenced Aug 17, 2026
stefan-burke
pushed a commit
that referenced
this pull request
Aug 19, 2026
TODO.md holds work that is still open, so a finished entry leaves it instead of staying in place marked "done". AGENTS.md now says so, and this change applies the rule to every entry the code already answers: - the payment-provider split out of `src/shared/db/settings.ts`, done in the commit before this one; - `src/features/api/webhooks.ts` below 400 lines, done by #2065; - a completed Square webhook whose order reads as missing, retried rather than acknowledged since #2106; - two suites covering the attendees list; - four feature modules that now have a test at their mirrored path. One entry was half open. The shared "reasons" shape shipped, but the surfaces it makes cheap did not, so the entry keeps the four open surfaces and the three decisions that bound them, and loses the record of what shipped. The stripe-mock port-steal entry stays: AGENTS.md cites it as the worked example for reading a written-down diagnosis, so it is documentation, not a job. The branch mutation gate found four survivors in the moved provider code. Two were real: - `changePaymentProvider` defaulted `first` to `false`. No caller reads that default, because only the credentials kind reads `first` at all, and that caller always passes the flag. The parameter is now optional with no default. - `settings.square.hasToken` moved into the new module, but its only test stayed in `public-api.test.ts`, which is not a direct test of that file. The mirror suite now covers the getter on both answers. The other two are one expression, `provider ?? ""`, and both are recorded in `scripts/mutation/equivalent-mutants/shared-db.txt` with their proof. No provider id is the empty string, so `??` and `||` agree. Only the disable kind takes the fallback, and the disable statement never evaluates a placeholder bound to that value. `deno task precommit:mutation` — 197 mutants, 195 killed, 2 suppressed, 100%. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EW4n7LWYByACzixWob1X21
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
2 participants
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.
Why
Refunds used to depend on partial signals: the currently configured provider,
nullable reads, boolean send results, attendee notes, and route-local retry
rules. That left gaps where a lost provider reply or failed local write could
send money twice, hide returned money, or create work with no safe exit.
PR4-A replaces those seams with one durable refund authority and one declared
payment-work lifecycle. The system sends only from exact stored facts, records
what it learns before releasing blockers, and fails closed when it cannot prove
what happened.
What changed
One refund engine
contracts. Missing, unavailable, invalid, completed, accepted, rejected,
unsent, and uncertain outcomes cannot be collapsed into booleans or nulls.
the same provider identity, authority, Money recording, and settlement path.
the charge and is never guessed from current settings.
same generation; uncertain keyless requests become owner work.
retain the claim or persist returned-but-unrecorded money before propagating.
Forward-only payment provenance
New attendee rows carry one non-sensitive provenance field:
NULL: historical or otherwise unqualified payment history;"": the stored PII was proven to contain no payment id;processed_paymentsrow that proves theencrypted PII payment id.
New paid bookings write this proof in the booking transaction. Placeholder
creation atomically writes the attendee, payment anchor, provenance, ready
refund authority, and terminal checkout outcome. Merge propagates uncertainty;
pruning retains the row named by proven provenance.
Historical/unqualified payments are intentionally not refundable in the app. The
operator refunds those directly in Stripe, Square, or SumUp. There is no
population decrypt, backfill, re-save requirement, compatibility reader, guessed
provider, or legacy refund engine.
Blocking states have reachable exits
clears it, the owner route that reaches that clearer, and whether a real money
choice is required.
PaymentMoveSnapshotdrives attendee actions, Delete confirmation, mergepreview, and the transactional delete/merge writers. A link is never offered
when the writer must refuse it; a race still fails inside the transaction.
legacy PII payment id.
refresh existing evidence.
Bounded work and privacy
candidate with
LIMIT 1and decrypts at most that one attendee.NULLprovenance, mismatched provenance, paid bookingswithout a canonical row, and reference drift before any provider call.
claims refuse above 100 holders before protected state is opened or written.
Equality uses DB-keyed one-way indexes. Possessing the database and
DB_ENCRYPTION_KEYalone cannot open those values when the modern wrappedowner key is in use.
DB-key-encrypted; that boundary is explicit rather than presented as PII
secrecy.
No parallel compatibility path
The durable provider-refund authority is the only live refund engine.
Architecture tests reject ambient provider selection, credential enumeration,
direct adapter calls, extra send-permit minters, and extra authority writers in
refund-facing code.
The remaining pre-cutover whole-checkout selector is confined to callback
session retrieval and authentication. It cannot authorize or send a refund. The
later aggregate cutover must replace and delete it in one release rather than
add a bridge or fallback.
What the tests show
checks, delayed/partial/uncertain outcomes, failed local recording, moved
work, blocked Delete/Merge controls, required owner choices, and historical
PII-only deposits mixed with new indexed balance payments.
post-terminal recovery, claim and merge races, pruning, bounded Refund All
admission, privacy-safe reads, migration/restore parity, and every lifecycle
exit.
code. Targeted mutation detected all non-equivalent changes in the new Refund
All and lifecycle decision modules (32/32 and 61 kills plus one recorded
sign-equivalent comparator mutation).
Scope and durable record
This is M4 / PR4 Part A. Stable per-order booking obligations, exact allocation
for shared or merged charges, free package-member cancellation, and the atomic
aggregate cutover remain named future work in
PLAN.mdandTODO.md.The temporary
PR4_PLAN.mdhas been removed. Its useful as-built decisions,deliberate limits, privacy boundary, and future constraints now live in the
permanent plans, code, tests, and reports.
branch.htmlis the database-first five-minute briefing.src-diff-distilled.htmlremoves imports, comments, blanks, moves, renames,and file-split noise from the source comparison.
Verification
env CI=1 nix develop -c deno task precommitmigration regressions
performance, and encryption boundaries