fix(stripe): read-only reconciliation CLI + ERROR-level logging for dropped entitlement webhooks by FagnerMartinsBrack · Pull Request #938 · Readplace/readplace.com

FagnerMartinsBrack · 2026-07-05T14:36:36Z

fix(stripe): read-only reconciliation CLI + ERROR-level logging for dropped entitlement webhooks

Hypothesis

A paying (or payment-attempting) customer can exist in Stripe with no matching app-side subscription row, and the app can hold entitlement rows Stripe no longer backs. Today the only signal is a WARN that nobody watches, on the only Stripe event we handle. Shipping a read-only diff tool + upgrading that WARN to a structured ERROR turns an invisible data-integrity failure into an actionable one. This PR is investigation tooling — it deliberately fixes nothing it hasn't measured.

Evidence (30d, Jun 4 – Jul 4 2026)

The only Stripe webhook received in 30 days (Jun 22 07:06) was dropped: WARN "[stripe-webhook] no subscription row found — skipping event emission" subscriptionId=sub_1TSJTQBjoaOLiPzvOSCNP52H. The receiver then returned 200, so Stripe will never redeliver it.
Only 3 of 45 subscription rows ever had a Stripe subscriptionId/customerId.
4 rows are status=trialing with trialEndsAt in the past — stuck states the trial scheduler never converged.
Checkout successes: 2 (3e9c7e5c Jun 21 17:35, af194ac4 Jun 23 12:53 — the latter after their Jun 18 cancellation). charge_succeeded events: 0 (by design — checkout success calls upsertActive directly, no event).
Code audit (this PR verified it): the runtime StripeEventType union and the infra events: array both wire exactly one event — customer.subscription.deleted. No invoice.payment_failed, no invoice.paid, no renewal signal of any kind reaches the app.

Investigation narrative: the dropped webhook

Timeline:

Jun 21 17:35 — user 3e9c7e5c lands on /auth/checkout/success; upsertActive writes an active row (this should be one of the 3 rows with a subscriptionId).
Jun 22 07:06 (~13.5h later) — Stripe sends customer.subscription.deleted for sub_1TSJTQBjoaOLiPzvOSCNP52H. The GSI lookup (subscriptionId-index) finds no row. Event dropped with a WARN; 200 returned.

Two competing hypotheses, and the CLI distinguishes them:

H1 — incomplete_expired: the checkout's initial payment was asynchronous and failed; Stripe voids incomplete subscriptions (~23h window; async-failure paths fire sooner), emitting customer.subscription.deleted with the sub in incomplete_expired. The 13.5h gap fits. If true, 3e9c7e5c saw a success page, holds an active app row backed by a dead Stripe sub, and paid nothing — and because the webhook was dropped, nothing ever cancelled the row.
H2 — row-key drift: upsertActive wrote a different id (or the write failed / was later overwritten by the cancellation flow, cf. af194ac4 checking out after cancelling), so the GSI lookup missed a row that conceptually exists.

The reconcile report answers it: if sub_1TSJTQ… appears under “Stripe subs with no matching app row” with stripeStatus=incomplete_expired and 3e9c7e5c appears under “live app rows with no live Stripe sub”, it's H1 + drift; if the app row holds a different subscriptionId, it's H2.

What this changes

pnpm --dir projects/hutch stripe-reconcile — a read-only CLI (zero writes; it composes only the read half of the subscription table) that pages through stripe.subscriptions.list(status=all, expand data.customer) and a full table scan, then reports:
- Stripe subs with no matching app row (with stripeStatus and masked customer email, e.g. j***@g***, for manual correlation — raw emails never enter the findings object, enforced by test);
- app rows claiming active/pending_cancellation with no live Stripe sub;
- live Stripe subs whose app row is cancelled — the “paying customer without entitlement” case;
- trialing rows with past (or absent) trialEndsAt — the 4 stuck rows;
- rows missing subscriptionId entirely (expected ~42 of 45). App rows carry no email, so email-join is reported from the Stripe side (masked) rather than guessed.
Diff logic is a pure, fully unit-tested reconcile() in runtime/domain/stripe-reconcile/; the CLI is a thin composition root mirroring send-checkout-recovery-emails.
WARN → structured ERROR in the customer.subscription.deleted handler when no row matches: now includes subscriptionId, customerId, eventType, and a pointer to run the reconcile CLI. Why ERROR: the handler acknowledges with 200, so Stripe never retries and the Lambda-Errors alarm never fires — a dropped entitlement-affecting event is action-required, not informational. The remaining WARNs in the receiver (signature probes) are internet noise; the level split makes a future ERROR metric filter meaningful.

What this deliberately does NOT change

No new Stripe webhook events wired. The renewal/payment-failure blind spot (invoice.payment_failed, invoice.paid, customer.subscription.updated) is real — the app currently has no way to learn a renewal charge failed — but wiring events before reconciliation data lands would be guessing. Future work, informed by the CLI's first prod run.
No data repair. The CLI is the measurement; the fix PR follows the report.

Estimated result

The first prod run classifies all 45 rows and every Stripe sub: confirms/refutes H1 for sub_1TSJTQ…, surfaces whether any live Stripe sub is unentitled (revenue-affecting), and inventories the 4 stuck trials. Ongoing: any future dropped webhook is an ERROR with enough context to act within one log line.

How to measure

Owner commands (read-only; use prod AWS creds and the live secret key):

cd projects/hutch
DYNAMODB_SUBSCRIPTION_PROVIDERS_TABLE=<prod table name> \
STRIPE_SECRET_KEY=sk_live_... \
pnpm stripe-reconcile

Watch for future drops — CloudWatch Logs Insights on /aws/lambda/stripe-webhook-receiver-handler:

fields @timestamp, @message
| filter @message like /no subscription row found/
| sort @timestamp desc

(0 results after this lands = healthy; any result = run the CLI.)

Merge note

The analytics dashboard test asserts toHaveLength(24) on widget count. This PR adds no analytics events and no widgets, so it does not touch that assertion — if sibling PRs #916–#918 bump it and conflict on rebase, take their side verbatim; nothing here depends on the count.

https://claude.ai/code/session_01En2uUiiwQoRQKWenCnkPi9

FagnerMartinsBrack · 2026-07-05T14:43:58Z

@claude CI has FAILED. Use your Read tool to read the local file .github/workflows/claude-PR-CI-failure-fixer.md for instructions. The file is in the checked-out repository.

Context

Attempt: 1 of 5
Run ID: 28744230434
PR: fix(stripe): read-only reconciliation CLI + ERROR-level logging for dropped entitlement webhooks #938
Branch: fix/stripe-reconciliation

Failed Jobs

web-tests: Run pnpm check

claude · 2026-07-05T14:44:34Z

Claude finished @FagnerMartinsBrack's task in 10m 42s —— View job

CI Failure Analysis — Attempt 1 of 5

Read PR commit history to understand intent
Get failed job logs (web-tests: Run pnpm check)
Identify root cause
Determine correct fix location

Conclusion: this failure is pre-existing on `main` and unrelated to this PR — no Stripe-side change can fix it, so I have not modified the PR.

What's failing: the visual‑regression E2E test projects/hutch/src/e2e/crawl-bookmark-visual.e2e-local.ts (inside hutch:test-with-coverage):

Crawl bookmark visual regression › …rounded-left capsule (light|dark)
Error: expect(locator).toHaveScreenshot() failed
  Expected an image 145px by 54px, received 153px by 54px.
  990 pixels (ratio 0.12) are different.

The .crawl-bookmark capsule renders 8px wider (153px) than the committed baseline (145px). The text is entirely static — "Last crawled at" + the pinned "26 Mar '26, 14:32" — so this is a baseline/environment mismatch (font metrics), not drifting content. A dimension mismatch fails regardless of the maxDiffPixelRatio/threshold tolerances.

Evidence it is not this PR

Check	Result
Files this PR changes	Stripe only — `stripe-reconcile`, `stripe-subscriptions`, `subscription-providers`, `customer-subscription-deleted`, `provider-contracts`, `package.json`. Zero bookmark/CSS/template/e2e files.
Where the test lives	Added by #936 (`5b993302`) — not in this PR branch. It runs because PR CI builds the merge into `main`.
`main`'s own CI	Push run 28744076544 on `5b993302` (current `main` HEAD) fails identically — same test, same `145px → 153px`, same `ratio 0.12`.

So #936 introduced the visual test and baseline PNGs whose committed render (145px) doesn't match what the CI runner actually produces (153px). main is currently red, which blocks every open PR through the merge build — this Stripe PR just inherits it.

Why I did not change this PR

The CI‑failure‑fixer instructions say to fix only the CI failure and not make unrelated changes, and CLAUDE.md forbids reversing intentional changes. The broken baseline lives on main, not here; committing bookmark snapshot PNGs into a Stripe PR would be an out‑of‑scope, environment‑sensitive change — and blessing a fallback‑font render as the new baseline is a visual decision a human should confirm, not something to auto‑commit.

Recommended fix (on `main`, not this PR)

