Skip to content

fix(web): stabilize auth loading states#2447

Merged
Patrick-Erichsen merged 1 commit into
openclaw:mainfrom
vyctorbrzezowski:brzezowski/fix-loading-flashes
Jun 1, 2026
Merged

fix(web): stabilize auth loading states#2447
Patrick-Erichsen merged 1 commit into
openclaw:mainfrom
vyctorbrzezowski:brzezowski/fix-loading-flashes

Conversation

@vyctorbrzezowski
Copy link
Copy Markdown
Contributor

@vyctorbrzezowski vyctorbrzezowski commented May 30, 2026

Summary

  • What changed: stabilized protected-route loading so unresolved auth/profile data stays in route-shaped skeletons instead of briefly rendering signed-out, empty, or permission-denied states.
  • Why: auth/profile resolution and route data queries could complete in different frames, so pages sometimes rendered intermediate states before the real content.

Behavioural Proof

Covered /dashboard, /settings, /stars, /import, and /management locally. Protected routes now keep skeletons up until auth/profile and the first required user-scoped queries have resolved, and user-scoped queries skip until a current user exists.

Manual route validation:

  • /dashboard: before, it could briefly show sign-in, an empty dashboard, or the wrong publisher state before the real dashboard loaded. Now it stays on the dashboard skeleton until auth, publishers, skills, and plugins are ready.
  • /settings, /settings?view=organizations, /settings?view=tokens: before, these could flash sign-in or empty settings sections while user/org/token queries were still resolving. Now the settings skeleton holds until the active section has the data it needs.
  • /stars, /stars?view=list: before, the page could briefly render sign-in or the empty stars state before the starred list resolved. Now it shows the stars skeleton until the user and starred skills query are ready.
  • /import: before, it used a generic login/loading transition while auth was still being checked. Now it shows the import-shaped skeleton before rendering either the signed-in import flow or the final sign-in prompt.
  • /management: before, staff users could briefly see Management only. before the staff profile resolved. Now management stays on its skeleton until auth and the management datasets are ready.
  • /skills/publish and /plugins/publish: before, publisher ownership data could resolve after the form started rendering, causing owner/publisher state to shift. Now publisher queries wait for the current user and the publish skeleton stays up during auth/profile loading.
  • /cli/device, /cli/auth, /docs/auth: before, these auth handoff pages could show a sign-in prompt before the auth check finished. Now they show an auth-flow skeleton until the session status is resolved.

Security / Trust Impact

No security or trust model changes. This only changes when protected UI renders while auth/profile state is unresolved.

Data / Deploy Impact

No data migration or deploy sequencing needed.

Verification

  • bun run ci:static
  • bun run test -- src/lib/useAuthStatus.test.tsx src/routes/-dashboard.test.tsx src/routes/-settings.test.tsx src/routes/-stars.test.tsx src/__tests__/import.route.test.tsx src/routes/cli/-auth.test.tsx src/routes/docs/-auth.test.tsx src/routes/-management.test.tsx
  • bun run test -- src/__tests__/skills-publish-route.test.tsx
  • bun run ci:unit
  • bunx tsc --noEmit --pretty false
  • Local Playwright protected-route smoke
  • Public PR preflight on a clean temp clone based on current upstream/main
  • Manual Chrome validation on the routes listed above

@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented May 30, 2026

@vyctorbrzezowski is attempting to deploy a commit to the Amantus Machina Team on Vercel.

A member of the Team first needs to authorize it.

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 30, 2026

Codex review: needs real behavior proof before merge. Reviewed May 30, 2026, 11:35 AM ET / 15:35 UTC.

Summary
The branch centralizes auth/profile loading in useAuthStatus, skips user-scoped queries until me exists, adds protected-route skeletons/tests, and documents auth-loading semantics.

Reproducibility: yes. from source inspection: current main tracks only Convex auth loading while protected routes also depend on users.me, allowing prompts or denied states before profile data settles. I did not run a live browser repro in this read-only review.

Review metrics: 1 noteworthy metric.

  • Diff surface: 20 files changed, +433/-147. The patch touches a shared auth hook plus several protected routes, so reviewers should treat it as broader than single-page loading polish.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Attach redacted real behavior proof for the protected-route loading transition, preferably a short recording or terminal/log output from the local protected-route smoke pass.
  • Update the PR body after adding proof so ClawSweeper re-reviews automatically; if it does not, ask a maintainer to comment @clawsweeper re-review.

Proof guidance:

  • [P1] Needs real behavior proof before merge: Missing: the PR body describes local manual validation and tests, but no screenshot, recording, terminal output, copied live output, linked artifact, or redacted log is attached. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
Visible browser proof would materially help verify that protected routes keep skeletons up during auth/profile resolution. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: capture /dashboard, /settings, /stars, /import, and /management during auth/profile resolution and verify skeletons remain until final content or sign-in state.

