Skip to content

test(ui): accept a final invoice Stripe already finalized - #7378

Merged
otavio merged 2 commits into
masterfrom
test/ui/billing-final-invoice-paid
Oct 8, 2026
Merged

otavio merged 2 commits into
masterfrom
test/ui/billing-final-invoice-paid

Conversation

@luizhf42

@luizhf42 luizhf42 commented Oct 7, 2026

Copy link
Copy Markdown
Member

Summary

Since 2026-10-07 around 18:00 UTC, the cloud e2e job fails on master and on PRs in billing.spec.ts:406, "cancelling at the end of the period is not undone when the final invoice is paid". Stripe now finalizes and pays the final invoice during the test-clock advance. The test expected to find it as a draft and finalize it itself.

 advance the test clock past the end of the period
 wait for the clock to be ready, then for the subscription to be canceled
-list draft invoices; throw if there are none
-finalize the draft
+list draft invoices
+finalize the draft only if there is one
 wait for the server to log that it abandoned the final invoice's invoice.paid

The poll for that log line still fails the test if invoice.paid never arrives, whether Stripe or the test finalizes the invoice.

Evidence

  • Before: the cloud e2e job fails with expected a draft final invoice for sub_… on master 9a15de8a2 (job) and twice on test(ui): cover licensing and the admin panel end to end #7369. The same test passed at 16:20 on 2ec6590dc, and no billing code changed in shellhub or cloud since.
    In the failed run, Stripe sent invoice.updated, invoice.finalized and invoice.paid five seconds after creating the final invoice, before test_clock.ready, and the server abandoned that invoice.paid as it should.
  • After: the cloud e2e job on this PR. It can't run locally without Stripe keys.

The cause is on Stripe's side and still unknown. Neither the server nor the pinned stripe-cli finalizes invoices, and Stripe's changelog doesn't mention a change. The previous_attributes on evt_1UO0OGDjjCj7k46jqqSmjE3A (invoice.updated) in the Stripe test dashboard should show what changed.

Merge Danger

Door: two-way

Blast Radius: e2e

Only the cloud billing e2e spec changes.

@luizhf42
luizhf42 requested a review from a team as a code owner October 7, 2026 19:51
@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Claude finished @luizhf42's task in 8m 3s —— View job


Code Review Complete

Reviewed 1 file. 1 inline issue posted. Standards: 0 violations, 0 judgement calls. Spec: 0. Correctness: 1.

Standards

Nothing found. The new name finalizeInvoiceIfDraft says that the function may do nothing, and the commit message records why Stripe's behaviour forced the change. The diff adds no comments, and the test still ends on what the server did.

Correctness

When Stripe pays the final invoice during the clock advance, invoice.paid can now reach the server before customer.subscription.deleted. The test can then time out waiting for the abandon line (billing.spec.ts:430, handler at cloud/internal/billing/services/handles.go:125). This makes the test flaky. It cannot make it pass falsely: the only place that logs the line is the invoice.paid handler, and only for this subscription.

Spec

The Spec axis did not run because the PR and its commit reference no issue.


To request another review round, comment /review.

@luizhf42 luizhf42 self-assigned this Oct 7, 2026
Comment thread ui/apps/console/e2e/billing.spec.ts Outdated
@luizhf42
luizhf42 added this pull request to stack #7386 October 7, 2026 20:30
@luizhf42

luizhf42 commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

/review

@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Claude finished @luizhf42's task in 6m 52s —— View job


Code Review Complete

Reviewed 1 file. 1 inline issue posted. Standards: 0 violations, 1 judgement call. Spec: 0. Correctness: 0.

Standards

The one judgement call is in sendFinalInvoicePaid at ui/apps/console/e2e/billing.spec.ts:129-136. It looks for the invoice.paid event among the newest 100 across the whole shared Stripe test account. If concurrent cloud e2e runs push the event off that page, the test fails with a message that points at the wrong cause. This can make the test flaky but cannot make it pass falsely. No rule was broken. The diff adds no comments, and both commit messages explain the Stripe behaviour and the ordering issue behind the workaround.

Correctness

The previous review's ordering finding is fixed. When Stripe has already paid the final invoice, stripe events resend now delivers its invoice.paid after the test has seen canceled, which is the same point where the draft path finalizes. The webhook does not skip event IDs it has already seen, so the resent event reaches the abandon log line (cloud/internal/billing/services/handles.go:125-131). I found no new defects. Because the test clock stamps invoices with its frozen time, limit=1 returns the period-end invoice and not the subscribe-time invoice.

Spec

The Spec axis did not run because neither the PR nor its commits reference an issue.


To request another review round, comment /review.

Comment thread ui/apps/console/e2e/billing.spec.ts
@luizhf42
luizhf42 force-pushed the test/ui/billing-final-invoice-paid branch from 7d63b13 to 9854d22 Compare October 7, 2026 20:43
@luizhf42

luizhf42 commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

/review

@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Claude finished @luizhf42's task in 6m 30s —— View job


Code Review Complete

Reviewed 1 file. 1 inline issue posted. Standards: 0 violations, 1 judgement call. Spec: 0. Correctness: 0.

Standards

The one judgement call is in sendFinalInvoicePaid at ui/apps/console/e2e/billing.spec.ts:122-124. limit=1 takes the newest invoice of any status and nothing checks that it is the period-end one. If Stripe made no final invoice, the helper would resend the invoice.paid from subscribing. The server logs the abandon line for that one too, so the test would pass without a final invoice. The fix is to compare invoice.created with periodEnd. The diff adds no comments, and both commit messages explain the workaround.

Correctness

Nothing new found. Both earlier findings, the delivery order and the created[gte] filter, are fixed. The server does not deduplicate event IDs, so the resent event is handled again. stripe listen in cloud/docker-compose.yml has no --events filter. Every value passed to composeExec is a Stripe ID or a number.

Spec

The Spec axis did not run because neither the PR nor its commits reference an issue.


To request another review round, comment /review.

Comment thread ui/apps/console/e2e/billing.spec.ts Outdated
@luizhf42
luizhf42 force-pushed the test/ui/billing-final-invoice-paid branch from 9854d22 to 0efcf28 Compare October 7, 2026 21:05
@luizhf42

luizhf42 commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

/review

@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

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


Code Review Complete

Reviewed 1 file on two axes: Standards (repo rules, language conventions, tests, code smells) and Correctness (security and runtime defects). The Spec axis did not run because neither the PR nor its commits reference an issue. No new issues found. The code looks good as-is.

The period-end cancellation test listed the subscription's draft
invoices after the test clock read ready, and threw when there was
none. When the test was written, the clock read ready with the final
invoice still a draft, so the test finalized it to send `invoice.paid`.

Since 2026-10-07 around 18:00 UTC the invoice no longer waits. The
cloud e2e job failed this way on master (9a15de8) and on #7369 twice,
with no billing change in shellhub or cloud since its last pass at
16:20. In the failed run Stripe sent `invoice.updated`,
`invoice.finalized` and `invoice.paid` five seconds after creating the
invoice, before `test_clock.ready`, and the server abandoned that
`invoice.paid` as it should. Neither the server nor the pinned
stripe-cli finalizes invoices, and Stripe's changelog does not mention
it, so the cause is on Stripe's side and still unknown.

The test now finalizes the invoice only when it is still a draft. The
poll for the server's abandon line still fails the test when no
`invoice.paid` arrives either way.
@gustavosbarreto
gustavosbarreto force-pushed the test/ui/billing-final-invoice-paid branch from 0efcf28 to df75420 Compare October 7, 2026 21:37
@otavio

otavio commented Oct 8, 2026

Copy link
Copy Markdown
Member

/review

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Claude finished @otavio's task in 2m 44s —— View job


Code Review Complete

Reviewed 1 file. 1 inline issue posted. Standards: 1 violation, 0 judgement calls. Spec: 0. Correctness: 0.

Standards

The violation is at ui/apps/console/e2e/billing.spec.ts:126 (same issue at :141 and :438), under code-style.md. When 9854d22 and 0efcf28 were squashed into df75420, their reasons were dropped from the commit messages. Those reasons are why the periodEnd guard depends on invoice created following the test clock, and why the created[gte] events filter uses wall-clock startedAt. They now live only in replies on this PR's review threads. Fixing this means rewording the commit message. The code doesn't need to change.

Spec

The Spec axis did not run because neither the PR nor its commits reference an issue.

Additional notes

I left out three things. The 100-event page limit was already raised and fixed in an earlier thread. The other two were possible flakes that I couldn't back with a concrete failing path. One is an invoice still open when the helper runs. The other is the abandon-line poll matching the first delivery. Both would make the test fail, not pass when it shouldn't.


To request another review round, comment /review.

Comment thread ui/apps/console/e2e/billing.spec.ts
@otavio
otavio force-pushed the test/ui/billing-final-invoice-paid branch from df75420 to a2a161b Compare October 8, 2026 00:34
@otavio

otavio commented Oct 8, 2026

Copy link
Copy Markdown
Member

/review

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Claude finished @otavio's task in 4m 27s —— View job


Code Review Complete

Reviewed 1 file. 1 inline issue posted. Standards: 0 violations, 1 judgement call. Spec: 0. Correctness: 0.

Standards

The one judgement call is at ui/apps/console/e2e/billing.spec.ts:150. The new stripeCli("events", "resend", paid.id) ignores what it returns, though every other mutating Stripe helper in the file checks its result. If Stripe returns an error body, the test fails later at the abandon-line poll, which blames the server. That gives a misleading failure but cannot make the test pass falsely. The earlier finding about the commit message is fixed: a2a161b records the newest-first listing, the test-clock created behind the periodEnd guard, and the wall-clock event created behind startedAt.

Correctness

I found nothing new. Every value passed to composeExec is a Stripe ID or a number. The created[gte] bound can't drop the target event. On the resend path, the abandon line can only appear after the poll has seen canceled.

Spec

The Spec axis did not run because neither the PR nor its commits reference an issue.

Additional notes

I left out two low-value judgement calls:

  • The got undefined wording in the guard's error message.
  • Math.floor(Date.now() / 1000) written out in two places.

To request another review round, comment /review.

Comment thread ui/apps/console/e2e/billing.spec.ts Outdated
When Stripe pays the final invoice during the clock advance, the test
no longer decides when `invoice.paid` reaches the server. The server
logs the abandon line only when the namespace is already `canceled`
locally, and the webhook's per-tenant lock serializes the two handlers
without ordering them. Stripe does not guarantee delivery order, so an
`invoice.paid` handled before `customer.subscription.deleted` would
never log it and the poll would time out.

The test now resends that `invoice.paid` with `stripe events resend`
after it has seen `canceled`. Without `--webhook-endpoint` the resend
goes to the running `stripe listen` session. A draft final invoice is
still finalized as before, which sends `invoice.paid` at the same point.

The final invoice comes from `invoices list` with `limit=1`, which
relies on Stripe listing a subscription's invoices newest first. The
`created < periodEnd` check then refuses the invoice from subscribing,
for the case where Stripe made no final invoice; it holds because an
invoice's `created` follows the customer's test clock.

The `invoice.paid` event is looked up with `created[gte]` set to when
the test started, so `invoice.paid` events from other runs on the
shared Stripe test account cannot push it off the 100-event page. The
bound is the wall-clock start, not `periodEnd`, because an event's
`created` follows the wall clock even for a test-clock customer.
@otavio
otavio force-pushed the test/ui/billing-final-invoice-paid branch from a2a161b to 429af9e Compare October 8, 2026 00:42
@otavio

otavio commented Oct 8, 2026

Copy link
Copy Markdown
Member

/review

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Claude finished @otavio's task in 2m 15s —— View job


Code Review Complete

Reviewed 1 file on two axes: Standards (repo rules, language conventions, tests, code smells) and Correctness (security and runtime defects). The Spec axis did not run because neither the PR nor its commits reference an issue. No new issues found. The code looks good as-is.

@otavio
otavio merged commit cc4fd83 into master Oct 8, 2026
29 checks passed
@otavio
otavio deleted the test/ui/billing-final-invoice-paid branch October 8, 2026 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants