Skip to content

Conversation

@jchris
Copy link
Contributor

@jchris jchris commented Nov 25, 2025

Summary

Initial proof-of-concept for Fireproof Dashboard integration at /fireproof route. This PR prepares the vibes.diy codebase to consume the Fireproof dashboard API (currently in unmerged PR in fireproof repo).

What's Implemented

  • ✅ Installed and configured @tanstack/react-query for data fetching
  • ✅ Added QueryClientProvider to root layout (inside ClerkProvider, wrapping entire app)
  • ✅ Created /fireproof route with dashboard component
  • ✅ Implemented API client setup using Clerk authentication with "with-email" template
  • ✅ Added queries for listing user tenants and ledgers
  • ✅ Built UI to display tenant and ledger information with proper loading/error states
  • ✅ Used existing VITE_CONNECT_API_URL environment variable for API endpoint

Current Blocker

Build Error: DashboardApi is not exported from @fireproof/core-protocols-dashboard@0.23.15

error TS2305: Module '"@fireproof/core-protocols-dashboard"' has no exported member 'DashboardApi'.

The dashboard API client and types exist in the Fireproof codebase but are not yet published in the npm package. Once the Fireproof dashboard PR is merged and published, this will work.

Architecture Decisions

  1. Simple POC approach: Dashboard logic lives directly in the route component (not a reusable context) for faster iteration
  2. React Query: Chose React Query for API state management - aligns well with async data fetching patterns
  3. Clerk "with-email" token template: Required by Fireproof dashboard API for user identification
  4. Result monad handling: Followed Fireproof's pattern of throwing errors directly from .Err() without wrapping

Files Changed

  • vibes.diy/pkg/app/root.tsx - Added QueryClientProvider setup
  • vibes.diy/pkg/app/routes.ts - Added /fireproof route configuration
  • vibes.diy/pkg/app/routes/fireproof.tsx - New dashboard route component
  • vibes.diy/pkg/package.json - Added dependencies

Next Steps

  1. Wait for Fireproof dashboard API to be published to npm
  2. Test with real API endpoint once available
  3. Add create/update/delete functionality (currently read-only)
  4. Consider extracting to reusable context if needed beyond POC

Test Plan

  • Code compiles (blocked by missing export)
  • Route accessible at /fireproof
  • Auth check works (logged out users see prompt)
  • API calls succeed with Clerk token
  • Tenants display correctly
  • Ledgers display correctly with tenant associations

🤖 Generated with Claude Code

@netlify
Copy link

netlify bot commented Nov 25, 2025

Deploy Preview for fireproof-ai-builder ready!

Name Link
🔨 Latest commit ebd9e03
🔍 Latest deploy log https://app.netlify.com/projects/fireproof-ai-builder/deploys/692eeaabdb9af400083b1aa8
😎 Deploy Preview https://deploy-preview-659--fireproof-ai-builder.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 45 (🟢 up 1 from production)
Accessibility: 97 (no change from production)
Best Practices: 92 (no change from production)
SEO: 92 (no change from production)
PWA: 60 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 56 to 169
const tenantsQuery = useQuery<ResListTenantsByUser>({
queryKey: ["listTenantsByUser"],
queryFn: wrapResultToPromise(() => api.listTenantsByUser({})),
enabled: isLoaded && isSignedIn,
});

Choose a reason for hiding this comment

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

P1 Badge Avoid leaking cached tenant/ledger data across users

React Query caches both tenant and ledger fetches under fixed keys (["listTenantsByUser"] and ["listLedgersByUser"], lines 56‑66) while the global QueryClient in root.tsx marks query data fresh for five minutes (lines 25‑30). If user A signs out and user B signs into the same browser within that window, React Query treats the cached data as fresh and will render A’s tenants/ledgers for B without refetching, leaking account data across sessions. The cache needs to be invalidated on auth changes or the keys scoped by user identity.

Useful? React with 👍 / 👎.

Copy link
Contributor

@charliecreates charliecreates bot left a comment

Choose a reason for hiding this comment

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

The main functional concern is that DashboardApi is imported from @fireproof/core-protocols-dashboard even though it is not yet exported by the published package, which will cause build failures and tightly couples this POC to an unpublished upstream change. There is also a compatibility risk from unguarded window.fetch usage in the route component, which will break SSR or non-DOM environments. UX-wise, the dashboard does not clearly handle the !isLoaded Clerk state, and uses array indices as React keys for ledger users, which could lead to subtle rendering issues once mutations are added. Overall structure and React Query integration look reasonable, but these issues should be addressed before merging into a mainline branch.

Additional notes (1)
  • Maintainability | vibes.diy/pkg/app/routes/fireproof.tsx:114-271
    The tenant and ledger sections both implement nearly identical patterns for loading, error, empty, and list states, with only labels and field mappings differing. While acceptable for a POC, this duplication will be hard to keep in sync as you evolve the dashboard (e.g. shared styling, skeletons, or error behaviors).

Extracting a small reusable Section/List component that accepts title, query, and a render function for items would simplify future feature additions and reduce the chance of diverging UX between related lists.

Summary of changes

Summary of Changes

  • Added @fireproof/core-protocols-dashboard and @tanstack/react-query as app dependencies and wired them into the pnpm-lock.yaml.
  • Introduced a global QueryClient instance and wrapped the app tree with QueryClientProvider inside ClerkProvider in root.tsx.
  • Registered a new /fireproof route in routes.ts.
  • Implemented a new FireproofDashboard route component that:
    • Uses Clerk auth (useAuth) and a DashboardApi client (from @fireproof/core-protocols-dashboard).
    • Uses React Query to fetch and display tenants and ledgers for the signed-in user.
    • Provides loading, error, and empty states, plus an unauthenticated view.
  • Added basic meta tags for the Fireproof dashboard route.