Risk before merge

  • [P1] No inspectable after-fix proof shows the protected-route transition in a real browser or runtime; tests and PR prose are useful but do not satisfy the external contributor proof gate.
  • [P1] The Vercel preview is authorization-required, so maintainers cannot rely on that preview for browser verification until it is authorized or another artifact is posted.

Maintainer options:

  1. Decide the mitigation before merge
    Land a narrow auth-loading fix after the contributor attaches redacted real protected-route proof and maintainers complete normal review.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] Contributor-supplied real behavior proof and normal maintainer review are still needed; there is no narrow ClawSweeper code repair to queue.

Security
Cleared: No concrete security or supply-chain concern was found; the diff is frontend route/hook/test/spec work with no dependency, workflow, secret, or release-script change.

Review details

Best possible solution:

Land a narrow auth-loading fix after the contributor attaches redacted real protected-route proof and maintainers complete normal review.

Do we have a high-confidence way to reproduce the issue?

Yes from source inspection: current main tracks only Convex auth loading while protected routes also depend on users.me, allowing prompts or denied states before profile data settles. I did not run a live browser repro in this read-only review.

Is this the best way to solve the issue?

Yes, the proposed direction is the narrow maintainable fix: centralize the auth/profile gate, skip user-scoped queries until me exists, and keep route-shaped skeletons during unresolved data. The remaining blocker is proof, not a code-direction defect found in review.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 9a20795b5432.

Label changes

Label justifications:

  • P2: This is a normal protected-route auth/loading bug fix with limited blast radius and no emergency security, data-loss, or availability signal.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Missing: the PR body describes local manual validation and tests, but no screenshot, recording, terminal output, copied live output, linked artifact, or redacted log is attached. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

What I checked:

  • AGENTS policy read: AGENTS.md was read fully; its durable-spec guidance is relevant because this PR adds specs/auth-loading.md, and the Convex guidance was checked for the query-skip changes. (AGENTS.md:1, 9a20795b5432)
  • No maintainer notes: No .agents/maintainer-notes/ files exist in this checkout, so there were no matching internal maintainer decisions to apply.
  • Current main auth/profile split: Current main's useAuthStatus returns isLoading: auth.isLoading while also reading api.users.me, so profile-query resolution can lag behind the loading flag. (src/lib/useAuthStatus.ts:7, 9a20795b5432)
  • Current main route flash path: Current main settings renders the signed-out prompt for any falsy me, including the unresolved undefined profile-query state. (src/routes/settings.tsx:221, 9a20795b5432)
  • PR implementation direction: The PR changes useAuthStatus to skip users.me until auth resolves and keeps loading true while the authenticated profile query is unresolved. (src/lib/useAuthStatus.ts:5, 0ee3d66a1fc8)
  • Protected-route skeleton gating: The PR keeps /dashboard on DashboardSkeleton while auth, publishers, paginated skills, or packages are unresolved. (src/routes/dashboard.tsx:153, 0ee3d66a1fc8)

Likely related people:

  • Patrick Erichsen: Blame attributes the current shared useAuthStatus hook and most protected-route loading code to the v0.18.0 release commit. (role: introduced behavior; confidence: high; commits: 875f026a2300; files: src/lib/useAuthStatus.ts, src/routes/dashboard.tsx, src/routes/settings.tsx)
  • Dallin Romney: Recent settings-route work touched a route whose signed-out/loading behavior this PR changes. (role: recent area contributor; confidence: medium; commits: 83cc4d0f8733; files: src/routes/settings.tsx)
  • Vyctor H. Brzezowski: Recent merged work by this contributor touched the skill/plugin publishing flows that this PR also updates, independent of authoring this PR. (role: adjacent owner; confidence: medium; commits: 321df223b2ef, 6adf379f3220, 87f2b846efce; files: src/routes/skills/publish.tsx, src/routes/plugins/publish.tsx, src/components/SkillDetailPage.tsx)
  • Jesse Merhi: Recent management-route work is adjacent to the /management loading and permission-denied behavior this PR changes. (role: recent area contributor; confidence: medium; commits: 8a2c0c06fd17, 90de729fe135; files: src/routes/management.tsx)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels May 30, 2026
@vyctorbrzezowski vyctorbrzezowski marked this pull request as ready for review May 30, 2026 15:24
@vyctorbrzezowski vyctorbrzezowski requested review from a team and BunsDev as code owners May 30, 2026 15:24
@vyctorbrzezowski vyctorbrzezowski changed the title fix: stabilize auth loading states fix(web): stabilize auth loading states May 30, 2026
@Patrick-Erichsen Patrick-Erichsen merged commit ded9ff4 into openclaw:main Jun 1, 2026
33 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants