Repository navigation
Conversation
|
Claude finished @otavio's task in 6m 26s —— View job Code Review CompleteReviewed 11 files. 2 inline issues posted. Standards: 0 violations, 2 judgement calls. Spec: not run. Correctness: 0. StandardsNo rule is broken. The more useful of the two judgement calls is the SpecThe Spec axis did not run. The only issue this PR references is shellhub-io/team#243, which is in another repository this run cannot read. Additional notes
To request another review round, comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
POST /api/billing/customer and POST /api/billing/subscription answer 201, as the cloud handlers always have, but the spec declared 200. The strict response validator the e2e stacks run does not catch it: kin-openapi passes a status an operation leaves undocumented unless the validator is built with IncludeResponseStatus, and ours is not. The webhook now says a request without a valid Stripe-Signature is answered with 400, which the matching shellhub-io/cloud fix makes true; it answered 500 before.
bf8ba2c to
c5b0f43
Compare
|
/review |
|
Claude finished @otavio's task in 4m 46s —— View job Code Review CompleteReviewed 11 files for Standards (repo rules, Go conventions, tests, code smells) and Correctness (security and runtime defects). No new issues found. The code looks good as-is. The Spec axis did not run. The only issue this PR references is shellhub-io/team#243, and this run cannot read another repository. |
The cloud test stack ran stripe-cli, which forwards every event on the Stripe test account to the stack's webhook, asynchronously and in no fixed order. A case that moves a subscription to past_due by sending an event would race the customer.subscription.updated Stripe itself emits moments after the subscription is created, and every other run or dev stack on the account feeds events in too. A stack now runs stripe-cli only when its Config sets StripeEvents. A case leaves it unset, signs the event it needs with the secret `stripe listen --print-secret` returns, the one the server verifies against, and posts it itself. The stack command behind the console's Playwright suite sets StripeEvents for a cloud stack: billing.spec.ts subscribes through the UI and waits on the events Stripe sends back, and it cleans up through `docker compose exec stripe-cli`. The cloud stack serves openapi-cloud.json under strict validation, but TestMain bundled only the community and enterprise specs, so a cloud stack would mount a file the run never wrote. StripeAPI pins Stripe-Version to 2024-06-20, the version stripe-go v79 in cloud speaks. Versions from 2025-03-31 on dropped the legacy usage records the server reports devices through and the tests read back, and an account's default version can be one of them. BillNamespaceThrough writes the namespace's billing row directly: ShellHub creates the customer it bills a namespace through itself, and only the test can create one on a Stripe test clock.
c5b0f43 to
fe59642
Compare
|
/review |
|
Claude finished @otavio's task in 7m 15s —— View job Code Review CompleteReviewed 13 files. 2 inline issues posted. Standards: 0 violations, 2 judgement calls. Spec: not run. Correctness: 0. StandardsNo rule is broken. The more useful of the two judgement calls: the same SpecThe Spec axis did not run. The only issue this PR references is shellhub-io/team#243, which is in another repository this run cannot read. Additional notes
To request another review round, comment |
fe59642 to
8f87efa
Compare
|
/review |
|
Claude finished @otavio's task in 4m 24s —— View job Code Review CompleteReviewed 13 files. 2 inline issues posted. Standards: 0 violations, 2 judgement calls. Spec: not run. Correctness: 0. StandardsNo rule is broken. The more useful of the two judgement calls is about the webhook event types ( SpecThe Spec axis did not run. The only issue this PR references is shellhub-io/team#243, which is in another repository this run cannot read. Additional notes
To request another review round, comment |
The cases read Stripe the way the test account bills: the price is graduated with the first three devices free and aggregates usage with max. So the invoice.paid case accepts the free tier's devices before subscribing, leaving Stripe no usage record, and expects the paid invoice to set usage to three; and the re-subscribe case sets usage to four so the period's invoice is not $0 and the declining card fails it. The re-subscribe case runs its customer on a Stripe test clock and advances it in 30-day steps until Stripe cancels the subscription with reason payment_failed. That relies on the account's failed payment setting canceling the subscription after the last retry, as it does today. The past-due case holds Stripe's usage for ten seconds after the refused accept. The server reports inside the accept request, before it answers, so any record already exists by then; the hold only covers the usage summary catching up. Refs: shellhub-io/team#243
The enterprise job becomes a matrix over the editions that build from the cloud source. Its name template still yields "validate (tests - enterprise)", so the check keeps the name it has today. The cloud leg reads the Stripe test secrets ui-e2e.yml already uses, and a fork skips it as it skips the enterprise leg, for want of the cloud source. The community job's skip pattern is quoted because run: pastes it into a bash line, where an unquoted ( is a syntax error.
8f87efa to
1c4c9e9
Compare
|
/review |
|
Claude finished @otavio's task in 2m 44s —— View job Code Review CompleteI reviewed 13 files for Standards (repo rules, Go conventions, tests, code smells) and Correctness (security and runtime defects). I found no new issues, so the code can stay as it is. The Spec axis did not run. The only issue this PR references is shellhub-io/team#243, and this run cannot read another repository. |
This branch has not been deployed
1 participant
Summary
testcontainers cases for the Cloud billing items of shellhub-io/team#243, on the cloud stack against Stripe test mode, a CI leg that runs them, and the billing contract fixes the cases turned up.
Each case owns a namespace and a real test-mode subscription, deleted with its customer when the case ends. Stripe's own events no longer reach a
TestCloudBillingstack: real events for a case's own subscription would race the events the case sends itself, and every other run on the account feeds events in too. A case signs the event it needs with the webhook secret and posts it to/api/webhook-billing.The re-subscribe case puts its customer on a Stripe test clock with a card that declines, and advances the clock until Stripe cancels the subscription with reason
payment_failed. It then pays the open invoice and sends two concurrentinvoice.paidevents. The namespace must re-subscribe exactly once.Paired with shellhub-io/cloud#2652, which makes the webhook answer 400 instead of 500 to a request without a valid
Stripe-Signature. The cloud CI leg checks out that branch by name.Running them
Locally, put the three Stripe test-mode values (secret key, price ID, publishable key) in
shellhub/.env.override, under the names the cloud compose file reads, and run fromshellhub/:The cases assume what the test account has today:
maxCI
The job keeps the
validate (tests - enterprise)check name, and forks skip the cloud leg as they skip the enterprise one. The job was small enough to add here instead of leaving it as a follow-up.team#243 items
Lines for the issue, once merged:
"
invoice.paidactivates subscription" covers the handler's path for a subscription that is already active: it stays active and the period's usage is set to the namespace's accepted device count. Recovering an unpaid or incomplete subscription is its own item. "Unpaid/incomplete/paused blocks both accept AND connect" stays out, as the issue marks it unreachable. Nothing else is left uncovered. The SSH case also checks that SSH past the free tier is refused once the subscription is deleted, which shows the subscription is what lets it through.Not duplicated from #7437 or #7429: no case here asserts the community device limit, slot freeing or name conflicts.
Refs: shellhub-io/team#243
Evidence
Every run is in the
testcompose runner against Stripe test mode.invoice.paidneither recovering nor setting usage; thepayment_failedlog reworded;customer.deletedkeeping the subscription;subscription.updateddropping the period end and the ID checks;cancel_at_period_endignored;subscription.deletedkeepingMaxNamespaces;customer.subscription.createdhandled as an update. Every item case failed for its own reason:MaxNamespacesraised twice and two active Stripe subscriptions where one is expected.canceled.TestCloudBillingpasses, 19/19 subtests in 290 s, the re-subscribe case taking about 90 s.golangci-lintv2.14.0 ontests/with--build-tags dockerand uncapped issues: 0 issues.go vet, theenvironmentunit tests andgo mod tidyare clean. The OpenAPI prettier and redocly checks pass, with the same two redocly warnings master has. actionlint passes onqa.yml. The community suite did not run locally; this branch changes it only by bundling one more spec inTestMain, and CI runs it.Review
Three Codex (
gpt-6.1-sol, high effort) rounds on the Standards and Spec axes:open, and the case voids it.maxaggregation. Accepted: both cases were rebuilt around it.ctxdoc nit. Accepted.qa.ymljob runs Go throughsetup-go, the enterprise leg this extends included.pkg/errors. Thetestsmodule's helpers usefmt.Errorfthroughout.-skip ^(TestEnterprise|TestCloud)broke bash in the community job. Fixed: the regex is quoted.fix(openapi).StripeWebhookdoc. The doc comment I touched didn't state its whole contract. Fixed.Merge Danger
Door: two-way
Blast Radius: CI
tests/and CI change, plus two status codes and one sentence in the OpenAPI spec that now match what the server answers. The cloud leg adds avalidate (tests - cloud)check that creates and deletes customers, subscriptions and test clocks on the shared Stripe test account. It depends on that account keeping the price and failed-payment settings listed above.TestCloudBillingruns its stack withoutstripe-cli. The stack the console's Playwright suite runs on still has it, becausebilling.spec.tswaits on Stripe's own events and cleans up through it. Thevalidate (tests - postgres)job now also skips^TestCloud.