[web] Gate page tree fetch on auth readiness

[2witstudios]

Motivation

• The page tree hook could start network fetches before the auth store was hydrated on desktop/capacitor cold starts, causing the UI to hang in an infinite skeleton state until a manual refresh.
• Prior fixes reduced some failure modes, but usePageTree still initiated SWR fetches too early when session/auth state wasn't ready.

Description

• Gate usePageTree fetch key on auth readiness by reading hasHydrated, isLoading, and isAuthenticated from useAuthStore and only setting the SWR key when auth is ready (shouldFetch).
• Adjust isLoading returned by usePageTree to account for auth hydration/loading state so the UI doesn't misinterpret pre-auth state as a permanent fetch failure.
• Update unit tests for usePageTree to mock the auth store (useAuthStore) and set expected hydrated/auth states in apps/web/src/hooks/__tests__/usePageTree.test.ts.

Testing

• Updated the unit test file apps/web/src/hooks/__tests__/usePageTree.test.ts to mock useAuthStore state for the new gating logic; tests were modified but not executed as part of this change.
• No automated test runs were performed for this patch (no CI/test commands executed).

---

Codex Task

Summary by CodeRabbit

• Tests

  • Strengthened test setup for authentication state handling and data operations.

• Bug Fixes

  • Loading indicators now accurately reflect authentication and synchronization status.
  • Data fetching properly awaits authentication completion before proceeding.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds authentication and hydration awareness to the usePageTree hook. The hook now checks auth state before fetching, gates data loading initiation based on completion of hydration and authentication flows, and extends the loading indicator to reflect these states. Test setup is augmented with hoisted mock auth state for controlled testing.

Changes

Cohort / File(s)	Summary
Authentication-aware test setup apps/web/src/hooks/__tests__/usePageTree.test.ts	Introduces hoisted mockAuthState and useAuthStore mock to centralize auth state management in tests, enabling explicit control over hydration, loading, and authentication status during page-tree operations.
Authentication-gated data fetching apps/web/src/hooks/usePageTree.ts	Adds shouldFetch guard requiring driveId, completed hydration, no active loading, and authenticated status; extends isLoading to reflect auth/hydration state; gates SWR revalidation with hydration-aware pausing (isPaused).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• test: improve test quality with observable outcomes #82: Modifies usePageTree tests and auth-store mocking/hydration behavior alongside the main hook updates to gate fetching on auth/hydration.
• Fix middle section headers stuck loading #120: Adjusts SWR fetch-gating logic in usePageTree; this PR adds auth/hydration-based shouldFetch guards while that PR focuses on initial-load safeguards.

Poem

🐰 Hops through hydration's careful gate,
Auth checks must come before the wait,
No fetch begins till all align,
State-aware flows in perfect line! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title '[web] Gate page tree fetch on auth readiness' directly and concisely summarizes the main change: adding authentication readiness checks to gate the page tree data fetching.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch codex/fix-loading-issue-on-cold-start

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

apps/web/src/hooks/__tests__/usePageTree.test.ts (1)

99-108: 🛠️ Refactor suggestion | 🟠 Major

No tests exercise the auth-gating logic this PR introduces.

All tests set mockAuthState to the fully-authenticated happy path. The core behavior change — gating fetches on hasHydrated, isAuthLoading, and isAuthenticated — has zero coverage:

• hasHydrated: false → isLoading should be true, SWR key should be null (no fetch).
• isAuthenticated: false (after hydration) → isLoading should be false, tree should be [].
• Transition from not-hydrated to hydrated+authenticated → should trigger fetch.

Since this is the exact bug the PR fixes (infinite skeleton on cold start), at least the first two scenarios should have regression tests.

🧹 Nitpick comments (3)

apps/web/src/hooks/usePageTree.ts (2)

178-178: isLoading logic is subtly correct but warrants a clarifying comment.

The expression (!hasHydrated || isAuthLoading || isAuthenticated) serves as "auth not ready OR user is authenticated (so we expect data to come)." When auth completes and the user is not authenticated, isLoading drops to false — avoiding the infinite skeleton this PR aims to fix. The logic checks out, but a future reader could misread isAuthenticated as a bug (why would "authenticated" mean "loading"?).

Consider adding a brief inline comment:

Suggested clarification

  return {
    tree: data ?? [],
-   isLoading: !error && !data && !!driveId && (!hasHydrated || isAuthLoading || isAuthenticated),
+   // Show loading while auth isn't ready, or while authenticated and waiting for data.
+   // When auth is ready and user is NOT authenticated, immediately stop loading (no fetch will occur).
+   isLoading: !error && !data && !!driveId && (!hasHydrated || isAuthLoading || isAuthenticated),
    isError: error,

---

85-100: Missing revalidateOnFocus: false in SWR config.

The SWR options don't include revalidateOnFocus: false. Per the project guidelines, this should be set for editing protection so that tabbing back into the app doesn't trigger a tree refetch mid-edit. This is pre-existing, so not blocking this PR, but worth noting since you're already touching the SWR configuration surface.

Based on learnings: "Use SWR for server state and caching with proper configuration including revalidateOnFocus: false for editing protection".

apps/web/src/hooks/__tests__/usePageTree.test.ts (1)

26-30: Consider adding authFailedPermanently to the mock state.

The auth store exposes authFailedPermanently (visible in the relevant snippet from useAuthStore.ts). If usePageTree or a future consumer ever needs to distinguish "not authenticated" from "auth failed permanently," the mock would need this field. Low priority for now, but noting it since you're defining the mock shape.