Skip to content

Retry failed initial Cloud account status loads - #543

Merged
Maxerns merged 4 commits into
getsotto:mainfrom
sb123sb123:fix/539-initial-status-retry
Oct 13, 2026
Merged

Maxerns merged 4 commits into
getsotto:mainfrom
sb123sb123:fix/539-initial-status-retry

Conversation

@sb123sb123

@sb123sb123 sb123sb123 commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #539.

When the initial Cloud eligibility request fails, the panel now offers a clearly labelled retry. Retrying clears the stale error, shows a loading state, and disables duplicate requests. A repeated failure restores the retry control; successful eligibility loading reveals only the controls permitted by the returned state.

A StrictMode mount regression also verifies that the initial fetchEligibility request runs exactly once.

Validation:

  • Current rebased head: CloudAccountPanel.test.tsx passed 8/8; git diff --check passed.
  • Before the latest rebase and test follow-up, the WASM build, typecheck, full test suite (132 tests), and Vite build passed.
  • Local typecheck on the current checkout cannot complete because generated web/src/wasm/sotto_wasm.js is absent and wasm-pack is not installed.
  • GitHub Actions for the current head are marked action_required with no jobs started, awaiting repository approval.

The commits include the required DCO sign-off.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

Thanks for your first pull request to Sotto, and welcome.

Two things that trip up newcomers:

  • Every commit needs a Signed-off-by line under the Developer Certificate of Origin - PRs with unsigned commits cannot be merged. Sign off with git commit -s; if your branch already has unsigned commits, run git rebase --signoff main and push again.
  • Please run cargo fmt --all --check and cargo clippy --workspace --all-targets -- -D warnings before pushing, and add tests for new behaviour (cargo test --workspace runs the suite).

The full guide is in CONTRIBUTING.md - a maintainer will review shortly.

@Maxerns Maxerns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Strong PR overall.

I verified it in isolation under CI-equivalent conditions with the real build:wasm output present:

  • full suite: 132/132 green
  • typecheck: fully clean
  • commit: signed off

The issue criteria are all covered: the retry is labelled and paired with the alert, stale errors clear correctly, retry is disabled while pending, a repeat failure restores the retry state, successful eligibility gates the controls properly, and no quote request is made before eligibility has loaded.

The two wasmLoader failures on a bare checkout are the known generated-bindings issue from #442, so nothing here is introducing that.

There’s just one test gap before I approve.

The overlap test currently clicks the disabled Retrying button, but disabled buttons don’t dispatch the click handler in this test setup. I removed the in-flight guard and all 7 tests still passed, so the guard itself isn’t actually pinned.

Could you add a StrictMode mount case that asserts fetchEligibility is called exactly once?

I tried that locally and it behaves exactly how we want: it passes with the guard in place and fails when the guard is removed. That would test the actual mechanism rather than only the disabled UI state.

Once that’s in, I’m happy to approve.

Signed-off-by: 张峻祎 <zhangjunyi20130307@163.com>
Signed-off-by: 张峻祎 <zhangjunyi20130307@163.com>
@sb123sb123
sb123sb123 force-pushed the fix/539-initial-status-retry branch from 54fb7fe to 80d23ee Compare October 7, 2026 23:40

@Maxerns Maxerns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The StrictMode test is exactly what was needed.

I verified the panel suite at 8/8 green, and removing the in-flight guard breaks exactly that test, so the overlap protection is now genuinely pinned rather than just implied by the disabled button state.

The fix itself looks good, so I’m approving on content.

One note: the branch has picked up a small conflict with main around the adjacent state declarations, where the offer type was renamed. I’ll handle that merge resolution myself, so there’s nothing else you need to do here.

Signed-off-by: Maxerns <maksafk@gmail.com>
The 'monthly' literal no longer satisfies the renamed BillingOffer type after merging main; use standard_monthly.

Signed-off-by: Maxerns <maksafk@gmail.com>
@Maxerns
Maxerns merged commit 77c10b0 into getsotto:main Oct 13, 2026
22 checks passed
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.

Web: allow retrying a failed initial Cloud account status request

3 participants