Comment on lines 41 to 71
const api = useMemo(() => {
return new DashboardApi({
apiUrl: VibesDiyEnv.CONNECT_API_URL(),
fetch: window.fetch.bind(window),
getToken: async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instantiating DashboardApi with window.fetch.bind(window) will throw at render time in any non-DOM environment (SSR, tests, or future worker contexts) because window is undefined there. Since this route component is imported from the routes table, any server-side evaluation or pre-rendering will crash before auth or query logic runs.

For route components in frameworks that may execute code on the server, you should avoid unconditional window access in the render path. A safer approach is to:

  • Use the global fetch (which is available in more environments) if the API client supports it, or
  • Lazily create the client only when typeof window !== 'undefined', and show a loading or unsupported message otherwise.

As-is, this POC could break SSR or automated tests that render the route.

Suggestion

Switch to the global fetch if DashboardApi does not require a bound window instance, e.g. fetch: fetch, or guard the creation with a runtime check such as if (typeof window === 'undefined') return null; (with a corresponding UI fallback) before you call new DashboardApi(...). I can suggest a concrete guard pattern and minimal UI fallback for non-browser environments if you’d like — reply with "@CharlieHelps yes please" and I’ll add it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just simplify dont test window

Comment on lines 37 to 239
export default function FireproofDashboard() {
const { isSignedIn, isLoaded, getToken } = useAuth();

// Create DashboardApi instance with Clerk auth
const api = useMemo(() => {
return new DashboardApi({
apiUrl: VibesDiyEnv.CONNECT_API_URL(),
fetch: window.fetch.bind(window),
getToken: async () => {
const token = await getToken({ template: "with-email" });
return {
type: "clerk" as const,
token: token || "",
};
},
});
}, [getToken]);

// Query to list all tenants for the logged-in user
const tenantsQuery = useQuery<ResListTenantsByUser>({
queryKey: ["listTenantsByUser"],
queryFn: wrapResultToPromise(() => api.listTenantsByUser({})),
enabled: isLoaded && isSignedIn,
});

// Query to list all ledgers for the logged-in user
const ledgersQuery = useQuery<ResListLedgersByUser>({
queryKey: ["listLedgersByUser"],
queryFn: wrapResultToPromise(() => api.listLedgersByUser({})),
enabled: isLoaded && isSignedIn,
});

// Not authenticated view
if (isLoaded && !isSignedIn) {
return (
<SimpleAppLayout
headerLeft={
<div className="flex items-center">
<a
href="/"
className="text-light-primary dark:text-dark-primary hover:text-accent-02-light dark:hover:text-accent-02-dark flex items-center px-3 py-2"
aria-label="Go to home"
>
<HomeIcon className="h-6 w-6" />
</a>
</div>
}
>
<div className="mx-auto max-w-4xl px-4 py-8">
<h1 className="text-2xl font-bold mb-6">Fireproof Dashboard</h1>
<div className="border-light-decorative-01 dark:border-dark-decorative-01 rounded-sm border p-6">
<p className="text-light-secondary dark:text-dark-secondary">
Please sign in to view your Fireproof tenants and ledgers.
</p>
</div>
</div>
</SimpleAppLayout>
);
}

return (
Copy link
Contributor

Choose a reason for hiding this comment

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

The FireproofDashboard component renders tenant and ledger sections even when isLoaded is false, which means you can hit a state where Clerk has not finished loading but the React Query hooks are disabled (enabled: false). In that case, the UI shows neither loading nor auth status explicitly, which can be confusing — the page appears mostly empty instead of showing a loading indicator while auth initializes.

A small explicit !isLoaded loading state before the signed-in/unsigned branches would improve perceived correctness and make the auth lifecycle clearer.

Suggestion

Add an early if (!isLoaded) branch that returns a simple loading view (e.g., a spinner or "Loading account..." message) before checking isSignedIn. This keeps the UX consistent while Clerk initializes. If you’d like a concrete snippet added to the component, reply with "@CharlieHelps yes please" and I’ll propose one.

Comment on lines 254 to 407
{ledger.users.map((user, idx) => (
<span
key={idx}
className="px-2 py-1 text-xs rounded bg-gray-100 dark:bg-gray-800 text-light-secondary dark:text-dark-secondary"
Copy link
Contributor

Choose a reason for hiding this comment

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

The users.map in the ledger section uses the array index (idx) as the React key. This can cause subtle UI bugs if the users array changes over time (e.g., order or membership), because React will reuse DOM nodes incorrectly. Since each user appears to have a stable identifier (user.userId), that would be a more appropriate key.

In a dashboard that will eventually support mutations (add/remove users), using a stable key now will avoid headaches later.

Suggestion

Prefer a stable identifier over the array index for the React key, e.g. key={user.userId} (or a composite of ledger.ledgerId and user.userId if needed). I can draft the exact change in this map call if you reply with "@CharlieHelps yes please".

Comment on lines 26 to 53
// Helper to convert Result monad to Promise for React Query
function wrapResultToPromise<T>(pro: () => Promise<Result<T>>) {
return async (): Promise<T> => {
const res = await pro();
if (res.isOk()) {
return res.Ok();
}
throw res.Err();
};
Copy link
Contributor

Choose a reason for hiding this comment

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

wrapResultToPromise is a nice adapter, but it currently throws res.Err() directly, which for non-Error values will produce opaque error output in React Query (e.g. "Error: [object Object]") and can break places that do instanceof Error checks. In this file you are already checking tenantsQuery.error instanceof Error, so non-Error values will silently fall back to the generic "Unknown error" message even if the underlying error contained useful details.

Given you mentioned following the pattern of throwing errors directly from .Err(), it's worth normalizing here at the boundary with React Query to guarantee consumers always see an Error with a meaningful message while still preserving the original error for logging or debugging.

Suggestion

You can wrap non-Error results into an Error instance while preserving details for inspection, so React Query and your UI logic behave consistently:

function wrapResultToPromise<T>(pro: () => Promise<Result<T>>) {
  return async (): Promise<T> => {
    const res = await pro();
    if (res.isOk()) {
      return res.Ok();
    }

    const err = res.Err();
    if (err instanceof Error) {
      throw err;
    }

    // Preserve original error object for debugging
    // while ensuring React Query always gets an Error
    const wrapped = new Error(
      typeof err === "string" ? err : "Request failed",
    );
    (wrapped as any).cause = err;
    throw wrapped;
  };
}

This keeps the Result-monad semantics but makes error handling in the UI predictable. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.

Comment on lines 55 to 179
// Query to list all tenants for the logged-in user
const tenantsQuery = useQuery<ResListTenantsByUser>({
queryKey: ["listTenantsByUser"],
queryFn: wrapResultToPromise(() => api.listTenantsByUser({})),
enabled: isLoaded && isSignedIn,
});

// Query to list all ledgers for the logged-in user
const ledgersQuery = useQuery<ResListLedgersByUser>({
queryKey: ["listLedgersByUser"],
queryFn: wrapResultToPromise(() => api.listLedgersByUser({})),
enabled: isLoaded && isSignedIn,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Both tenants and ledgers queries use static keys without incorporating the current user or auth state. If a user signs out and another user signs in within the same browser session, React Query will happily serve the cached data for the previous user under the same keys ("listTenantsByUser", "listLedgersByUser"), leading to potential cross-user data leakage within the client session.

Using Clerk, you likely have access to a stable user identifier (e.g. user ID or email from the token/template). Incorporating this identifier into the query keys and/or clearing the query cache on auth changes avoids accidental reuse of previous users' cached dashboard data.

Suggestion

Include a user-specific element in the query keys so cache entries are properly segmented per user, or explicitly clear these queries on sign-out:

const { isSignedIn, isLoaded, getToken, userId } = useAuth();

const tenantsQuery = useQuery<ResListTenantsByUser>({
  queryKey: ["listTenantsByUser", userId],
  queryFn: wrapResultToPromise(() => api.listTenantsByUser({})),
  enabled: isLoaded && isSignedIn && !!userId,
});

const ledgersQuery = useQuery<ResListLedgersByUser>({
  queryKey: ["listLedgersByUser", userId],
  queryFn: wrapResultToPromise(() => api.listLedgersByUser({})),
  enabled: isLoaded && isSignedIn && !!userId,
});

Alternatively, you can subscribe to Clerk auth events and call queryClient.removeQueries({ queryKey: ["listTenantsByUser"] }) / removeQueries({ queryKey: ["listLedgersByUser"] }) on sign-out. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.

Comment on lines +23 to +33
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";

// Create a client instance for React Query
const queryClient = new QueryClient({
defaultOptions: {
queries: {
staleTime: 1000 * 60 * 5, // 5 minutes
refetchOnWindowFocus: false,
},
},
});
Copy link
Contributor

Choose a reason for hiding this comment

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

The global QueryClient instance is created at module scope with default options, which is good, but there is no strategy for cache invalidation on auth changes. Because the QueryClientProvider wraps the entire app, when a user signs out and another user signs in, any user-scoped queries (like your dashboard data) will remain in the cache unless keys or session boundaries are managed explicitly.

Especially when combined with static query keys, this can create subtle cross-user data exposure within the same browser session. Even if you fix the keys, it's generally a good idea to reset or partially clear the query cache on auth state transitions for user-scoped data.

Suggestion

Consider either scoping the QueryClient to the current auth session or listening to Clerk auth events to clear user-specific queries on sign-out:

// example: create a hook or effect near your auth boundary
useEffect(() => {
  if (!isSignedIn && isLoaded) {
    queryClient.clear();
  }
}, [isSignedIn, isLoaded]);

Alternatively, if you segment query keys by userId, target only those keys with removeQueries. This keeps the global cache while preventing leakage of user-specific data across sessions. Reply with "@CharlieHelps yes please" if you'd like me to add a commit wiring this up with Clerk auth events.

@charliecreates charliecreates bot removed the request for review from CharlieHelps November 25, 2025 12:54
@github-actions
Copy link

🎉 Hosting Preview Deployment

Preview URL: https://pr-659-vibes-hosting-v2.jchris.workers.dev
Worker: pr-659-vibes-hosting-v2
Commit: 2bae9a0

Test the preview:

  1. Visit the preview URL above
  2. Test hosting functionality (KV assets, queue operations)
  3. Verify API integrations work correctly
  4. Check domain routing is disabled (workers.dev only)

This preview will be automatically updated on each push to this PR

@jchris jchris force-pushed the jchris/fireproof-dashboard-api branch from 25d0366 to 1488985 Compare November 30, 2025 17:14
@jchris jchris marked this pull request as draft December 1, 2025 14:49
@jchris jchris marked this pull request as ready for review December 1, 2025 14:57
@charliecreates charliecreates bot requested a review from CharlieHelps December 1, 2025 14:57
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 139 to 142
useEffect(() => { // Moved useEffect here
if (!isLoaded || !isSignedIn || fpCloudToken) return;

const fetchFpCloudToken = async () => {

Choose a reason for hiding this comment

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

P1 Badge Reset fp-cloud token on sign-out

The effect that fetches fp-cloud-jwt short-circuits whenever fpCloudToken is truthy, but the token state is never cleared when a user signs out. If user A opens /fireproof, gets a token, signs out, and user B signs in, fpCloudToken still holds A’s token so the effect never requests a new session token and all subsequent API calls use the stale UCAN. That results in B seeing A’s data or hitting auth errors until the page is refreshed.

Useful? React with 👍 / 👎.

Comment on lines 162 to 165
const tenantsQuery = useQuery<ResListTenantsByUser>({
queryKey: ["listTenantsByUser"],
queryFn: wrapResultToPromise(
() => api.listTenantsByUser({}),

Choose a reason for hiding this comment

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

P1 Badge Scope query cache to current user

Both tenant and ledger queries use static keys (e.g. queryKey: ["listTenantsByUser"]) while the new QueryClient is configured with a 5‑minute stale window (root.tsx lines 26‑30). If user A loads the dashboard and signs out, then user B signs in within that window, React Query will treat the cached response as fresh and serve A’s tenants/ledgers to B without refetching. The query keys should include the signed-in user (or the cache should be invalidated on auth changes) to avoid cross-user data leakage.

Useful? React with 👍 / 👎.

Copy link
Contributor

@charliecreates charliecreates bot left a comment

Choose a reason for hiding this comment

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

  • The DashboardApi import from @fireproof/core-protocols-dashboard will continue to break builds until the upstream package exports it, tightly coupling this PR to an unpublished change.
  • The FireproofDashboard route directly uses window.fetch, which will throw in non-DOM environments and should be guarded or replaced with global fetch.
  • React Query usage currently lacks user-scoped keys and cache/session management, risking cross-user data leakage within a single browser session.
  • Error normalization and minor UI/logging details (e.g. wrapResultToPromise, missing !isLoaded state, array index keys) can be improved for robustness and maintainability.
Summary of changes

Summary of Changes

  • Added @tanstack/react-query and @fireproof/core-protocols-dashboard as dependencies and wired them into pnpm-lock.yaml and package.json.
  • Wrapped the app in a global QueryClientProvider (inside ClerkProvider) in root.tsx, configuring default query options.
  • Registered a new /fireproof route in routes.ts.
  • Implemented a new FireproofDashboard route that:
    • Uses Clerk auth and a DashboardApi client to obtain a fp-cloud-jwt via getCloudSessionToken and then fetch tenants and ledgers with React Query.
    • Adds extensive logging, JWT decoding, and special handling for JWKS verification failures.
    • Renders tenants and ledgers sections with loading, error, and empty states.
  • Added token-auth-and-exchange.md documenting the current auth/token exchange debugging findings and hypotheses.

Comment on lines 59 to 137
const [fpCloudToken, setFpCloudToken] = useState<string | null>(null); // Moved useState here

// Create DashboardApi instance with Clerk auth
const api = useMemo(() => {
const apiUrl = VibesDiyEnv.CONNECT_API_URL();
console.log(
"[Fireproof Dashboard] 🔧 Creating DashboardApi instance with URL:",
apiUrl,
);
return new DashboardApi({
apiUrl,
fetch: window.fetch.bind(window),
getToken: async () => {
if (fpCloudToken) {
console.log("[Fireproof Dashboard] 🔑 Using existing fp-cloud-jwt.");
return {
type: "ucan" as const, // Corrected type here
token: fpCloudToken,
};
}

console.log(
"[Fireproof Dashboard] 🔑 Getting Clerk token with template: with-email",
);
const token = await getToken({ template: "with-email" });

if (token) {
try {
const claims = decodeJwt(token);
const header = decodeProtectedHeader(token);

// Check KID match for debugging
const expectedKid = "ins_35qNS5Jwyc7z4aJRBIS7o205yzb";
if (header.kid === expectedKid) {
console.log("[Fireproof Dashboard] ✅ KID MATCHES expected dev key:", header.kid);
} else {
console.warn("[Fireproof Dashboard] ⚠️ KID MISMATCH:", {
tokenKid: header.kid,
expectedKid,
note: "This might be why verification fails if the backend is using the 'expected' key."
});
}

const now = Date.now() / 1000;
const exp = claims.exp || 0;
const ttl = exp - now;

console.log("[Fireproof Dashboard] 🕵️‍♀️ Token Details:", {
header,
payload: {
iss: claims.iss,
aud: claims.aud,
exp: claims.exp,
iat: claims.iat,
ttl: ttl.toFixed(2) + "s",
},
});

if (exp < now) {
console.error(
"/[Fireproof Dashboard] ❌ Token is EXPIRED! Client clock may be wrong or Clerk returned old token.",
);
}
} catch (e) {
console.error("[Fireproof Dashboard] ⚠️ Failed to parse token:", e);
}
}

console.log(
"/[Fireproof Dashboard] 🎫 Token retrieved:",
token ? `${token.substring(0, 20)}...` : "null",
);
return {
type: "clerk" as const,
token: token || "",
};
},
});
}, [getToken, fpCloudToken]);
Copy link
Contributor

Choose a reason for hiding this comment

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

The memoized DashboardApi client is configured to use an fp-cloud-jwt when fpCloudToken is present, otherwise it falls back to a Clerk token. Once fpCloudToken is set, you never attempt to refresh it or check its expiry, and getToken will continue to return it indefinitely.

Given that you already decode the Clerk token and log expiry information, not doing any TTL/expiry management on the exchanged token risks 401s/failures later in the session due to an expired fp-cloud-jwt being reused. At minimum, this should be called out as a known limitation, but ideally there should be a basic expiry check and refresh mechanism.

Suggestion

Augment the fpCloudToken state to also track its expiry (if available from the getCloudSessionToken response) or last-issued timestamp, and teach getToken to fall back to a fresh Clerk-based exchange when the cached fp-cloud-jwt is stale. This can be as simple as storing { token, exp } and checking exp against Date.now()/1000 before deciding whether to reuse or refresh. If you’d like, I can propose a specific shape for this state and the corresponding refresh logic—reply with "@CharlieHelps yes please" and I’ll include a concrete code change.

Comment on lines 139 to 158
useEffect(() => { // Moved useEffect here
if (!isLoaded || !isSignedIn || fpCloudToken) return;

const fetchFpCloudToken = async () => {
console.log("[Fireproof Dashboard] 🚀 Attempting to get fp-cloud-jwt...");
try {
const result = await api.getCloudSessionToken({});
if (result.isOk()) {
setFpCloudToken(result.Ok().token);
console.log("[Fireproof Dashboard] ✅ Successfully retrieved fp-cloud-jwt. Enabling data queries.");
} else {
console.error("[Fireproof Dashboard] ❌ Error getting fp-cloud-jwt:", result.Err());
}
} catch (e) {
console.error("[Fireproof Dashboard] ❌ Exception getting fp-cloud-jwt:", e);
}
};

fetchFpCloudToken();
}, [isLoaded, isSignedIn, fpCloudToken, api]);
Copy link
Contributor

Choose a reason for hiding this comment

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

fetchFpCloudToken is called from a useEffect that depends on api, and api is recreated whenever fpCloudToken changes. After setFpCloudToken, the component re-renders, api is recreated with a new closure over the updated fpCloudToken, and the effect runs again. The guard if (!isLoaded || !isSignedIn || fpCloudToken) return; currently prevents an infinite loop, but the dependency on api is unnecessary and makes the effect’s behaviour harder to reason about.

Since the effect only needs a stable client reference plus auth flags and fpCloudToken, tying it directly to api is over-coupling and can surprise future maintainers if api’s dependencies change.

Suggestion

Consider decoupling the effect from the full api object by either (1) extracting just the getCloudSessionToken function into a stable callback or (2) suppressing api in the dependency list with a clear comment and relying on the existing fpCloudToken/auth guards to avoid stale reads. Option (1) is cleaner long term; I can sketch the minimal refactor that memoizes getCloudSessionToken separately and uses that in the effect—reply with "@CharlieHelps yes please" and I’ll provide a patch.

Comment on lines 256 to 421
<div className="space-y-8">
{/* Tenants Section */}
<div>
<h2 className="text-xl font-semibold mb-4">Tenants</h2>
{tenantsQuery.isLoading && (
<div className="border-light-decorative-01 dark:border-dark-decorative-01 rounded-sm border p-6">
<p className="text-light-secondary dark:text-dark-secondary">
Loading tenants...
</p>
</div>
)}
{tenantsQuery.isError && (
<div className="border-red-500 rounded-sm border p-6 bg-red-50 dark:bg-red-900/20">
<p className="text-red-700 dark:text-red-300">
Error loading tenants:{" "}
{tenantsQuery.error instanceof Error
? tenantsQuery.error.message
: "Unknown error"}
</p>
</div>
)}
{tenantsQuery.isSuccess && (
<div className="space-y-3">
{tenantsQuery.data.tenants.length === 0 ? (
<div className="border-light-decorative-01 dark:border-dark-decorative-01 rounded-sm border p-6">
<p className="text-light-secondary dark:text-dark-secondary">
No tenants found.
</p>
</div>
) : (
tenantsQuery.data.tenants.map((tenant: UserTenant) => (
<div
key={tenant.tenantId}
className="border-light-decorative-01 dark:border-dark-decorative-01 rounded-sm border p-4"
>
<div className="flex justify-between items-start">
<div>
<h3 className="font-medium text-light-primary dark:text-dark-primary">
{tenant.tenant.name || "Unnamed Tenant"}
</h3>
<p className="text-sm text-light-secondary dark:text-dark-secondary font-mono">
ID: {tenant.tenantId}
</p>
</div>
<div className="flex gap-2">
<span
className={`px-2 py-1 text-xs rounded ${
tenant.role === "admin"
? "bg-blue-100 text-blue-800 dark:bg-blue-900/30 dark:text-blue-300"
: "bg-gray-100 text-gray-800 dark:bg-gray-800 dark:text-gray-300"
}`}
>
{tenant.role}
</span>
{tenant.default && (
<span className="px-2 py-1 text-xs rounded bg-green-100 text-green-800 dark:bg-green-900/30 dark:text-green-300">
default
</span>
)}
</div>
</div>
<div className="mt-2 text-sm text-light-secondary dark:text-dark-secondary">
<p>Status: {tenant.tenant.status}</p>
<p>
Created:{" "}
{new Date(
tenant.tenant.createdAt,
).toLocaleDateString()}
</p>
</div>
</div>
))
)}
</div>
)}
</div>

{/* Ledgers Section */}
<div>
<h2 className="text-xl font-semibold mb-4">Ledgers</h2>
{ledgersQuery.isLoading && (
<div className="border-light-decorative-01 dark:border-dark-decorative-01 rounded-sm border p-6">
<p className="text-light-secondary dark:text-dark-secondary">
Loading ledgers...
</p>
</div>
)}
{ledgersQuery.isError && (
<div className="border-red-500 rounded-sm border p-6 bg-red-50 dark:bg-red-900/20">
<p className="text-red-700 dark:text-red-300">
Error loading ledgers:{" "}
{ledgersQuery.error instanceof Error
? ledgersQuery.error.message
: "Unknown error"}
</p>
</div>
)}
{ledgersQuery.isSuccess && (
<div className="space-y-3">
{ledgersQuery.data.ledgers.length === 0 ? (
<div className="border-light-decorative-01 dark:border-dark-decorative-01 rounded-sm border p-6">
<p className="text-light-secondary dark:text-dark-secondary">
No ledgers found.
</p>
</div>
) : (
ledgersQuery.data.ledgers.map((ledger: LedgerUser) => (
<div
key={ledger.ledgerId}
className="border-light-decorative-01 dark:border-dark-decorative-01 rounded-sm border p-4"
>
<div className="flex justify-between items-start">
<div>
<h3 className="font-medium text-light-primary dark:text-dark-primary">
{ledger.name}
</h3>
<p className="text-sm text-light-secondary dark:text-dark-secondary font-mono">
ID: {ledger.ledgerId}
</p>
<p className="text-sm text-light-secondary dark:text-dark-secondary font-mono">
Tenant: {ledger.tenantId}
</p>
</div>
<div
className="text-sm text-light-secondary dark:text-dark-secondary"
>
<p>Users: {ledger.users.length}</p>
<p>Max shares: {ledger.maxShares}</p>
</div>
</div>
<div
className="mt-2 text-sm text-light-secondary dark:text-dark-secondary"
>
<p>
Created:{" "}
{new Date(ledger.createdAt).toLocaleDateString()}
</p>
</div>
{ledger.users.length > 0 && (
<div
className="mt-3 pt-3 border-t border-light-decorative-01 dark:border-dark-decorative-01"
>
<p
className="text-xs font-medium text-light-secondary dark:text-dark-secondary mb-2"
>
User Access:
</p>
<div className="flex flex-wrap gap-2">
{ledger.users.map((user, idx) => (
<span
key={idx}
className="px-2 py-1 text-xs rounded bg-gray-100 dark:bg-gray-800 text-light-secondary dark:text-dark-secondary"
>
{user.name || user.userId} ({user.role}/
{user.right})
</span>
))}
</div>
</div>
)}
</div>
))
)}
</div>
)}
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

The tenants and ledgers UI sections are nearly identical in structure: both have loading/error/empty/list states, similar card styling, and similar list rendering. This duplication increases the chance of subtle drift (e.g. one gets improved skeletons or error handling and the other doesn’t) and makes future enhancements harder.

Given this dashboard is expected to evolve, centralizing the shared list behavior would make the codebase easier to maintain and reason about.

Suggestion

Extract a small reusable list/section component that encapsulates the common pattern—accepting props like title, query, and an renderItem callback—so you only define the loading/error/empty scaffolding once. I can propose a concrete DashboardSection abstraction using your existing Tailwind classes and React Query shape if you’d like—reply with "@CharlieHelps yes please" and I’ll add a refactor patch.

@charliecreates charliecreates bot removed the request for review from CharlieHelps December 1, 2025 15:01
@mabels
Copy link
Contributor

mabels commented Dec 1, 2025

@jchris pls use the sts service from fireproof instead of doing evering again .

}

// Helper to convert Result monad to Promise for React Query
function wrapResultToPromise<T>(pro: () => Promise<Result<T>>, label: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

exception2Result from fireproof does that

);
return new DashboardApi({
apiUrl,
fetch: window.fetch.bind(window),
Copy link
Contributor

Choose a reason for hiding this comment

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

you should be able to not to pass the fetch ---

if (fpCloudToken) {
console.log("[Fireproof Dashboard] 🔑 Using existing fp-cloud-jwt.");
return {
type: "ucan" as const, // Corrected type here
Copy link
Contributor

Choose a reason for hiding this comment

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

thats is nonsensen


if (token) {
try {
const claims = decodeJwt(token);
Copy link
Contributor

Choose a reason for hiding this comment

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

use sts service from fireproof


// Check KID match for debugging
const expectedKid = "ins_35qNS5Jwyc7z4aJRBIS7o205yzb";
if (header.kid === expectedKid) {
Copy link
Contributor

Choose a reason for hiding this comment

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

never use constants in prod code

const fetchFpCloudToken = async () => {
console.log("[Fireproof Dashboard] 🚀 Attempting to get fp-cloud-jwt...");
try {
const result = await api.getCloudSessionToken({});
Copy link
Contributor

Choose a reason for hiding this comment

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

this logic is temp i will give you something better in the near future



// Query to list all tenants for the logged-in user
const tenantsQuery = useQuery<ResListTenantsByUser>({
Copy link
Contributor

Choose a reason for hiding this comment

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

should we make a dashboard-react packages so that this things are only once exist

@jchris jchris force-pushed the jchris/fireproof-dashboard-api branch from 7e6fcdd to 4978636 Compare December 1, 2025 18:20
jchris added 15 commits December 1, 2025 10:25
- Add loading state for Clerk auth initialization (!isLoaded)
- Add userId to React Query keys for proper cache scoping
- Keep using global fetch (not window.fetch)
- Simplify to just ensureUser and show success

Addresses feedback from @mabels on PR #659

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Fixes 'Illegal invocation' error by wrapping fetch call in an arrow
function to maintain proper this binding when passed to DashboardApi.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.

3 participants