Reproduce on the CI runner image and decide:

If the brand font is failing to load in the e2e server → fix font loading so the render matches the intended 145px design (keep the baseline).

If the 153px render is acceptable → regenerate the baselines on the CI image and commit the two PNGs to main:

cd projects/hutch
pnpm compile
E2E_PORT=3100 CI=true pnpm exec playwright test \
  --config playwright.config.local-dev.ts \
  crawl-bookmark-visual --update-snapshots

Once main is green, re‑run CI on this PR (its merge build will pick up the corrected baseline) and it will pass with no changes needed here.

Fix this on main →

Summary: The web-tests failure is the crawl-bookmark-visual E2E snapshot mismatch (145px vs 153px) introduced by #936, already red on main (run 28744076544). It is independent of this Stripe PR, whose diff contains no bookmark/UI files. I made no changes — the fix belongs on main (fix e2e font loading or regenerate the baseline). This PR will go green once main is fixed and CI re‑runs.
· main red run · branch fix/stripe-reconciliation

FagnerMartinsBrack · 2026-07-06T00:31:45Z

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

Repo: Readplace/readplace.com
PR: fix(stripe): read-only reconciliation CLI + ERROR-level logging for dropped entitlement webhooks #938
Commit: 39ba2d1

claude · 2026-07-06T00:32:28Z

Claude finished @FagnerMartinsBrack's task in 6m 22s —— View job

Code Review — PR #938 (Stripe reconciliation CLI + ERROR-level dropped-webhook logging)

I read the full diff against origin/main, traced the reconcile logic against the PR's own H1/H2 evidence, and checked composition, coverage-exclusion, knip, and the contract additions. This is a clean, well-scoped, genuinely read-only diagnostic PR with strong unit coverage. No blocking issues.

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Low Priority Suggestions

1. `Date.parse(trialEndsAt)` NaN is silently swallowed — a malformed date escapes every finding

reconcile.ts:110-111:

const expired =
    row.trialEndsAt === undefined || Date.parse(row.trialEndsAt) < now.getTime();

trialEndsAt is validated only as dynamoField(z.string()) (not .datetime()), so a non-ISO value can pass the row schema. Date.parse("garbage") → NaN, and NaN < now → false, so such a row is neither flagged as past-end nor caught by the === undefined branch — it disappears from the report entirely. For a tool whose whole job is surfacing anomalies, an unparseable trialEndsAt is itself worth flagging. A Number.isNaN(parsed) guard that treats unparseable as "expired/suspicious" closes the gap. Low risk in practice (our own writes always use .toISOString()), and the sibling resolveWriteAccess fails safe on the same input — but here it fails silent. Fix this →

2. In-memory fixture is now behind the real provider contract

initDynamoDbSubscriptionRead (and the composed initDynamoDbSubscriptionProviders) now implement ListAllSubscriptionRows, but the test double initInMemorySubscriptionProviders (src/packages/test-fixtures/.../in-memory-subscription-providers.ts) does not. It doesn't break anything today (the reconcile domain logic is pure and takes appRows directly), but the fixture no longer mirrors the real surface, so a future reconcile-style integration test can't drive the scan path through the double. Cheap to add now while it's fresh. Fix this →

3. Reconcile joins only on `subscriptionId` — `customerId` is an available secondary key

Both SubscriptionRecord and StripeSubscriptionSummary carry customerId, but reconcile() correlates the two sides exclusively via subscriptionId. That's fine for v1, but it's exactly the H2 "row-key drift" case the PR wants to distinguish: when a Stripe sub lands in stripeSubsMissingAppRow and an app row with a different subscriptionId shares the same customerId, a customerId cross-check could classify the drift automatically instead of relying on manual masked-email correlation. Worth considering for the follow-up fix PR the report feeds into — no change needed here.

