Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
6b1a52f
Accept only confirmed Square refund outcomes
stefan-burke Jul 24, 2026
ec78336
Throw on malformed Square refund responses (PR #1911 review)
stefan-burke Jul 24, 2026
2588b35
Log malformed Square refund responses directly (PR #1911 review)
stefan-burke Jul 24, 2026
62689e1
Accept APPROVED Square refunds (PR #1911 review)
stefan-burke Jul 24, 2026
2858c6e
Refresh equivalent-mutant line numbers and TODO after #1909/#1910/#19…
stefan-burke Jul 24, 2026
1693aa5
Validate refund id/status as strings; document APPROVED (PR #1911 rev…
stefan-burke Jul 24, 2026
464576d
Drop SUCCEEDED (not a Square refund status) from the confirmed set (P…
stefan-burke Jul 24, 2026
ebec21d
Reject empty-string refund ids; sync refundPayment docstring (PR #191…
stefan-burke Jul 24, 2026
073f91b
Resolve all three merge blockers from independent review
stefan-burke Jul 24, 2026
b933296
Add placeholder recovery regression test + resolving system note + cl…
stefan-burke Jul 24, 2026
6c64434
Restrict placeholder bypass; limit resolving note; i18n; deleted-list…
stefan-burke Jul 24, 2026
0a97591
Validate refund status with picklist; check ledger for sale legs (PR …
stefan-burke Jul 24, 2026
ed2bfa7
Document picklist throw behavior; remove stale line-number references…
stefan-burke Jul 25, 2026
90b9bba
Use legMatches for account comparison; fix deleted-listing test ID (P…
stefan-burke Jul 25, 2026
c3bfaa1
Ensure 200 responses always reach Valibot parse (PR #1911 review)
stefan-burke Jul 25, 2026
e017a9e
Re-throw SyntaxError from invalid JSON 200 bodies (PR #1911 review)
stefan-burke Jul 25, 2026
8999951
Validate refund payment_id and amount_money at the boundary (PR #1911…
stefan-burke Jul 25, 2026
037b812
Fix refundPayment docstring: distinguish HTTP failures (false) from m…
stefan-burke Jul 25, 2026
99d3860
Add Square e2e refund exercise; verify refund amount; delete stale no…
stefan-burke Jul 25, 2026
23cae69
Split placeholder refresh tests; extract shared postPaymentLeg helper…
stefan-burke Jul 25, 2026
d549c0b
Make stale-note cleanup retryable on already-refunded attendees (PR #…
stefan-burke Jul 25, 2026
2532c11
Gate refund activity log on first-time only (PR #1911 review)
stefan-burke Jul 25, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 104 additions & 0 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -1330,6 +1330,110 @@ out of scope for that PR's brief — recorded here for a follow-up.*
a deliberate test review, which is the only way the test catches the failure
mode it is meant to catch.

## Stale equivalent-mutant line numbers across recent refactors

*Origin: running `deno task mutation:audit-equivalents` while hardening
`src/shared/square.ts` for the confirmed-Square-refunds job.*

`scripts/mutation/equivalent-mutants.txt` carries several entries whose
`file:line:col` no longer points at a generated mutant, so
`deno task mutation:audit-equivalents` aborts with "No generated mutant matches".
The mutants are still real and equivalent; only the code moved. Confirmed stale
entries span at least:
- `src/shared/uptime-kuma/socket.ts` (lines 107, 130, 167, 187 — `===`/`!==`↔loose)
- `src/shared/uptime-kuma/monitors.ts` (lines 87, 101, 116, 119, 138, 193, 227,
250, 306, 325 — `===`/`!==`↔loose and `1000→1001`)
- `src/shared/scheduled-access.ts:16:20 ??→||`
- `src/shared/storage.ts:376:18 application/octet-stream→""`
- `src/ui/templates/admin/images.tsx:84:28` and `:85:20 ??→||`
- `src/features/app/routes.ts:235:14 →"mutated"`

These most likely drifted when the Uptime Kuma modules were split into
one-concept-per-file (#1906) and other recent move/refactor PRs. The audit is a
standalone task (`mutation:audit-equivalents`, not part of `deno task
precommit`), so CI doesn't gate on it; it surfaced here only because the
Square-refunds job ran exhaustive mutation and used the audit to validate its
own equivalent entries. The `square.ts` entries in the equivalent-mutants
registry have been refreshed in place (they drift as the source shifts lines —
the audit command's own output lists every stale `file:line:col`);

Fix: for each stale entry, re-run `deno task mutation <file> '<tests>'
--exhaustive`, locate the surviving equivalent mutant's current `file:line:col`
from the report, and update the line/col in `equivalent-mutants.txt` (or remove
the entry if the static gates now kill it — `mutation:audit-equivalents --write`
does this automatically for entries the lint/type-check gates catch). Then
re-run the audit until it reports no stale entries. Starting point: the audit's
own output lists every stale `file:line:col`.

---

## Square PENDING refunds — propagate a pending result, not a plain false

*Origin: Codex review of PR #1911 (confirmed Square refund outcomes), thread
on `squareApi.refundPayment` (`src/shared/square.ts`). This PR deliberately
does NOT address it; recorded so the follow-on work can pick it up.*

`squareApi.refundPayment` returns `false` for a Square refund that is still
`PENDING` (an accepted-but-unsettled refund). That is the honest current-main
boolean contract this PR ships, but it has a real downstream cost the reviewer
flagged: the webhook/admin refund flow reads `refunded === false` as a failed
refund, so a pending Square refund releases the reservation, returns 503, and
— because each call mints a fresh `crypto.randomUUID()` idempotency key — a
redelivery posts another full-refund attempt instead of waiting on the
existing refund id. A PENDING Square refund is documented as a normal accepted
`RefundPayment` response, so collapsing it into `false` loses the "accepted,
not yet settled" signal.

Update: PR #1912 (stable Stripe and Square refund idempotency keys) has since
landed on `main`; the Square refund idempotency key is now the stable
`refundIdempotencyKey("square", paymentId)` rather than a fresh
`crypto.randomUUID()`, so a redelivery re-posts with the SAME key and Square
dedupes it — the double-pay half of the risk above is now mitigated. The
PENDING-still-returns-false behaviour itself (a retryable re-attempt that waits
on `isPaymentRefunded` rather than holding the refund id) remains, so the
pending-result union below is still the real fix; the stale-key concern is
resolved.

The fix is the staged-checkout pending-result union / callback resolution this
PR was explicitly told not to introduce: surface a pending outcome (carrying
the refund id) separately from a plain false, and have the webhook/admin refund
paths hold/redeliver against that id instead of re-posting. That is the same
machinery planned for #1853 (`split/staged-checkout-runtime` — "Finish and
recover paid checkouts safely") and overlaps #1905
(`split/authoritative-payment-callbacks` — provider-neutral webhook retry
resolution), so it must be designed with those branches, not duplicated here.
Starting points: `squareApi.refundPayment` in `src/shared/square.ts` (where the
boolean contract lives), the idempotency key in its `withClient` callback, and
the downstream `tryRefund` in `src/features/api/payment-processing/refunds.ts`
plus `refundReferenceAtProvider` in
`src/features/admin/refunds/provider.ts` (both treat `false` as failed and fall
back to `isPaymentRefunded`, which a still-pending refund also fails).

---

## Validate Square orders/payments responses with Valibot schemas

*Origin: CodeRabbit review of PR #1911. The refund response validation is done
(`SquareRefundResponseSchema` in `src/shared/square.ts`), and the test file
splits are complete (`refund-payment.test.ts`, `refund-transport.test.ts`,
and the shared `mock-fetch.ts` helper all exist; `retrieve-refund.test.ts` is
240 lines and `rest-transport.test.ts` is 372). What remains is extending the
same boundary-validation pattern to the orders and payments client methods.*

The Square REST client still maps order and payment responses with type casts
(`get<T>` for orders and payments). `squareFetch` returns `JSON.parse(response.text)`
cast as `<T>`, so a malformed order or payment object — wrong field types, an
unexpected shape — passes through unvalidated. The refund path now has a Valibot
schema (`SquareRefundSchema` / `SquareRefundResponseSchema`) parsed with
`v.parse` OUTSIDE `withClient`, so a malformed refund response fails loudly.
Doing the same for orders and payments means defining `SquareOrderSchema` and
`SquarePaymentSchema` and parsing in their respective `squareApi` methods, so
a malformed response throws rather than being silently cast. Starting point:
`squareFetch` and the `SquareOrderResponse` / `SquarePaymentResponse` types in
`src/shared/square.ts`; mirror the refund schema shape that already exists.

---

## Split oversized test files moved by PR #1903

*Origin: Codex review of PR #1903 ("Load heavy modules only when needed").
Expand Down
60 changes: 59 additions & 1 deletion e2e-payments/src/providers/shared.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
/* jscpd:ignore-start */
import type { BrowserSession } from "#e2e/browser.ts";
import { type BrowserSession, requirePageText } from "#e2e/browser.ts";
import type { ProviderName } from "#e2e/config.ts";
import { config } from "#e2e/config.ts";
import { BOOKER_NAME } from "#e2e/flow.ts";
import { log } from "#e2e/log.ts";
import type { ConfigureProvider, PayHostedCheckout } from "./types.ts";

Expand Down Expand Up @@ -70,3 +71,60 @@ export const hostedCheckout =
await page.waitForLoadState("domcontentloaded");
await drive(page, ctx);
};

/**
* Exercise the admin refund flow after a paid booking: open the attendee,
* refresh the payment status (polls the real provider API), then submit the
* admin refund form and verify the refund is recorded. This exercises the
* provider's real sandbox refund API (POST /v2/refunds for Square,
* refunds.create for Stripe), the Valibot boundary validation, and the ledger
* posting — the full round-trip from admin UI through provider to ledger.
*/
export const exerciseAdminRefund = async (
session: BrowserSession,
): Promise<void> => {
const attendee = session.page.getByRole("link", {
exact: true,
name: BOOKER_NAME,
});
const attendeeHref = await attendee.getAttribute("href");
if (!attendeeHref) {
throw new Error(`Could not open paid attendee "${BOOKER_NAME}"`);
}
await session.goto(attendeeHref);

await session.clickButton("Refresh payment status");
await requirePageText(
session,
"Payment status is up to date",
"payment-status-failed",
'Expected the app page to contain "Payment status is up to date"',
);

await session.clickLink("Actions");
await session.clickLink("Refund");
await session.fill("confirm_identifier", BOOKER_NAME);
await session.clickButton("Refund Attendee");
await requirePageText(
session,
"Refund issued",
"refund-failed",
'Expected the app page to contain "Refund issued"',
Comment on lines +108 to +112

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Wait for accepted Square refunds in the e2e flow

When the Square sandbox returns the documented PENDING outcome, this assertion fails even though the refund request was accepted: squareApi.refundPayment now returns false for that status, and the inspected admin path in src/features/admin/refunds/provider.ts:39-56 performs only one immediate isPaymentRefunded check before rendering “Refund failed.” The new Square afterPaidBooking hook therefore makes the nightly journey depend on the refund becoming COMPLETED synchronously; poll through the refresh flow until the accepted refund settles before requiring the success flash.

Useful? React with 👍 / 👎.

);

await session.clickLink("Overview");
const paymentDetails = await session.page
.locator(".prose", { hasText: "Payment Details" })
.first()
.innerText();
if (
!paymentDetails.includes("Refund Status:") ||
!paymentDetails.includes("Refunded")
) {
await session.dumpPage("refund-not-recorded");
throw new Error(
`Refund was not recorded on the attendee. Payment details:\n${paymentDetails}`,
);
}
log(" refund, ledger recording, and status verification passed");
};
11 changes: 10 additions & 1 deletion e2e-payments/src/providers/square.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import type { Page } from "playwright";
import { log } from "#e2e/log.ts";
import { sleep } from "#e2e/util.ts";
import { squareRequestInit } from "#shared/square.ts";
import { configureProvider, hostedCheckout } from "./shared.ts";
import {
configureProvider,
exerciseAdminRefund,
hostedCheckout,
} from "./shared.ts";
import type { HostedCheckoutContext, PaymentProvider } from "./types.ts";

/**
Expand Down Expand Up @@ -193,6 +197,11 @@ const completeViaSandboxApi = async (
};

export const square: PaymentProvider = {
// Exercise the real Square sandbox refund API (POST /v2/refunds → Valibot
// schema parse → COMPLETED check → payment_id/amount verification → ledger
// posting) plus the admin refresh-payment route. This is the only e2e path
// that exercises the confirmed-Square-refund contract from this PR.
afterPaidBooking: exerciseAdminRefund,
configure: configureProvider("square", async (session, secrets) => {
await session.fill("square_access_token", secrets.token);
await session.fill("square_location_id", secrets.locationId);
Expand Down
72 changes: 7 additions & 65 deletions e2e-payments/src/providers/stripe.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
/* jscpd:ignore-start */
import { type BrowserSession, requirePageText } from "#e2e/browser.ts";
import type { BrowserSession } from "#e2e/browser.ts";
import { config } from "#e2e/config.ts";
import { BOOKER_NAME } from "#e2e/flow.ts";
import { log, warn } from "#e2e/log.ts";
import { clickFirst, fillFirst } from "./card.ts";
import { configureProvider, hostedCheckout } from "./shared.ts";
import {
configureProvider,
exerciseAdminRefund,
hostedCheckout,
} from "./shared.ts";
import type { PaymentProvider } from "./types.ts";

/* jscpd:ignore-end */
Expand Down Expand Up @@ -47,54 +50,6 @@ const testStripeConnection = async (session: BrowserSession): Promise<void> => {
);
};

const exerciseStripeRefund = async (session: BrowserSession): Promise<void> => {
const attendee = session.page.getByRole("link", {
exact: true,
name: BOOKER_NAME,
});
const attendeeHref = await attendee.getAttribute("href");
if (!attendeeHref) {
throw new Error(`Could not open paid attendee "${BOOKER_NAME}"`);
}
await session.goto(attendeeHref);

// This polls the real PaymentIntent with latest_charge expanded.
await session.clickButton("Refresh payment status");
await requirePageText(
session,
"Payment status is up to date",
"stripe-payment-status-failed",
'Expected the app page to contain "Payment status is up to date"',
);

await session.clickLink("Actions");
await session.clickLink("Refund");
await session.fill("confirm_identifier", BOOKER_NAME);
await session.clickButton("Refund Attendee");
await requirePageText(
session,
"Refund issued",
"stripe-refund-failed",
'Expected the app page to contain "Refund issued"',
);

await session.clickLink("Overview");
const paymentDetails = await session.page
.locator(".prose", { hasText: "Payment Details" })
.first()
.innerText();
if (
!paymentDetails.includes("Refund Status:") ||
!paymentDetails.includes("Refunded")
) {
await session.dumpPage("stripe-refund-not-recorded");
throw new Error(
`Stripe refund was not recorded on the attendee. Payment details:\n${paymentDetails}`,
);
}
log(" Stripe PaymentIntent lookup and full refund passed");
};

/**
* Whether `url` points at a cloudflared quick-tunnel host
* (`trycloudflare.com` or any `*.trycloudflare.com` subdomain). Substring
Expand All @@ -116,21 +71,8 @@ const isTrycloudflareTunnelUrl = (raw: string | undefined): boolean => {
}
};

/**
* Stripe. Configuring the key registers a webhook endpoint against the site's
* public HTTPS URL, so this provider REQUIRES the cloudflared tunnel.
*
* Hosted Stripe Checkout (checkout.stripe.com) exposes its inputs at the top
* level (not iframed), addressable via the WHATWG cc-* autocomplete tokens the
* generic card filler tries first. The billing country shown on Checkout is
* driven by the Stripe account, so the postal field expects a matching format —
* a US sandbox account rejects a UK postcode ("your ZIP is incomplete"). Set up
* the site as US/USD and enter a US ZIP so the two agree.
* Sandbox test card: 4242 4242 4242 4242, any future expiry, any CVC.
* Docs: https://docs.stripe.com/testing
*/
export const stripe: PaymentProvider = {
afterPaidBooking: exerciseStripeRefund,
afterPaidBooking: exerciseAdminRefund,
// Each run registers a webhook endpoint for its ephemeral *.trycloudflare.com
// URL, and the throwaway DB forgets the id — so without cleanup they pile up
// and Stripe eventually rejects new ones (accounts cap webhook endpoints).
Expand Down
4 changes: 2 additions & 2 deletions scripts/mutation/equivalent-mutants.txt
Original file line number Diff line number Diff line change
Expand Up @@ -872,8 +872,8 @@ src/shared/runtime.ts:67:45 ?? → || # nodeCompatVersion is string|undefined
src/shared/runtime.ts:68:18 ?? → || # os is string|undefined and falls back to "", so both operators agree
src/shared/runtime.ts:70:43 ?? → || # typescriptVersion is string|undefined and falls back to "", so both operators agree
src/shared/runtime.ts:71:38 ?? → || # userAgent is string|undefined and falls back to "", so both operators agree
src/shared/square.ts:644:18 ??|| # the callback returns true while missing clients and caught errors return null; both operators produce the same boolean
src/shared/square.ts:714:37 ?? → || # locations is an array or undefined; arrays are truthy and undefined takes [] under both operators
src/shared/square.ts:312:21 ?:consequent only # the buyer_phone_number ternary spreads {} when the phone is absent; the consequent-only mutant always spreads { buyer_phone_number: undefined }, but JSON.stringify omits undefined values, so the serialized request body is identical in both cases
src/shared/square.ts:783:37 ?? → || # locations is an array or undefined; arrays are truthy and undefined takes [] under both operators

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Refresh the shifted locations mutant entry

Fresh evidence after the earlier thread: the final amount-verification block shifted response.locations ?? [] to src/shared/square.ts:795:37, but this changed registry entry still records :783:37, which now points to the closing };. Because resolveEntries performs an exact location/operator lookup, deno task mutation:audit-equivalents aborts with No generated mutant matches before auditing the registry; update this entry to the new location.

AGENTS.md reference: AGENTS.md:L946-L949

Useful? React with 👍 / 👎.

src/ui/client/dom.ts:13:19 = → += # createElement returns a new button with className "", so both assignments produce the supplied class string

# Paid-payment processing: private defaults are always supplied by their only
Expand Down
Loading