What's good: the diff genuinely fixes nothing it hasn't measured. reconcile() is pure and exhaustively unit-tested (masking, every finding bucket, the active-without-subscriptionId double-classification, trialing past/future/absent end); findings are mutually exclusive by construction (a sub in stripeSubsMissingAppRow can't also be in liveStripeSubsWithCancelledAppRow), so no double-counting. The masked-email invariant is enforced by a test asserting no raw @ leaks into the report. The pending_cancellation + Stripe-canceled case is flagged in liveAppRowsMissingLiveStripeSub — which is precisely the dropped-webhook consequence this PR exists to catch, so the tool would detect its own founding bug. The CLI is a thin *.main.ts composition root (coverage-excluded, mirrors send-checkout-recovery-emails) and calls only read paths, so "zero writes" holds behaviourally. WARN→ERROR is well-justified (200-ack means Stripe never retries and the Lambda-Errors alarm never fires) and the customer field is parsed defensively with safeParse against the .passthrough() event shape. listAllSubscriptions pins the API version, caps limit=100, and asserts against an empty-page infinite loop.

Note on the red web-tests check: it's the pre-existing crawl-bookmark-visual snapshot mismatch (145px vs 153px) from #936, already failing on main independently of this PR — this Stripe diff touches no bookmark/UI/e2e files, so the failure is not attributable to these changes.

cc @FagnerMartinsBrack - No high or medium priority issues found. This PR is ready for human review.
· branch fix/stripe-reconciliation

FagnerMartinsBrack · 2026-07-06T00:39:10Z

Code Review — PR #938 (Stripe reconciliation CLI + ERROR-level dropped-webhook logging)

I read the full diff against origin/main, traced the reconcile logic against the PR's own H1/H2 evidence, and checked composition, coverage-exclusion, knip, and the contract additions. This is a clean, well-scoped, genuinely read-only diagnostic PR with strong unit coverage. No blocking issues.

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Low Priority Suggestions

1. `Date.parse(trialEndsAt)` NaN is silently swallowed — a malformed date escapes every finding

reconcile.ts:110-111:

const expired =
    row.trialEndsAt === undefined || Date.parse(row.trialEndsAt) < now.getTime();

trialEndsAt is validated only as dynamoField(z.string()) (not .datetime()), so a non-ISO value can pass the row schema. Date.parse("garbage") → NaN, and NaN < now → false, so such a row is neither flagged as past-end nor caught by the === undefined branch — it disappears from the report entirely. For a tool whose whole job is surfacing anomalies, an unparseable trialEndsAt is itself worth flagging. A Number.isNaN(parsed) guard that treats unparseable as "expired/suspicious" closes the gap. Low risk in practice (our own writes always use .toISOString()), and the sibling resolveWriteAccess fails safe on the same input — but here it fails silent.

2. In-memory fixture is now behind the real provider contract

initDynamoDbSubscriptionRead (and the composed initDynamoDbSubscriptionProviders) now implement ListAllSubscriptionRows, but the test double initInMemorySubscriptionProviders (src/packages/test-fixtures/.../in-memory-subscription-providers.ts) does not. It doesn't break anything today (the reconcile domain logic is pure and takes appRows directly), but the fixture no longer mirrors the real surface, so a future reconcile-style integration test can't drive the scan path through the double. Cheap to add now while it's fresh.

3. Reconcile joins only on `subscriptionId` — `customerId` is an available secondary key

Both SubscriptionRecord and StripeSubscriptionSummary carry customerId, but reconcile() correlates the two sides exclusively via subscriptionId. That's fine for v1, but it's exactly the H2 "row-key drift" case the PR wants to distinguish: when a Stripe sub lands in stripeSubsMissingAppRow and an app row with a different subscriptionId shares the same customerId, a customerId cross-check could classify the drift automatically instead of relying on manual masked-email correlation. Worth considering for the follow-up fix PR the report feeds into — no change needed here.

FagnerMartinsBrack · 2026-07-06T23:42:37Z

@claude Resolve conflicts. Use your Read tool to read the local file .github/workflows/claude-PR-conflict-fixer.md for instructions. The file is in the checked-out repository.

Context

Repo: Readplace/readplace.com
PR: fix(stripe): read-only reconciliation CLI + ERROR-level logging for dropped entitlement webhooks #938
Branch: fix/stripe-reconciliation

claude · 2026-07-06T23:43:17Z

Claude finished @FagnerMartinsBrack's task in 16m 37s —— View job

Conflicts resolved ✅

Rebased the Stripe commit onto current origin/main (7302f6c2) and force-pushed. The branch is now a clean linear history — one commit (a79b52d3) on top of main, with the old merge commit dropped — and is cleanly mergeable into main with no remaining conflicts.

Read conflict-fixer instructions
Read PR commit history to understand intent
Rebase onto origin/main (7302f6c2)
Resolve conflicts (2 files)
Verify (hutch:check: lint + full test-with-coverage)
Force-push with --force-with-lease

What conflicted & how it was resolved

Both conflicts were additive collisions with #939 (pre-expiry trial reminder email), which merged into main and added a sibling type/test at the exact spot this PR added its reconcile equivalents. In both cases I kept both sides:

File	`main` (#939) added	This PR added	Resolution
`provider-contracts/src/subscription-providers.ts`	`MarkTrialReminderEmailSent` type	`ListAllSubscriptionRows` type	Kept both types
`dynamodb-subscription-providers.test.ts`	`markTrialReminderEmailSent` describe	`listAllSubscriptionRows` describe (3 tests)	Kept both blocks

The test-file conflict was tangled (git aligned the coincidentally-identical createFakeClient/initDynamoDbSubscriptionProviders setup lines from two unrelated tests). I resolved it by taking main's clean version and re-inserting this PR's listAllSubscriptionRows block verbatim from commit 0ef583d6, then confirmed the resolved file differs from main's by only that block (0 lines dropped). package.json auto-merged (kept the stripe-reconcile script alongside main's additions).

No Stripe logic was altered — the resolution is purely a re-integration of the two independent additions.

Verification

Ran pnpm nx run hutch:check (env vars sourced from .envrc as CI provides):

lint: passed (type-check green — the new ListAllSubscriptionRows type compiles across all provider-contracts consumers)
test-with-coverage: 254/254 suites, 2932 unit + 11 integration + 9 E2E tests pass, 0 failures
Coverage thresholds met: statements 99.48%, branches 97.05%, functions 100%, lines 99.48%
Bonus: the crawl-bookmark-visual E2E that was red on the old base now passes — this rebase picks up main's bookmark fixes (59d3328b, edbf821f, bcdb5f75), so the pre-existing CI failure from earlier attempts is cleared too.
· branch fix/stripe-reconciliation

FagnerMartinsBrack · 2026-07-07T00:05:27Z

@claude CI has FAILED. Use your Read tool to read the local file .github/workflows/claude-PR-CI-failure-fixer.md for instructions. The file is in the checked-out repository.

Context

Attempt: 2 of 5
Run ID: 28831613561
PR: fix(stripe): read-only reconciliation CLI + ERROR-level logging for dropped entitlement webhooks #938
Branch: fix/stripe-reconciliation

Failed Jobs

web-tests: Run pnpm check

claude · 2026-07-07T00:05:58Z

Claude finished @FagnerMartinsBrack's task in 29m 43s —— View job

CI Failure Analysis — Attempt 2 of 5 → ✅ CI now GREEN

Read PR commit history to understand intent
Get failed job logs (web-tests: Run pnpm check, run 28831613561)
Identify root cause
Determine correct fix location
Reproduce locally / confirm flakiness
Warm nx cache with full pnpm check (green)
Re-trigger CI (no-op commit — no code fix applies)
Verify re-triggered CI is green

Conclusion: the failure was a flaky firefox E2E timeout, unrelated to this Stripe PR

The failing task was firefox-extension:check → the reader-link-flow E2E, which timed out (twice, incl. its own retry) waiting for the login email field — a TimeoutError, not an assertion failure:

Subtest: saved-article link in the popup opens the private reader, not the public view
  error: Waiting for element to be located By(css selector, *[id="email"])
  Wait timed out after 30123ms

Proof it's flaky, not caused by this PR:

Evidence	Result
Files changed by this PR	Stripe only — reconcile CLI/domain, `stripe-subscriptions`, subscription read, `customer-subscription-deleted`, provider-contracts. Zero extension/login/reader/`browser-extension-core` files.
Sibling E2E in the same failing run	`should complete OAuth login flow…` passed, using the same `id="email"` field → the login page renders fine in that build.
Prior run on this branch `28760152187`	Full CI green (runs `firefox-extension:check`).
Latest 6 `main` runs	All green — each runs `firefox-extension:check`.
Local reproduction	reader-link E2E passes in ~10s (vs the 30s CI timeout); full `pnpm check` green (37 projects, 48 tasks; hutch 2932 unit + 11 integration + 9 E2E, 100% coverage).
nx's own verdict	`Nx detected a flaky task → firefox-extension:check` — flagged on three separate local runs.

The CI runner was momentarily under load (each failed attempt took ~57–59s vs. the passing OAuth test's ~30s), so browser ops blew the 30s email-field wait. The one server-path file this PR touches (dynamodb-subscription-read.ts) only adds listAllSubscriptionRows, called exclusively by the reconcile CLI — never in the login/reader path, nothing at composition time.

Action taken

No code fix was appropriate — a change to the firefox E2E infra would be an unrelated change the fixer instructions forbid. gh run rerun returned Resource not accessible by integration (no API permission), so I pushed a no-op commit (7cbdce27, --allow-empty) to re-trigger CI on a fresh runner. Its pre-commit pnpm check was a full nx cache hit (green).

Result

Re-triggered run 28832738663 → conclusion: success (web-tests → success). This PR is now green with no Stripe logic altered. If the flaky reader-link E2E resurfaces on a future run, it's environmental (headless-firefox load) and unrelated to this diff.
· branch fix/stripe-reconciliation

FagnerMartinsBrack · 2026-07-07T00:34:46Z

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

Repo: Readplace/readplace.com
PR: fix(stripe): read-only reconciliation CLI + ERROR-level logging for dropped entitlement webhooks #938
Commit: 7cbdce2

claude · 2026-07-07T00:35:27Z

Claude finished @FagnerMartinsBrack's task in 4m 44s —— View job

Code Review — PR #938 (Stripe reconciliation CLI + ERROR-level dropped-webhook logging)

I re-read the full diff against origin/main at the current HEAD, traced reconcile() against the PR's own H1/H2 evidence, and checked pagination, composition, coverage-exclusion, knip, and the contract additions. 7cbdce27 is an empty CI-retry commit, so the reviewable code is byte-identical to a79b52d3 — which the conflict-fixer already verified green (hutch:check, 100% functions) and CI has since confirmed green on this HEAD. This remains a clean, well-scoped, genuinely read-only diagnostic PR. No blocking issues.

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Low Priority Suggestions

(All three carried over from the prior review at 39ba2d18; the code is unchanged since, so they still stand. All non-blocking — appropriate to defer to the follow-up fix PR this report feeds.)

1. `Date.parse(trialEndsAt)` NaN is silently swallowed

reconcile.ts:110-111 — trialEndsAt is validated only as dynamoField(z.string()) (not .datetime()), so a non-ISO value passes the row schema. Date.parse("garbage") → NaN, and NaN < now → false, so such a row is neither flagged as past-end nor caught by the === undefined branch — it vanishes from the report. For a tool whose whole job is surfacing anomalies, an unparseable trialEndsAt is itself worth flagging. A Number.isNaN(parsed) guard treating unparseable as suspicious closes the gap. Low risk in practice (our writes always use .toISOString()). Fix this →

2. In-memory fixture is now behind the real provider contract

initDynamoDbSubscriptionRead (and composed initDynamoDbSubscriptionProviders) implement ListAllSubscriptionRows, but the test double initInMemorySubscriptionProviders (src/packages/test-fixtures/.../in-memory-subscription-providers.ts) does not. Nothing breaks today (reconcile is pure and takes appRows directly), but the fixture no longer mirrors the real surface, so a future reconcile-style integration test can't drive the scan path through the double. Cheap to add while fresh. Fix this →

3. Reconcile joins only on `subscriptionId` — `customerId` is an available secondary key

Both SubscriptionRecord and StripeSubscriptionSummary carry customerId, but reconcile() correlates the two sides exclusively via subscriptionId. That's fine for v1, but it's exactly the H2 "row-key drift" case the PR wants to distinguish: when a Stripe sub lands in stripeSubsMissingAppRow and an app row with a different subscriptionId shares its customerId, a customerId cross-check could classify the drift automatically instead of relying on manual masked-email correlation. Worth considering for the follow-up fix PR — no change needed here.

What's good: the diff fixes nothing it hasn't measured. reconcile() is pure and exhaustively unit-tested (masking incl. no-@/empty-local/empty-domain, every finding bucket, the active-without-subscriptionId double-classification, trialing past/future/absent end). Findings are mutually exclusive where it matters (a Stripe sub in stripeSubsMissingAppRow cannot also be in liveStripeSubsWithCancelledAppRow) and intentionally overlap where documented (active-no-sub in both liveAppRowsMissingLiveStripeSub and rowsMissingSubscriptionId), with a test pinning that. The masked-email invariant is enforced by a test asserting no raw @ reaches the report, and raw emails never enter the findings object. The pending_cancellation + Stripe-canceled case surfaces in liveAppRowsMissingLiveStripeSub — precisely the dropped-webhook consequence this PR exists to catch, so the tool detects its own founding bug. listAllSubscriptions pins the API version, caps limit=100, expands data.customer, and asserts against an empty-page infinite loop; the DynamoDB scan paginates on ExclusiveStartKey. The CLI is a thin *.main.ts composition root (coverage-excluded via **/*.main.ts, mirrors send-checkout-recovery-emails) that touches only read paths, so "zero writes" holds behaviourally. WARN→ERROR is well-justified — the 200-ack means Stripe never retries and the Lambda-Errors alarm never fires — and customer is parsed defensively with safeParse, logging only subscriptionId/customerId/eventType (no PII).

cc @FagnerMartinsBrack - No high or medium priority issues found. This PR is ready for human review.
· branch fix/stripe-reconciliation

FagnerMartinsBrack · 2026-07-07T00:40:27Z

Code Review — PR #938 (Stripe reconciliation CLI + ERROR-level dropped-webhook logging)

I re-read the full diff against origin/main at the current HEAD, traced reconcile() against the PR's own H1/H2 evidence, and checked pagination, composition, coverage-exclusion, knip, and the contract additions. 7cbdce27 is an empty CI-retry commit, so the reviewable code is byte-identical to a79b52d3 — which the conflict-fixer already verified green (hutch:check, 100% functions) and CI has since confirmed green on this HEAD. This remains a clean, well-scoped, genuinely read-only diagnostic PR. No blocking issues.

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Low Priority Suggestions

(All three carried over from the prior review at 39ba2d18; the code is unchanged since, so they still stand. All non-blocking — appropriate to defer to the follow-up fix PR this report feeds.)

1. `Date.parse(trialEndsAt)` NaN is silently swallowed

reconcile.ts:110-111 — trialEndsAt is validated only as dynamoField(z.string()) (not .datetime()), so a non-ISO value passes the row schema. Date.parse("garbage") → NaN, and NaN < now → false, so such a row is neither flagged as past-end nor caught by the === undefined branch — it vanishes from the report. For a tool whose whole job is surfacing anomalies, an unparseable trialEndsAt is itself worth flagging. A Number.isNaN(parsed) guard treating unparseable as suspicious closes the gap. Low risk in practice (our writes always use .toISOString()).

2. In-memory fixture is now behind the real provider contract

initDynamoDbSubscriptionRead (and composed initDynamoDbSubscriptionProviders) implement ListAllSubscriptionRows, but the test double initInMemorySubscriptionProviders does not. Nothing breaks today (reconcile is pure and takes appRows directly), but the fixture no longer mirrors the real surface, so a future reconcile-style integration test can't drive the scan path through the double. Cheap to add while fresh.

3. Reconcile joins only on `subscriptionId` — `customerId` is an available secondary key

Both SubscriptionRecord and StripeSubscriptionSummary carry customerId, but reconcile() correlates the two sides exclusively via subscriptionId. That's fine for v1, but it's exactly the H2 "row-key drift" case the PR wants to distinguish. Worth considering for the follow-up fix PR — no change needed here.

FagnerMartinsBrack · 2026-07-07T11:26:24Z

@claude Resolve conflicts. Use your Read tool to read the local file .github/workflows/claude-PR-conflict-fixer.md for instructions. The file is in the checked-out repository.

Context

Repo: Readplace/readplace.com
PR: fix(stripe): read-only reconciliation CLI + ERROR-level logging for dropped entitlement webhooks #938
Branch: fix/stripe-reconciliation

claude · 2026-07-07T11:27:07Z

Claude finished @FagnerMartinsBrack's task in 16m 58s —— View job

Conflicts resolved ✅

Rebased onto current origin/main (6ebaa1f0) and force-pushed with --force-with-lease. The branch is now a clean linear history — two commits on top of main (ea2d373b + the empty CI-retry cec20d95) — and cleanly mergeable into main with no remaining conflicts (verified via merge-tree).

Read conflict-fixer instructions
Read PR commit history to understand intent
Rebase onto origin/main (6ebaa1f0)
Resolve conflicts (3 files)
Verify — full pnpm check green
Force-push with --force-with-lease
Confirm branch is conflict-free against main

What conflicted & how it was resolved

All three conflicts were additive collisions with #940 (self-serve account deletion), which merged into main and added a sibling deleteCustomer / DeleteCustomer at the exact spots this PR added listAllSubscriptions / ListAllStripeSubscriptions. In every case I kept both sides — no Stripe logic altered, no account-deletion logic dropped:

File	`main` (#940) added	This PR added	Resolution
`provider-contracts/src/stripe-subscriptions.ts`	`DeleteCustomer` type	`STRIPE_SUBSCRIPTION_STATUSES` + summary types + `ListAllStripeSubscriptions`	Kept both
`stripe-subscriptions/stripe-subscriptions.ts`	`deleteCustomer` impl (needed `import type`)	`listAllSubscriptions` impl (needed the value `STRIPE_SUBSCRIPTION_STATUSES`)	Merged the import to `import { STRIPE_SUBSCRIPTION_STATUSES, type … DeleteCustomer, type ListAllStripeSubscriptions … }`; kept both impls + both return-object keys
`dynamodb-subscription-providers.test.ts`	`deleteSubscription` describe	`listAllSubscriptionRows` describe (3 tests)	git had tangled the two blocks on their identical `createFakeClient`/`initDynamoDbSubscriptionProviders` scaffolding. Took main's clean file and re-inserted this PR's block byte-identical to `a79b52d3` (diff-verified; 0 lines dropped from main)

Verification

Ran the full pnpm check (nx run-many --target=check --all, env from .envrc as CI provides):

Successfully ran check for 37 projects / 48 tasks — 0 failures
hutch: lint + unused-css + knip clean; 2998 unit + 11 integration + 9 E2E pass; coverage statements 99.49%, branches 97.09%, functions 100%, lines 99.49% (all thresholds met)
The firefox-extension reader-link E2E that was flaky on attempt Add reading list view with URL filtering to popup #2 passed here too (100% coverage)

No Stripe logic was touched — the resolution is purely a re-integration of the two independent additions.

_{· branch fix/stripe-reconciliation}

FagnerMartinsBrack · 2026-07-07T11:50:14Z

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

Repo: Readplace/readplace.com
PR: fix(stripe): read-only reconciliation CLI + ERROR-level logging for dropped entitlement webhooks #938
Commit: cec20d9

claude · 2026-07-07T11:50:51Z

Claude finished @FagnerMartinsBrack's task in 5m 20s —— View job

Code Review — PR #938 (Stripe reconciliation CLI + ERROR-level dropped-webhook logging)

I re-read the full diff against origin/main at HEAD cec20d95, re-traced reconcile() against the PR's own H1/H2 evidence, and re-checked pagination, composition wiring, the **/*.main.ts coverage exclusion, knip/dead-exports, and the contract additions. cec20d95 is an empty CI-retry commit (0 files changed), so the reviewable code is byte-identical to a79b52d3 — now cleanly re-integrated on top of #940 (deleteCustomer) by the last rebase. This remains a clean, well-scoped, genuinely read-only diagnostic PR. No blocking issues.

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Low Priority Suggestions

(All three carried over from the reviews at 39ba2d18/7cbdce27; the code is unchanged, so they still stand — all non-blocking and appropriate to defer to the follow-up fix PR this report feeds.)

1. `Date.parse(trialEndsAt)` NaN is silently swallowed

reconcile.ts:110-111 — trialEndsAt is validated only as dynamoField(z.string()) (not .datetime()), so a non-ISO value passes the row schema. Date.parse("garbage") → NaN, and NaN < now → false, so such a row is neither flagged as past-end nor caught by the === undefined branch — it drops out of trialingRowsPastTrialEnd. It would still surface under rowsMissingSubscriptionId if it has no sub id, but the specific "stuck trial" bucket misses it. For a tool whose whole job is surfacing anomalies, an unparseable trialEndsAt is itself worth flagging. A Number.isNaN(parsed) guard treating unparseable as suspicious closes the gap. Low risk in practice (our writes always use .toISOString()). Fix this →

2. In-memory fixture is now behind the real provider contract

initDynamoDbSubscriptionRead (and composed initDynamoDbSubscriptionProviders) implement ListAllSubscriptionRows, but the test double initInMemorySubscriptionProviders does not. Nothing breaks today (reconcile is pure and takes appRows directly, and ListAllSubscriptionRows is a standalone type the fixture isn't structurally forced to satisfy), but the fixture no longer mirrors the real read surface, so a future reconcile-style integration test can't drive the scan path through the double. Cheap to add while fresh. Fix this →

3. Reconcile joins only on `subscriptionId` — `customerId` is an available secondary key

Both SubscriptionRecord and StripeSubscriptionSummary carry customerId, but reconcile() correlates the two sides exclusively via subscriptionId. That's fine for v1, but it's exactly the H2 "row-key drift" case the PR wants to distinguish: when a Stripe sub lands in stripeSubsMissingAppRow and an app row with a different subscriptionId shares its customerId, a customerId cross-check could classify the drift automatically instead of relying on manual masked-email correlation. Worth considering for the follow-up fix PR — no change needed here.

What's good (independently re-verified this pass):

Genuinely read-only. The CLI (stripe-reconcile.cli.main.ts) is a thin composition root that mirrors send-checkout-recovery-emails.cli.main.ts exactly and wires only the read half (initDynamoDbSubscriptionRead + Stripe list/read) — no writer is even constructed, so "zero writes" holds behaviourally, not just by intent. Coverage-excluded via the pre-existing **/*.main.ts pattern in enforce-coverage.config.base.js.
reconcile() is pure and exhaustively unit-tested — masking (normal, no-@, empty-local, empty-domain), every finding bucket, the active-without-subscriptionId double-classification, and trialing past/future/absent end. Findings are mutually exclusive where it matters and intentionally overlap where documented, with tests pinning both.
It would detect its own founding bug. A pending_cancellation/active app row whose Stripe sub is already canceled surfaces in liveAppRowsMissingLiveStripeSub — precisely the dropped-webhook consequence this PR exists to catch. LIVE_STRIPE_STATUSES correctly excludes only the two terminal-dead statuses (canceled, incomplete_expired).
No PII leak. Raw emails never enter the findings object; only maskEmail() output does, enforced by a test asserting no raw @-local reaches any report line.
Pagination is correct on both sides. Stripe listAllSubscriptions rebuilds URLSearchParams per page (no expand[] accumulation), pins the API version, caps limit=100, uses status=all + expand[]=data.customer, and asserts against an empty-page-with-has_more infinite loop; the DynamoDB scan drives ExclusiveStartKey/LastEvaluatedKey to exhaustion.
WARN→ERROR is well-justified — the 200-ack means Stripe never retries and the Lambda-Errors alarm never fires, so a dropped entitlement event is action-required, not informational. customer is parsed defensively with safeParse against the .passthrough() event shape, logging only subscriptionId/customerId/eventType (no PII). Adding members to the spread composition roots (app.ts) is a safe widening; knip is clean (both new symbols are consumed by the CLI).

cc @FagnerMartinsBrack - No high or medium priority issues found. This PR is ready for human review.
· branch fix/stripe-reconciliation

FagnerMartinsBrack · 2026-07-07T11:56:30Z

Code Review — PR #938 (Stripe reconciliation CLI + ERROR-level dropped-webhook logging)

I re-read the full diff against origin/main at HEAD cec20d95, re-traced reconcile() against the PR's own H1/H2 evidence, and re-checked pagination, composition wiring, the **/*.main.ts coverage exclusion, knip/dead-exports, and the contract additions. cec20d95 is an empty CI-retry commit (0 files changed), so the reviewable code is byte-identical to a79b52d3 — now cleanly re-integrated on top of #940 (deleteCustomer) by the last rebase. This remains a clean, well-scoped, genuinely read-only diagnostic PR. No blocking issues.

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Low Priority Suggestions

(All three carried over from the reviews at 39ba2d18/7cbdce27; the code is unchanged, so they still stand — all non-blocking and appropriate to defer to the follow-up fix PR this report feeds.)

1. `Date.parse(trialEndsAt)` NaN is silently swallowed

reconcile.ts:110-111 — trialEndsAt is validated only as dynamoField(z.string()) (not .datetime()), so a non-ISO value passes the row schema. Date.parse("garbage") → NaN, and NaN < now → false, so such a row is neither flagged as past-end nor caught by the === undefined branch — it drops out of trialingRowsPastTrialEnd. It would still surface under rowsMissingSubscriptionId if it has no sub id, but the specific "stuck trial" bucket misses it. A Number.isNaN(parsed) guard treating unparseable as suspicious closes the gap. Low risk in practice (our writes always use .toISOString()).

2. In-memory fixture is now behind the real provider contract

initDynamoDbSubscriptionRead (and composed initDynamoDbSubscriptionProviders) implement ListAllSubscriptionRows, but the test double initInMemorySubscriptionProviders does not. Nothing breaks today (reconcile is pure and takes appRows directly, and ListAllSubscriptionRows is a standalone type the fixture isn't structurally forced to satisfy), but the fixture no longer mirrors the real read surface, so a future reconcile-style integration test can't drive the scan path through the double. Cheap to add while fresh.

3. Reconcile joins only on `subscriptionId` — `customerId` is an available secondary key

Both SubscriptionRecord and StripeSubscriptionSummary carry customerId, but reconcile() correlates the two sides exclusively via subscriptionId. That's fine for v1, but it's exactly the H2 "row-key drift" case the PR wants to distinguish: when a Stripe sub lands in stripeSubsMissingAppRow and an app row with a different subscriptionId shares its customerId, a customerId cross-check could classify the drift automatically instead of relying on manual masked-email correlation. Worth considering for the follow-up fix PR — no change needed here.

FagnerMartinsBrack · 2026-07-09T09:01:42Z

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

Repo: Readplace/readplace.com
PR: fix(stripe): read-only reconciliation CLI + ERROR-level logging for dropped entitlement webhooks #938
Commit: fa8c843

FagnerMartinsBrack · 2026-07-10T10:34:47Z

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

Repo: Readplace/readplace.com
PR: fix(stripe): read-only reconciliation CLI + ERROR-level logging for dropped entitlement webhooks #938
Commit: b331d8d

claude · 2026-07-10T10:35:24Z

Claude finished @FagnerMartinsBrack's task in 3m 50s —— View job

Code Review — PR #938 (Stripe reconciliation CLI + ERROR-level dropped-webhook logging)

I read the full diff against origin/main at HEAD b331d8dc, re-derived reconcile() against the PR's H1/H2 evidence, and independently checked both pagination paths, the composition root, the **/*.main.ts coverage exclusion, the contract additions, and the WARN→ERROR handler. b331d8dc is an empty no-op CI-retry commit (git show --stat lists zero files) sitting on top of 6d6f8f97, so the reviewable code is byte-identical to the state reviewed at cec20d95/a79b52d3/fa8c8439, now cleanly rebased onto the provider-agnostic-ports rename (#58c287c6). Still a clean, well-scoped, genuinely read-only diagnostic PR. No blocking issues.

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Low Priority Suggestions

(All three carry over from the reviews at 39ba2d18/7cbdce27/cec20d95/fa8c8439; the code is unchanged, so they still stand — all non-blocking and appropriate to defer to the follow-up fix PR this report feeds.)

1. `Date.parse(trialEndsAt)` NaN is silently swallowed

reconcile.ts:110-111 — trialEndsAt is validated only as dynamoField(z.string()) (not .datetime()), so a non-ISO value passes the row schema. Date.parse("garbage") → NaN, and NaN < now → false, so such a row is neither flagged as past-end nor caught by the === undefined branch — it drops out of trialingRowsPastTrialEnd (it would still surface under rowsMissingSubscriptionId if it also lacks a sub id, but the specific "stuck trial" bucket misses it). For a tool whose whole job is surfacing anomalies, an unparseable trialEndsAt is itself worth flagging. A Number.isNaN(parsed) guard treating unparseable as suspicious closes the gap. Low risk in practice (our writes always use .toISOString()). Fix this →

2. In-memory fixture is now behind the real provider contract

initDynamoDbSubscriptionRead implements listAllSubscriptionRows (and the composed initDynamoDbSubscriptionProviders re-exports it via spread — confirmed the composed test drives it), but the test double initInMemorySubscriptionProviders does not (confirmed absent this pass). Nothing breaks today (reconcile is pure and takes appRows directly, and ListAllSubscriptionRows is a standalone type the fixture isn't structurally forced to satisfy), but the fixture no longer mirrors the real read surface, so a future reconcile-style integration test can't drive the scan path through the double. Cheap to add while fresh. Fix this →

3. Reconcile joins only on `subscriptionId` — `customerId` is an available secondary key

Both SubscriptionRecord and StripeSubscriptionSummary carry customerId, but reconcile() correlates the two sides exclusively via subscriptionId. That's fine for v1, but it's exactly the H2 "row-key drift" case the PR wants to distinguish: when a Stripe sub lands in stripeSubsMissingAppRow and an app row with a different subscriptionId shares its customerId, a customerId cross-check could classify the drift automatically instead of relying on manual masked-email correlation. Worth considering for the follow-up fix PR — no change needed here.

What's good (independently re-verified this pass):

Genuinely read-only. stripe-reconcile.cli.main.ts mirrors send-checkout-recovery-emails.cli.main.ts and wires only the read half (initDynamoDbSubscriptionRead) plus Stripe's list path — it calls listAllSubscriptionRows() and listAllSubscriptions() and nothing else; no writer is constructed, so "zero writes" holds behaviourally. Coverage-excluded via the pre-existing **/*.main.ts pattern.
reconcile() is pure and exhaustively unit-tested — masking (normal, no-@, empty-local, empty-domain), every finding bucket, the active-without-subscriptionId double-classification, and trialing past/future/absent end. Findings are mutually exclusive where it matters and overlap only where documented, with tests pinning both.
It detects its own founding bug. An active/pending_cancellation app row whose Stripe sub is canceled/incomplete_expired surfaces in liveAppRowsMissingLiveStripeSub — precisely the dropped-webhook consequence this PR exists to catch. LIVE_STRIPE_STATUSES correctly excludes only the two terminal-dead statuses, and a pending_cancellation row with a still-live Stripe sub (the legitimate grace-period state) is correctly not flagged.
No PII leak. Raw emails never enter the findings object; only maskEmail() output does, enforced by a test asserting no raw @-local reaches any report line.
Pagination correct on both sides. Stripe listAllSubscriptions rebuilds URLSearchParams per page (no expand[] accumulation), pins the API version (2026-04-22.dahlia), caps limit=100, uses status=all (so canceled/incomplete_expired subs are included) + expand[]=data.customer, and asserts against an empty-page-with-has_more infinite loop; the DynamoDB listAllSubscriptionRows drives ExclusiveStartKey/lastEvaluatedKey to exhaustion against the real defineDynamoTable.scan shape. The pinned API version also fixes the status enum, so the strict z.enum parse won't spuriously abort.
WARN→ERROR is well-justified — the 200-ack means Stripe never retries and the Lambda-Errors alarm never fires, so a dropped entitlement event is action-required, not informational. customer is parsed defensively with safeParse against the untyped event object, logging only subscriptionId/customerId/eventType (no PII), with an 'unknown' fallback tested. knip stays clean — both new provider symbols are consumed by the CLI.

Note on CI history: the earlier red checks on this branch were the pre-existing crawl-bookmark-visual snapshot mismatch (#936, since fixed on main) and a flaky firefox reader-link E2E — both independent of this Stripe-only diff.

cc @FagnerMartinsBrack - No high or medium priority issues found. This PR is ready for human review.
· branch fix/stripe-reconciliation

FagnerMartinsBrack · 2026-07-10T10:39:32Z

Code Review — PR #938 (Stripe reconciliation CLI + ERROR-level dropped-webhook logging)

I read the full diff against origin/main at HEAD b331d8dc, re-derived reconcile() against the PR's H1/H2 evidence, and independently checked both pagination paths, the composition root, the **/*.main.ts coverage exclusion, the contract additions, and the WARN→ERROR handler. b331d8dc is an empty no-op CI-retry commit (git show --stat lists zero files) sitting on top of 6d6f8f97, so the reviewable code is byte-identical to the state reviewed at cec20d95/a79b52d3/fa8c8439, now cleanly rebased onto the provider-agnostic-ports rename (#58c287c6). Still a clean, well-scoped, genuinely read-only diagnostic PR. No blocking issues.

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Low Priority Suggestions

(All three carry over from the reviews at 39ba2d18/7cbdce27/cec20d95/fa8c8439; the code is unchanged, so they still stand — all non-blocking and appropriate to defer to the follow-up fix PR this report feeds.)

1. `Date.parse(trialEndsAt)` NaN is silently swallowed

reconcile.ts:110-111 — trialEndsAt is validated only as dynamoField(z.string()) (not .datetime()), so a non-ISO value passes the row schema. Date.parse("garbage") → NaN, and NaN < now → false, so such a row is neither flagged as past-end nor caught by the === undefined branch — it drops out of trialingRowsPastTrialEnd (it would still surface under rowsMissingSubscriptionId if it also lacks a sub id, but the specific "stuck trial" bucket misses it). For a tool whose whole job is surfacing anomalies, an unparseable trialEndsAt is itself worth flagging. A Number.isNaN(parsed) guard treating unparseable as suspicious closes the gap. Low risk in practice (our writes always use .toISOString()). Fix this →

2. In-memory fixture is now behind the real provider contract

initDynamoDbSubscriptionRead implements listAllSubscriptionRows (and the composed initDynamoDbSubscriptionProviders re-exports it via spread — confirmed the composed test drives it), but the test double initInMemorySubscriptionProviders does not (confirmed absent this pass). Nothing breaks today (reconcile is pure and takes appRows directly, and ListAllSubscriptionRows is a standalone type the fixture isn't structurally forced to satisfy), but the fixture no longer mirrors the real read surface, so a future reconcile-style integration test can't drive the scan path through the double. Cheap to add while fresh. Fix this →

3. Reconcile joins only on `subscriptionId` — `customerId` is an available secondary key

Both SubscriptionRecord and StripeSubscriptionSummary carry customerId, but reconcile() correlates the two sides exclusively via subscriptionId. That's fine for v1, but it's exactly the H2 "row-key drift" case the PR wants to distinguish: when a Stripe sub lands in stripeSubsMissingAppRow and an app row with a different subscriptionId shares its customerId, a customerId cross-check could classify the drift automatically instead of relying on manual masked-email correlation. Worth considering for the follow-up fix PR — no change needed here.

FagnerMartinsBrack · 2026-07-10T11:32:29Z

@claude Resolve conflicts. Use your Read tool to read the local file .github/workflows/claude-PR-conflict-fixer.md for instructions. The file is in the checked-out repository.

Context

Repo: Readplace/readplace.com
PR: fix(stripe): read-only reconciliation CLI + ERROR-level logging for dropped entitlement webhooks #938
Branch: fix/stripe-reconciliation

claude · 2026-07-10T11:33:02Z

Claude encountered an error —— View job

Resolving merge conflicts — PR #938

Read conflict-fixer instructions
Read PR commit history to understand intent
Fetch latest origin/main and check conflict state
Rebase onto origin/main and resolve conflicts
Verify with pnpm check
Force-push with --force-with-lease
· branch fix/stripe-reconciliation

claude · 2026-07-10T11:34:31Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

claude · 2026-07-10T11:35:25Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

claude · 2026-07-10T11:36:23Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

FagnerMartinsBrack · 2026-07-10T11:37:31Z

@claude Resolve conflicts. Use your Read tool to read the local file .github/workflows/claude-PR-conflict-fixer.md for instructions. The file is in the checked-out repository.

Context

Repo: Readplace/readplace.com
PR: fix(stripe): read-only reconciliation CLI + ERROR-level logging for dropped entitlement webhooks #938
Branch: fix/stripe-reconciliation

claude · 2026-07-10T11:38:04Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

claude · 2026-07-10T11:39:07Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

claude · 2026-07-10T11:40:04Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

claude · 2026-07-10T11:40:53Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

FagnerMartinsBrack · 2026-07-10T11:55:27Z

@claude Resolve conflicts. Use your Read tool to read the local file .github/workflows/claude-PR-conflict-fixer.md for instructions. The file is in the checked-out repository.

Context

Repo: Readplace/readplace.com
PR: fix(stripe): read-only reconciliation CLI + ERROR-level logging for dropped entitlement webhooks #938
Branch: fix/stripe-reconciliation

claude · 2026-07-10T11:56:06Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

claude · 2026-07-10T11:57:02Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

claude · 2026-07-10T11:57:58Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

claude · 2026-07-10T11:59:06Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

FagnerMartinsBrack · 2026-07-10T15:14:27Z

@claude Resolve conflicts. Use your Read tool to read the local file .github/workflows/claude-PR-conflict-fixer.md for instructions. The file is in the checked-out repository.

Context

Repo: Readplace/readplace.com
PR: fix(stripe): read-only reconciliation CLI + ERROR-level logging for dropped entitlement webhooks #938
Branch: fix/stripe-reconciliation

claude · 2026-07-10T15:15:06Z

Claude finished @FagnerMartinsBrack's task in 20m 43s —— View job

Conflicts resolved ✅

Rebased onto current origin/main (78dfa44b) and force-pushed with --force-with-lease. The branch is a clean linear history — three commits on top of main (8d971e2b + the preserved empty CI-retry 860a07a1 + one adaptation commit 0995a63e) — and cleanly mergeable into main (verified via merge-tree).

Read conflict-fixer instructions
Read PR commit history to understand intent
Rebase onto origin/main (78dfa44b)
Resolve the semantic break from main's inbox extraction (feat(inbox): extract the inbox into its own deployable (phases A–C: pre-refactors, seam, additive project) #950)
Verify — full pnpm check green (42 projects / 60 tasks)
Force-push with --force-with-lease
Confirm branch is conflict-free against main

What actually conflicted & how it was resolved

The prior conflict-fixer attempts errored on the agent-SDK startup crash (#852) and never resolved anything — so this was the first real pass. The rebase itself applied with no textual conflicts, but that hid a semantic break: main's inbox extraction (#950, 963345c2) moved the subscription read half out of hutch —

projects/hutch/…/subscription-providers/dynamodb-subscription-read.ts → @packages/subscription-access/src/dynamodb-subscription-read.ts

Git's rename detection correctly reapplied this PR's listAllSubscriptionRows addition onto the file at its new home (imports and all). But it left two loose ends that only surface at build/coverage time — both fixed in 0995a63e:

Loose end	Why git missed it	Resolution
The reconcile CLI still imported `initDynamoDbSubscriptionRead` from the now-deleted hutch-local path	The CLI is a new file this PR adds, so there was no main-side version to merge — git kept the stale relative import verbatim	Repointed to `@packages/subscription-access`, matching `app.ts` and the composed `dynamodb-subscription-providers` (and CLAUDE.md's no-cross-project-relative-imports rule)
`listAllSubscriptionRows` coverage lived only in hutch's composed test	Coverage is per-project; the function body moved to `@packages/subscription-access`, so its own suite no longer exercised it → 100%-functions threshold would fail	Added the matching unit tests (single page, `ExclusiveStartKey` pagination, empty table) to the package's own `dynamodb-subscription-read.test.ts`

No Stripe logic was altered — the resolution purely re-integrates the PR's additions with main's module move. The hutch composed test that drives listAllSubscriptionRows through the composition is left intact.

Verification

Ran the full pnpm check (env sourced from .envrc; pnpm install --frozen-lockfile first to relink the workspace after the rebase pulled in the new projects/inbox + relocated package):

Successfully ran check for 42 projects / 60 tasks — 0 failures
@packages/subscription-access: lint + knip clean; 100% statements / branches / functions / lines (the new tests restore full coverage of the relocated listAllSubscriptionRows)
hutch: lint + unused-css + knip clean; 2946 unit + 11 integration + 9 E2E pass; coverage statements 99.46%, branches 97.05%, functions 100%, lines 99.46% (all thresholds met)
The firefox reader-link E2E that was flaky on an earlier attempt passed here too
· branch fix/stripe-reconciliation

…ok logging Add a read-only Stripe-vs-DynamoDB reconciliation CLI (pnpm --dir projects/hutch stripe-reconcile) that pages through Stripe subscriptions and a full subscription-table scan, then reports five categories of drift via a pure, fully-tested reconcile() domain function. Customer emails are masked before they enter any finding. Also upgrade the customer.subscription.deleted "no subscription row found" log from WARN to a structured ERROR carrying subscriptionId, customerId, and eventType. The handler returns 200 so Stripe never retries and the Lambda-Errors alarm never fires — a dropped entitlement event is action-required, which is ERROR, not informational. Claude-Session: https://claude.ai/code/session_01En2uUiiwQoRQKWenCnkPi9

No-op commit to re-trigger CI. The failure was a flaky firefox-extension reader-link E2E timeout (30s waiting for the login #email field), not a defect in this Stripe-only diff: - The sibling "OAuth login flow" E2E passed in the same CI run using the same #email field, so the login page renders fine in this build. - The reader-link E2E passes locally in ~10s and nx auto-flagged firefox-extension:check as a flaky task (three separate local runs). - This PR changes only Stripe files; the one server-path change adds listAllSubscriptionRows, called exclusively by the reconcile CLI, never in the login/reader request path. Full `pnpm check` is green locally (37 projects, 48 tasks). No API permission to re-run the workflow, so re-triggering CI with this commit. Co-authored-by: Fayner Brack <FagnerMartinsBrack@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

… read The rebase onto main picked up the inbox extraction (#950, 963345c), which moved initDynamoDbSubscriptionRead out of hutch into @packages/subscription-access. Git's rename detection reapplied this PR's listAllSubscriptionRows addition onto the relocated file, but left two loose ends the rebase could not see: - The reconcile CLI still imported initDynamoDbSubscriptionRead from the now-deleted hutch-local path. Repoint it to @packages/subscription-access, matching app.ts and the composed dynamodb-subscription-providers. - listAllSubscriptionRows coverage lived only in hutch's composed test, so the relocated function body was uncovered by its new package's own suite. Add the matching unit tests (single page, ExclusiveStartKey pagination, empty table) to subscription-access — restoring 100% coverage there. No Stripe logic changed; this only re-integrates the PR's additions with main's module move. Full `pnpm check` green (42 projects / 60 tasks). Co-authored-by: Fayner Brack <FagnerMartinsBrack@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

FagnerMartinsBrack · 2026-07-10T15:43:18Z

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

Repo: Readplace/readplace.com
PR: fix(stripe): read-only reconciliation CLI + ERROR-level logging for dropped entitlement webhooks #938
Commit: 0995a63

claude · 2026-07-10T15:43:53Z

Claude finished @FagnerMartinsBrack's task in 6m 3s —— View job

Code Review — PR #938 (Stripe reconciliation CLI + ERROR-level dropped-webhook logging)

Read code-reviewer instructions
Identify what 0995a63e changed vs. the last reviewed state
Review the full diff against origin/main
Trace the relocated listAllSubscriptionRows + CLI import repoint
Check coverage/knip/contract implications
Post review

Unlike the prior CI-retry commits, 0995a63e is a real adaptation commit (not a no-op) — it re-integrates this PR with main's inbox extraction (#950), which moved initDynamoDbSubscriptionRead from hutch into @packages/subscription-access. I reviewed that adaptation independently.

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Low Priority Suggestions

1. `Date.parse(trialEndsAt)` NaN is silently swallowed

reconcile.ts:110-111 — trialEndsAt is validated only as dynamoField(z.string()) (not .datetime()), so a non-ISO value passes the row schema. Date.parse("garbage") → NaN, and NaN < now → false, so such a row is neither flagged as past-end nor caught by the === undefined branch — it drops out of trialingRowsPastTrialEnd (it would still surface under rowsMissingSubscriptionId if it also lacks a sub id, but the specific "stuck trial" bucket misses it). A Number.isNaN(parsed) guard treating unparseable as suspicious closes the gap. Low risk in practice (our writes always use .toISOString()). Fix this →

2. In-memory fixture is now behind the real provider contract

initDynamoDbSubscriptionRead implements listAllSubscriptionRows, but the test double initInMemorySubscriptionProviders does not (confirmed absent this pass — it exposes only findByUserId/findBySubscriptionId + writes). Nothing breaks today (reconcile is pure and takes appRows directly), but the fixture no longer mirrors the real read surface. This is slightly more pertinent post-relocation: the read now lives in @packages/subscription-access, which already carries @packages/test-fixtures as a devDependency, so a package-level reconcile integration test would want the double to drive the scan path. Cheap to add while fresh. Fix this →

3. Reconcile joins only on `subscriptionId` — `customerId` is an available secondary key

Both SubscriptionRecord and StripeSubscriptionSummary carry customerId, but reconcile() correlates the two sides exclusively via subscriptionId. That's fine for v1, but it's exactly the H2 "row-key drift" case the PR wants to distinguish: when a Stripe sub lands in stripeSubsMissingAppRow and an app row with a different subscriptionId shares its customerId, a customerId cross-check could classify the drift automatically instead of relying on manual masked-email correlation. Worth considering for the follow-up fix PR — no change needed here.

4. `listAllSubscriptionRows` is now double-tested (new, from the relocation)

Post-relocation, listAllSubscriptionRows is exercised in two suites with near-identical single-page / ExclusiveStartKey-pagination / empty-table cases: the package's own dynamodb-subscription-read.test.ts (added in 0995a63e) and hutch's composed dynamodb-subscription-providers.test.ts:473-567. The package test is the necessary one — the function body lives there now, so the package's 100%-functions threshold depends on it. The composed block is redundant for coverage (the body is a dependency hutch doesn't instrument, and the composition is already covered by the findByUserId/findBySubscriptionId tests) and now only re-verifies that initDynamoDbSubscriptionProviders re-exports the method. Harmless, but a human may want to thin the composed block to a single wiring assertion. Fix this →

The 0995a63e adaptation is correct — independently verified:

CLI import repoint is sound. stripe-reconcile.cli.main.ts:4 now imports initDynamoDbSubscriptionRead from @packages/subscription-access; the package's index.ts exports exactly that symbol, and it matches how app.ts and the composed dynamodb-subscription-providers.ts import it. The now-deleted hutch-local path is gone (find finds no leftover), so this also satisfies CLAUDE.md's no-cross-project-relative-imports rule.
The new package tests exercise the real adapter, not a stub. The fake client returns the raw SDK shape (Items / LastEvaluatedKey); defineDynamoTable.scan (define-table.ts:148-157) is what maps those to items / lastEvaluatedKey and parses each row through SubscriptionProviderRow. So the test drives the genuine mapping the production code depends on. The pagination case correctly walks two pages and asserts receivedKeys = [undefined, { userId: "u-1" }], covering the do…while (lastEvaluatedKey !== undefined) loop and restoring the package's own 100% function coverage now that the body lives there.
Contract matches impl. ListAllSubscriptionRows = () => Promise<SubscriptionRecord[]> (subscription-providers.ts:69) lines up with the typed impl.

What's good (core PR, re-confirmed): genuinely read-only (the CLI constructs no writer); reconcile() is pure and exhaustively unit-tested (masking incl. no-@/empty-local/empty-domain, every finding bucket, the active-without-sub double-classification, trialing past/future/absent); it detects its own founding bug (an active/pending_cancellation row whose Stripe sub is canceled/incomplete_expired surfaces in liveAppRowsMissingLiveStripeSub, and LIVE_STRIPE_STATUSES excludes only the two terminal-dead statuses); no PII leak (only maskEmail() output enters findings, test-enforced); Stripe pagination rebuilds URLSearchParams per page, pins the API version, and asserts against an empty-page infinite loop; WARN→ERROR is well-justified (200-ack ⇒ no Stripe retry, no Lambda-Errors alarm) and logs only subscriptionId/customerId/eventType.

cc @FagnerMartinsBrack - No high or medium priority issues found. This PR is ready for human review.
· branch fix/stripe-reconciliation

FagnerMartinsBrack · 2026-07-10T15:50:12Z

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Low Priority Suggestions

1. `Date.parse(trialEndsAt)` NaN is silently swallowed

reconcile.ts:110-111 — trialEndsAt is validated only as dynamoField(z.string()) (not .datetime()), so a non-ISO value passes the row schema. Date.parse("garbage") → NaN, and NaN < now → false, so such a row is neither flagged as past-end nor caught by the === undefined branch — it drops out of trialingRowsPastTrialEnd (it would still surface under rowsMissingSubscriptionId if it also lacks a sub id, but the specific "stuck trial" bucket misses it). A Number.isNaN(parsed) guard treating unparseable as suspicious closes the gap. Low risk in practice (our writes always use .toISOString()). Fix this →

2. In-memory fixture is now behind the real provider contract

initDynamoDbSubscriptionRead implements listAllSubscriptionRows, but the test double initInMemorySubscriptionProviders does not (confirmed absent this pass — it exposes only findByUserId/findBySubscriptionId + writes). Nothing breaks today (reconcile is pure and takes appRows directly), but the fixture no longer mirrors the real read surface. This is slightly more pertinent post-relocation: the read now lives in @packages/subscription-access, which already carries @packages/test-fixtures as a devDependency, so a package-level reconcile integration test would want the double to drive the scan path. Cheap to add while fresh. Fix this →

3. Reconcile joins only on `subscriptionId` — `customerId` is an available secondary key

Both SubscriptionRecord and StripeSubscriptionSummary carry customerId, but reconcile() correlates the two sides exclusively via subscriptionId. That's fine for v1, but it's exactly the H2 "row-key drift" case the PR wants to distinguish: when a Stripe sub lands in stripeSubsMissingAppRow and an app row with a different subscriptionId shares its customerId, a customerId cross-check could classify the drift automatically instead of relying on manual masked-email correlation. Worth considering for the follow-up fix PR — no change needed here.

4. `listAllSubscriptionRows` is now double-tested (new, from the relocation)

Post-relocation, listAllSubscriptionRows is exercised in two suites with near-identical single-page / ExclusiveStartKey-pagination / empty-table cases: the package's own dynamodb-subscription-read.test.ts (added in 0995a63e) and hutch's composed dynamodb-subscription-providers.test.ts:473-567. The package test is the necessary one — the function body lives there now, so the package's 100%-functions threshold depends on it. The composed block is redundant for coverage (the body is a dependency hutch doesn't instrument, and the composition is already covered by the findByUserId/findBySubscriptionId tests) and now only re-verifies that initDynamoDbSubscriptionProviders re-exports the method. Harmless, but a human may want to thin the composed block to a single wiring assertion. Fix this →

FagnerMartinsBrack had a problem deploying to staging July 5, 2026 14:36 — with GitHub Actions Failure

FagnerMartinsBrack added the ci-fix-attempt-1 label Jul 5, 2026

FagnerMartinsBrack temporarily deployed to staging July 6, 2026 00:24 — with GitHub Actions Inactive

FagnerMartinsBrack force-pushed the fix/stripe-reconciliation branch from 39ba2d1 to a79b52d Compare July 6, 2026 23:59

FagnerMartinsBrack had a problem deploying to staging July 6, 2026 23:59 — with GitHub Actions Failure

FagnerMartinsBrack added ci-fix-attempt-2 and removed ci-fix-attempt-1 labels Jul 7, 2026

FagnerMartinsBrack temporarily deployed to staging July 7, 2026 00:27 — with GitHub Actions Inactive

FagnerMartinsBrack force-pushed the fix/stripe-reconciliation branch from 7cbdce2 to cec20d9 Compare July 7, 2026 11:43

FagnerMartinsBrack temporarily deployed to staging July 7, 2026 11:43 — with GitHub Actions Inactive

FagnerMartinsBrack force-pushed the fix/stripe-reconciliation branch from cec20d9 to fa8c843 Compare July 9, 2026 08:53

FagnerMartinsBrack temporarily deployed to staging July 9, 2026 08:53 — with GitHub Actions Inactive

FagnerMartinsBrack force-pushed the fix/stripe-reconciliation branch from fa8c843 to b331d8d Compare July 10, 2026 10:27

FagnerMartinsBrack temporarily deployed to staging July 10, 2026 10:27 — with GitHub Actions Inactive

FagnerMartinsBrack and others added 3 commits July 10, 2026 15:15

FagnerMartinsBrack force-pushed the fix/stripe-reconciliation branch from b331d8d to 0995a63 Compare July 10, 2026 15:35

FagnerMartinsBrack temporarily deployed to staging July 10, 2026 15:35 — with GitHub Actions Inactive

Uh oh!

Conversation

FagnerMartinsBrack commented Jul 5, 2026

fix(stripe): read-only reconciliation CLI + ERROR-level logging for dropped entitlement webhooks

Hypothesis

Evidence (30d, Jun 4 – Jul 4 2026)

Investigation narrative: the dropped webhook

What this changes

What this deliberately does NOT change

Estimated result

How to measure

Merge note

Uh oh!

FagnerMartinsBrack commented Jul 5, 2026

Context

Failed Jobs

Uh oh!

claude Bot commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI Failure Analysis — Attempt 1 of 5

Conclusion: this failure is pre-existing on main and unrelated to this PR — no Stripe-side change can fix it, so I have not modified the PR.

Evidence it is not this PR

Why I did not change this PR

Recommended fix (on main, not this PR)

Uh oh!

FagnerMartinsBrack commented Jul 6, 2026

Context

Uh oh!

claude Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review — PR #938 (Stripe reconciliation CLI + ERROR-level dropped-webhook logging)

High Priority Issues (Must Fix)

Medium Priority Issues

Low Priority Suggestions

1. Date.parse(trialEndsAt) NaN is silently swallowed — a malformed date escapes every finding

2. In-memory fixture is now behind the real provider contract

3. Reconcile joins only on subscriptionId — customerId is an available secondary key

Uh oh!

FagnerMartinsBrack commented Jul 6, 2026

Code Review — PR #938 (Stripe reconciliation CLI + ERROR-level dropped-webhook logging)

High Priority Issues (Must Fix)

Medium Priority Issues

Low Priority Suggestions

1. Date.parse(trialEndsAt) NaN is silently swallowed — a malformed date escapes every finding

2. In-memory fixture is now behind the real provider contract

3. Reconcile joins only on subscriptionId — customerId is an available secondary key

Uh oh!

FagnerMartinsBrack commented Jul 6, 2026

Context

Uh oh!

claude Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts resolved ✅

What conflicted & how it was resolved

Verification

Uh oh!

FagnerMartinsBrack commented Jul 7, 2026

Context

Failed Jobs

Uh oh!

claude Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI Failure Analysis — Attempt 2 of 5 → ✅ CI now GREEN

Conclusion: the failure was a flaky firefox E2E timeout, unrelated to this Stripe PR

Action taken

Result

Uh oh!

FagnerMartinsBrack commented Jul 7, 2026

Context

Uh oh!

claude Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review — PR #938 (Stripe reconciliation CLI + ERROR-level dropped-webhook logging)

High Priority Issues (Must Fix)

Medium Priority Issues

Low Priority Suggestions

1. Date.parse(trialEndsAt) NaN is silently swallowed

2. In-memory fixture is now behind the real provider contract

3. Reconcile joins only on subscriptionId — customerId is an available secondary key

Uh oh!

claude Bot commented Jul 5, 2026 •

edited

Loading

Conclusion: this failure is pre-existing on `main` and unrelated to this PR — no Stripe-side change can fix it, so I have not modified the PR.

Recommended fix (on `main`, not this PR)

claude Bot commented Jul 6, 2026 •

edited

Loading

1. `Date.parse(trialEndsAt)` NaN is silently swallowed — a malformed date escapes every finding

3. Reconcile joins only on `subscriptionId` — `customerId` is an available secondary key

1. `Date.parse(trialEndsAt)` NaN is silently swallowed — a malformed date escapes every finding

3. Reconcile joins only on `subscriptionId` — `customerId` is an available secondary key

claude Bot commented Jul 6, 2026 •

edited

Loading

claude Bot commented Jul 7, 2026 •

edited

Loading

claude Bot commented Jul 7, 2026 •

edited

Loading

1. `Date.parse(trialEndsAt)` NaN is silently swallowed

3. Reconcile joins only on `subscriptionId` — `customerId` is an available secondary key

1. `Date.parse(trialEndsAt)` NaN is silently swallowed

3. Reconcile joins only on `subscriptionId` — `customerId` is an available secondary key

claude Bot commented Jul 7, 2026 •

edited

Loading

claude Bot commented Jul 7, 2026 •

edited

Loading

1. `Date.parse(trialEndsAt)` NaN is silently swallowed

3. Reconcile joins only on `subscriptionId` — `customerId` is an available secondary key

1. `Date.parse(trialEndsAt)` NaN is silently swallowed

3. Reconcile joins only on `subscriptionId` — `customerId` is an available secondary key

claude Bot commented Jul 10, 2026 •

edited

Loading

1. `Date.parse(trialEndsAt)` NaN is silently swallowed

3. Reconcile joins only on `subscriptionId` — `customerId` is an available secondary key

1. `Date.parse(trialEndsAt)` NaN is silently swallowed