Admin Editor #1619
Admin Editor #1619
Conversation
…ve to update tabOrders to not be signed fiels)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughThis PR introduces admin identity management for navigation pages by adding a new NAV_PAGE space type, extending the database schema with admin_identity_public_keys, refactoring navigation page components and data loading logic, and implementing editability checks based on admin identity membership. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant PageServer as page.tsx (Server)
participant Utils as loadNavPageSpaceData
participant Config as SystemConfig
participant Storage as Supabase Storage
participant NavPageSpace as NavPageSpace (Client)
participant PublicSpace as PublicSpace
Client->>PageServer: Request /[navSlug]/[[...tabName]]
PageServer->>Utils: loadNavPageSpaceData(navSlug, tabName)
Utils->>Config: loadSystemConfig()
Config-->>Utils: systemConfig with navItems
Utils->>Utils: Find navItem by href matching navSlug
Utils->>Storage: getDefaultTabFromStorage(spaceId)
Storage-->>Utils: tabOrder data
Utils-->>Utils: Determine activeTab (tabName or defaultTab)
Utils-->>PageServer: NavPageSpacePageData (without isEditable/spacePageUrl)
PageServer->>NavPageSpace: Render with spacePageData, tabName
NavPageSpace->>NavPageSpace: useMemo: Compute spaceDataWithClientSideLogic
NavPageSpace->>NavPageSpace: useCurrentSpaceIdentityPublicKey() for currentUserKey
NavPageSpace->>NavPageSpace: Check if currentUserKey in adminIdentityPublicKeys
NavPageSpace->>NavPageSpace: Augment spaceData with isEditable & spacePageUrl
NavPageSpace->>PublicSpace: Render with enriched spaceData
PublicSpace-->>Client: Render nav page with editability
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (15)
src/pages/api/space/registry/[spaceId]/index.ts (1)
177-204: Consider query optimization for scalability.The current implementation fetches all published configs and iterates client-side. While functional for a small number of communities, this could be optimized with a PostgreSQL JSONB containment query if the number of community configs grows significantly.
For example, using JSONB path queries:
SELECT admin_identity_public_keys FROM community_configs WHERE is_published = true AND navigation_config->'items' @> '[{"spaceId": "target-space-id"}]'::jsonbThat said, if the number of communities is expected to remain small, the current approach is acceptable.
src/app/page.tsx (2)
16-34: Inconsistent indentation in try-catch block.The code has mixed indentation levels that hurt readability. Lines 20-21 use extra indentation, while lines 23-31 have misaligned braces and statements.
if (homeNavItem?.spaceId) { try { // Tab order is stored directly (not wrapped in SignedFile) const { data: tabOrderData } = await createSupabaseServerClient() .storage - .from('spaces') - .download(`${homeNavItem.spaceId}/tabOrder`); + .from('spaces') + .download(`${homeNavItem.spaceId}/tabOrder`); - if (tabOrderData) { + if (tabOrderData) { const tabOrderJson = JSON.parse(await tabOrderData.text()); const defaultTab = tabOrderJson.tabOrder?.[0]; - if (defaultTab) { - redirect(`/home/${encodeURIComponent(defaultTab)}`); - return null; - } + if (defaultTab) { + redirect(`/home/${encodeURIComponent(defaultTab)}`); + return null; } - } catch (error) { - console.warn('Failed to load home space default tab:', error); + } + } catch (error) { + console.warn('Failed to load home space default tab:', error); } }
24-25: Consider adding type validation for parsed JSON.The parsed
tabOrderJsonis used without type validation. If the storage data is malformed or has an unexpected structure, accessingtabOrderJson.tabOrder?.[0]could produce unexpected results.Consider adding a type guard or basic validation:
const tabOrderJson = JSON.parse(await tabOrderData.text()); if (Array.isArray(tabOrderJson?.tabOrder) && typeof tabOrderJson.tabOrder[0] === 'string') { const defaultTab = tabOrderJson.tabOrder[0]; // ... }src/common/types/spaceData.ts (1)
77-83: Consider adding adefaultTaboverride for consistency.Other space types like
ProfileSpacePageData,TokenSpacePageData,ProposalSpacePageData, andChannelSpacePageDataall define a specificdefaultTabvalue (e.g.,'Profile','Token','Overview','Channel').NavPageSpacePageDatarelies on the baseSpacePageData.defaultTab: stringwithout specifying a default.If nav pages have a conventional default tab name, consider adding it for consistency:
export interface NavPageSpacePageData extends SpacePageData { spaceType: typeof SPACE_TYPES.NAV_PAGE; + defaultTab: 'Main'; // or whatever the conventional default is // navSlug is the URL path segment (e.g., 'home', 'explore') navSlug: string; // Admin identity public keys that can edit this space adminIdentityPublicKeys: string[]; }docs/SYSTEMS/CONFIGURATION/ADMIN_API.md (1)
30-37: Add blank lines around tables for markdown compliance.Per markdown best practices (MD058), tables should be surrounded by blank lines.
**Query Parameters:** + | Param | Required | Description | |-------|----------|-------------| | `communityId` | Yes | Community ID to fetch config for | | `sections` | No | Comma-separated list of sections (e.g., `brand,navigation`) | | `publicKey` | No | For admin auth - include to get admin-only fields | | `signature` | No | For admin auth | | `timestamp` | No | For admin auth | + **Response:**Similar formatting should be applied to tables at lines 226-231 and 237-245.
src/pages/api/admin/config/index.ts (3)
25-25: Remove unused import.
pickis imported from lodash but never used in this file.-import { isArray, pick, merge } from "lodash"; +import { isArray, merge } from "lodash";
83-94: Validate section names before casting.User-provided section names are cast directly to
ConfigSection[]without validation. Invalid section names will pass through and be silently ignored due to theif (column && column in config)check, but explicit validation would be cleaner.if (sections && !isArray(sections)) { - const requestedSections = sections.split(",").map(s => s.trim()) as ConfigSection[]; + const allSections = Object.keys(SECTION_TO_COLUMN) as ConfigSection[]; + const requestedSections = sections + .split(",") + .map(s => s.trim()) + .filter((s): s is ConfigSection => allSections.includes(s as ConfigSection)); + for (const section of requestedSections) { const column = SECTION_TO_COLUMN[section]; - if (column && column in config) { + if (column in config) { // Only include adminIdentityPublicKeys if admin if (section === "adminIdentityPublicKeys" && !isAdminRequest) { continue; } result[section] = (config as any)[column]; } }
66-78: Silent failure on invalid admin credentials.When admin auth parameters are provided but validation fails (invalid signature or non-admin key), the request silently falls back to public access. Consider logging this for audit/debugging purposes.
// Check if this is an admin request (has auth params) let isAdminRequest = false; if (publicKey && signature && timestamp && !isArray(publicKey)) { const authRequest = { publicKey, signature, timestamp, communityId, }; if (isSignable(authRequest) && validateSignable(authRequest)) { isAdminRequest = await identityIsAdmin(publicKey, communityId); + } else { + console.warn('Admin auth params provided but signature validation failed'); } }src/app/[navSlug]/[[...tabName]]/page.tsx (1)
14-30: Consider logging or distinguishing error types in getDefaultTab.The function catches all errors and returns
null, making it impossible to distinguish between "no tabs exist" vs "storage/parsing error". This could mask issues in production.async function getDefaultTab(spaceId: string): Promise<string | null> { try { const { data: tabOrderData, error } = await createSupabaseServerClient() .storage .from('spaces') .download(`${spaceId}/tabOrder`); if (error || !tabOrderData) { + // Storage fetch failed or no data - this is expected for new spaces return null; } const tabOrderJson = JSON.parse(await tabOrderData.text()); return tabOrderJson.tabOrder?.[0] || null; - } catch { + } catch (e) { + console.warn(`[getDefaultTab] Failed to parse tab order for ${spaceId}:`, e); return null; } }src/pages/api/admin/config/navigation/index.ts (1)
81-109: Incomplete rollback: tabs folder may not be removed on orderError.When
orderErroroccurs (Line 104), the rollback removes the tab file and space registration, but doesn't attempt to remove thetabsfolder itself. While this may not cause functional issues, it leaves orphaned empty folders in storage. Also, consider that the rollback operations themselves could fail silently.if (orderError) { // Rollback - await supabase.storage.from("spaces").remove([`${spaceId}/tabs/${initialTabName}`]); + // Best-effort cleanup - log but don't fail on rollback errors + const { error: tabCleanupError } = await supabase.storage.from("spaces").remove([`${spaceId}/tabs/${initialTabName}`]); + if (tabCleanupError) { + console.warn(`[createNavPageSpace] Rollback tab cleanup failed: ${tabCleanupError.message}`); + } await supabase.from("spaceRegistrations").delete().eq("spaceId", spaceId); return { error: `Failed to create tab order: ${orderError.message}` }; }src/pages/api/admin/config/navigation/[itemId].ts (2)
30-70: Storage deletion may miss deeply nested folders or fail silently.The
deleteNavPageSpacefunction lists files at the space root and intabs/, but doesn't recursively handle other potential nested folders. Additionally, the storage list/remove operations don't check for errors (except for the final registration deletion). If the storage bucket has additional structure or if listing fails, files could be orphaned.async function deleteNavPageSpace(spaceId: string): Promise<{ success: boolean; error?: string }> { const supabase = createSupabaseServerClient(); // List and delete all files in the space - const { data: files } = await supabase.storage + const { data: files, error: listError } = await supabase.storage .from("spaces") .list(spaceId); + if (listError) { + console.warn(`[deleteNavPageSpace] Failed to list files for ${spaceId}: ${listError.message}`); + // Continue with registration deletion anyway + } + if (files && files.length > 0) { // Delete tabs folder contents - const { data: tabFiles } = await supabase.storage + const { data: tabFiles, error: tabListError } = await supabase.storage .from("spaces") .list(`${spaceId}/tabs`); + if (tabListError) { + console.warn(`[deleteNavPageSpace] Failed to list tabs for ${spaceId}: ${tabListError.message}`); + } + if (tabFiles && tabFiles.length > 0) { const tabPaths = tabFiles.map(f => `${spaceId}/tabs/${f.name}`); - await supabase.storage.from("spaces").remove(tabPaths); + const { error: tabRemoveError } = await supabase.storage.from("spaces").remove(tabPaths); + if (tabRemoveError) { + console.warn(`[deleteNavPageSpace] Failed to remove tabs: ${tabRemoveError.message}`); + } }
131-136: Potential type safety issue with spread operator on updates.Spreading
updatesdirectly could introduce unexpected fields if the type guard doesn't strictly validate the updates object's shape. Whileidis preserved, other unexpected fields might leak through.// Apply updates (don't allow changing the ID) const updatedItem: NavigationItem = { ...currentItems[itemIndex], - ...updates, + label: updates.label ?? currentItems[itemIndex].label, + href: updates.href ?? currentItems[itemIndex].href, + icon: updates.icon ?? currentItems[itemIndex].icon, + openInNewTab: updates.openInNewTab ?? currentItems[itemIndex].openInNewTab, + requiresAuth: updates.requiresAuth ?? currentItems[itemIndex].requiresAuth, + spaceId: updates.spaceId ?? currentItems[itemIndex].spaceId, id: itemId, // Preserve original ID };Alternatively, validate the
updatesobject more strictly inisUpdateNavItemRequestto only allow known NavigationItem fields.src/pages/api/admin/config/_helpers.ts (3)
210-215: Unsafe type assertion withas any.The return statement casts data to
any, bypassing TypeScript's type safety. Consider using a more specific type assertion or mapping the database fields explicitly.if (error || !data) { return null; } - return data as any; + // Map database fields to expected return type + return { + id: data.id, + brand_config: data.brand_config as BrandConfig | null, + assets_config: data.assets_config as AssetConfig | null, + community_config: data.community_config as CommunityConfig | null, + fidgets_config: data.fidgets_config as FidgetConfig | null, + navigation_config: data.navigation_config as NavigationConfig | null, + ui_config: data.ui_config as UIConfig | null, + admin_identity_public_keys: data.admin_identity_public_keys as string[] | null, + };
262-302: Consider adding timestamp replay protection.The
validateAdminRequestfunction validates the signature but doesn't check if thetimestampis recent. This could allow replay attacks where a valid signed request is captured and re-submitted later. Consider adding a timestamp freshness check (e.g., within 5 minutes).export async function validateAdminRequest<T extends AdminConfigRequest>( request: unknown, typeGuard: (r: unknown) => r is T, res: NextApiResponse<NounspaceResponse>, requiredFields: string[] = [], ): Promise<T | null> { // Check structure if (!typeGuard(request)) { res.status(400).json({ result: "error", error: { message: `Request must include: publicKey, timestamp, communityId, signature${requiredFields.length ? `, ${requiredFields.join(", ")}` : ""}`, }, }); return null; } + // Check timestamp freshness (prevent replay attacks) + const requestTime = new Date(request.timestamp).getTime(); + const now = Date.now(); + const MAX_AGE_MS = 5 * 60 * 1000; // 5 minutes + if (isNaN(requestTime) || Math.abs(now - requestTime) > MAX_AGE_MS) { + res.status(400).json({ + result: "error", + error: { + message: "Request timestamp is invalid or expired", + }, + }); + return null; + } + // Validate signature if (!validateSignable(request)) {
241-252: updateCommunityConfig doesn't verify the update affected any rows.The update operation doesn't check if any rows were actually updated. If the
community_iddoesn't exist oris_publishedis false, the update silently succeeds without modifying data. Consider checking the affected row count.- const { error } = await supabase + const { error, count } = await supabase .from("community_configs") .update(updateData) .eq("community_id", communityId) - .eq("is_published", true); + .eq("is_published", true) + .select('id', { count: 'exact', head: true }); if (error) { return { success: false, error: error.message }; } + if (count === 0) { + return { success: false, error: "No published config found for community" }; + } + return { success: true };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (18)
docs/SYSTEMS/CONFIGURATION/ADMIN_API.md(1 hunks)docs/SYSTEMS/CONFIGURATION/ARCHITECTURE_OVERVIEW.md(2 hunks)package.json(1 hunks)scripts/seed.ts(6 hunks)src/app/[navSlug]/[[...tabName]]/NavPageClient.tsx(0 hunks)src/app/[navSlug]/[[...tabName]]/NavPageSpace.tsx(1 hunks)src/app/[navSlug]/[[...tabName]]/page.tsx(3 hunks)src/app/page.tsx(2 hunks)src/common/types/spaceData.ts(3 hunks)src/config/systemConfig.ts(1 hunks)src/pages/api/admin/config/_helpers.ts(1 hunks)src/pages/api/admin/config/index.ts(1 hunks)src/pages/api/admin/config/navigation/[itemId].ts(1 hunks)src/pages/api/admin/config/navigation/index.ts(1 hunks)src/pages/api/space/registry/[spaceId]/index.ts(2 hunks)src/supabase/database.d.ts(1 hunks)supabase/migrations/20251209000000_add_admin_identity_public_keys.sql(1 hunks)supabase/migrations/20251209000001_add_community_configs_rls.sql(1 hunks)
💤 Files with no reviewable changes (1)
- src/app/[navSlug]/[[...tabName]]/NavPageClient.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-28T21:31:10.654Z
Learnt from: CR
Repo: Nounspace/nounspace.ts PR: 0
File: docs/DEVELOPMENT/AGENTS.md:0-0
Timestamp: 2025-11-28T21:31:10.654Z
Learning: Review PUBLIC_SPACES_PATTERN.md documentation when touching shared/public space layouts, publishing tools, or visibility rules
Applied to files:
src/app/[navSlug]/[[...tabName]]/NavPageSpace.tsx
🧬 Code graph analysis (6)
src/app/[navSlug]/[[...tabName]]/NavPageSpace.tsx (3)
src/common/types/spaceData.ts (1)
NavPageSpacePageData(77-83)src/common/lib/hooks/useCurrentSpaceIdentityPublicKey.ts (1)
useCurrentSpaceIdentityPublicKey(3-5)src/app/(spaces)/PublicSpace.tsx (1)
PublicSpace(33-463)
src/pages/api/admin/config/index.ts (3)
src/pages/api/admin/config/_helpers.ts (7)
ConfigSection(41-48)errorResponse(315-324)getCommunityConfig(190-215)identityIsAdmin(167-185)successResponse(308-313)isUpdateConfigRequest(115-121)updateCommunityConfig(220-252)src/common/data/api/requestHandler.ts (2)
NounspaceResponse(9-16)requestHandler(31-51)src/common/lib/signedFiles.ts (2)
isSignable(29-41)validateSignable(43-52)
src/pages/api/admin/config/navigation/index.ts (5)
src/common/data/database/supabase/clients/server.ts (1)
createSupabaseServerClient(7-7)src/common/lib/signedFiles.ts (1)
SignedFile(16-18)src/common/data/api/requestHandler.ts (2)
NounspaceResponse(9-16)requestHandler(31-51)src/pages/api/admin/config/_helpers.ts (7)
validateAdminRequest(262-302)isAddNavItemRequest(123-133)getCommunityConfig(190-215)errorResponse(315-324)updateCommunityConfig(220-252)successResponse(308-313)isReorderNavItemsRequest(151-157)src/config/systemConfig.ts (2)
NavigationConfig(155-160)NavigationItem(167-175)
src/app/[navSlug]/[[...tabName]]/page.tsx (4)
src/common/data/database/supabase/clients/server.ts (1)
createSupabaseServerClient(7-7)src/common/types/spaceData.ts (2)
NavPageSpacePageData(77-83)SPACE_TYPES(7-13)src/config/index.ts (1)
INITIAL_SPACE_CONFIG_EMPTY(94-94)src/app/[navSlug]/[[...tabName]]/NavPageSpace.tsx (1)
NavPageSpace(52-83)
src/app/page.tsx (1)
src/common/data/database/supabase/clients/server.ts (1)
createSupabaseServerClient(7-7)
src/pages/api/space/registry/[spaceId]/index.ts (1)
src/common/data/database/supabase/clients/server.ts (1)
createSupabaseServerClient(7-7)
🪛 markdownlint-cli2 (0.18.1)
docs/SYSTEMS/CONFIGURATION/ADMIN_API.md
31-31: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🔇 Additional comments (27)
package.json (1)
157-157: Verify dotenv placement: should it also be in dependencies for server-side code?The dotenv package is being added to devDependencies. However, the PR introduces server-side admin config APIs and environment-dependent configurations. Please clarify whether dotenv should also be added to
dependencies(for runtime server code) or ifdevDependencies(for the seed script) is sufficient.Next.js has built-in .env file support, so dotenv may only be needed for custom scripts or server code that runs outside Next.js's environment-loading mechanism. Confirm the full scope of dotenv usage across the PR changes.
The version
^17.2.3is appropriate.scripts/seed.ts (4)
30-44: LGTM! Good practice for environment loading in scripts.The prioritized
.envfile loading with fallback is a solid approach for local development scripts. The logging of which file was loaded aids debugging.
262-270: Helpful inline documentation for admin key setup.The comments explaining how to obtain
identityPublicKeyvalues from browser devtools (localStorage.getItem('nounspace-app-store')) provide useful guidance for developers setting up admin access.
655-667: Good refactor to align with app's expected format.The function now correctly returns a plain data object rather than a
SignedFilewrapper, matching the app's behavior where tab order is saved directly as a signed request object. The updated comments clearly document this distinction.
693-701: LGTM!The upload logic correctly uses the new
createTabOrderDatafunction and serializes the data directly without SignedFile wrapping.docs/SYSTEMS/CONFIGURATION/ARCHITECTURE_OVERVIEW.md (2)
528-534: Good documentation additions for Admin API.The new section properly documents the admin API file structure and references the detailed documentation in
ADMIN_API.md. This follows the existing documentation patterns in the architecture overview.
545-545: LGTM!The updated note correctly reflects that Admin UI can now be built using the Admin Config API endpoints that are ready for use.
src/config/systemConfig.ts (1)
35-35: LGTM!The new optional
adminIdentityPublicKeysproperty is well-placed in theSystemConfiginterface and clearly documented with its purpose. This aligns with the database schema additions.supabase/migrations/20251209000001_add_community_configs_rls.sql (3)
1-19: Well-structured RLS policies.The policies correctly implement:
- Public read access limited to published configs only
- Full access for service role (needed for seeding and admin operations)
The implicit denial of INSERT/UPDATE/DELETE for anon/authenticated roles is the secure default.
33-64: Useful documentation of future admin RPC pattern.The commented-out
update_community_config_as_adminfunction provides a good template for future client-side admin operations usingSECURITY DEFINERwith identity verification. This is a sound approach for authenticated admin modifications without exposing the service role key.
24-26: The comment accurately describes theget_active_community_configfunction. Verification confirms the function definition includesSECURITY DEFINER(present in supabase/migrations/20251209000000_add_admin_identity_public_keys.sql line 13 and supabase/migrations/20251129172847_create_community_configs.sql line 25), which correctly allows the function to bypass RLS and read configs regardless of the calling user's role.src/pages/api/space/registry/[spaceId]/index.ts (2)
140-152: LGTM! Good extension for navPage admin permissions.The logic correctly prioritizes admin identity checks for
navPagespaces before falling through to the existing permission resolution (contract-based, fid-based, or identityPublicKey-based). The early return when admin identities are found is appropriate.
193-200: Type assertions are reasonable but verify data integrity.The type casts on JSONB fields are necessary. The defensive null checks (
navConfig?.items,config.admin_identity_public_keys) provide adequate runtime safety. Consider adding validation if you encounter malformed config data in production.src/common/types/spaceData.ts (1)
110-112: LGTM!The type guard follows the established pattern used by other space type guards.
docs/SYSTEMS/CONFIGURATION/ADMIN_API.md (1)
1-304: Documentation is comprehensive and well-structured.The API documentation thoroughly covers authentication patterns, all endpoints with request/response schemas, error handling, and includes practical usage examples.
src/supabase/database.d.ts (1)
186-230: Type definitions align well with the database schema.The
community_configstable type correctly defines:
- Required fields (
community_id,is_published) vs optional ones- Auto-generated
id(optional in Insert)- Nullable config JSON fields and
admin_identity_public_keys- Timestamp fields as strings
src/pages/api/admin/config/index.ts (1)
134-202: PATCH handler implementation looks solid.The update logic correctly:
- Validates admin authorization via
validateAdminRequest- Deep-merges partial updates with existing config
- Replaces (rather than merges)
adminIdentityPublicKeysarray- Returns the full updated config
supabase/migrations/20251209000000_add_admin_identity_public_keys.sql (1)
1-40: Migration is well-structured and idempotent.The migration correctly adds the column with
IF NOT EXISTSand updates the function withCREATE OR REPLACE. The SECURITY DEFINER is appropriate for controlled access to config data.Regarding the index concern: separate indexes already exist on
community_idandis_published(from the initial table creation migration20251129172847_create_community_configs.sql). However, theget_active_community_configfunction queries both columns together (WHERE "community_id" = p_community_id AND "is_published" = true). Consider adding a composite index on(community_id, is_published)for optimal query performance, though the existing separate indexes may be sufficient depending on table selectivity.src/app/[navSlug]/[[...tabName]]/NavPageSpace.tsx (3)
38-50: LGTM! Clean editability helper.The
isNavPageSpaceEditablefunction correctly implements the authorization logic - requiring a logged-in user and checking membership in the admin list. The early return pattern is clean.
65-75: LGTM! Well-structured client-side data composition.The
useMemocorrectly enriches server data with client-side logic. ThespacePageUrlproperly encodes tab names, and theisEditableclosure captures the current user's identity key appropriately. Good documentation on why_currentUserFidis ignored.
59-62: Remove this concern — the code follows the recommended pattern for onchainkit's useMiniKit hook.
setFrameReadyis idempotent and designed to be called once. The guardif (!isFrameReady)prevents unnecessary invocations, and this exact pattern is the recommended approach in onchainkit documentation. No changes needed.src/app/[navSlug]/[[...tabName]]/page.tsx (1)
36-54: LGTM! Clean space data factory.The
createNavPageSpaceDatafunction follows the established pattern from ProfileSpace. SettingspaceOwnerFid: undefinedis correct since NavPages don't have FID-based ownership, and usingINITIAL_SPACE_CONFIG_EMPTYallows PublicSpace to load the actual config from storage.src/pages/api/admin/config/navigation/index.ts (3)
162-165: LGTM! Good duplicate ID validation.Checking for existing item IDs before insertion prevents duplicate navigation entries and provides a clear error message.
260-277: LGTM! Graceful partial reorder handling.The reordering logic correctly handles partial reorders by preserving items not included in
itemIdsat the end. Using a Map for O(1) lookups is efficient.
63-70: The hardcoded signature and publicKey values do not interfere with any validation logic. These machine-generated files are loaded back throughisSignedFile()(which only performs structural type checking) anddecryptEncryptedSignedFile()(which performs decryption without signature validation). Throughout the codebase,validateSignable()is never called on files created by this endpoint—only on user-submitted updates to tabs or tabOrder. The initial tab files remain safe as-is.src/pages/api/admin/config/navigation/[itemId].ts (1)
214-224: LGTM! Good optional space cleanup on delete.The delete handler correctly checks both
deleteSpaceflag anditem.spaceIdexistence before attempting space deletion, and properly handles errors from the space deletion operation.src/pages/api/admin/config/_helpers.ts (1)
106-113: LGTM! Comprehensive type guard.The
isAdminConfigRequesttype guard properly validates all required fields and delegates signable validation toisSignable.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/common/data/stores/app/space/spaceStore.ts (1)
535-538: Bug:remoteSpaces.orderuses wrong source array.Line 536 filters
draft.space.localSpaces[spaceId].orderwhen updatingremoteSpaces.order, but it should filter fromremoteSpaces.orderto maintain consistency.draft.space.remoteSpaces[spaceId].order = filter( - draft.space.localSpaces[spaceId].order, + draft.space.remoteSpaces[spaceId].order, (x) => x !== tabName, );
🧹 Nitpick comments (11)
src/common/components/organisms/NavigationEditItem.tsx (1)
88-99: Consider adding keyboard accessibility for the delete action.The delete button is only visible when selected and not shrunk. For better accessibility, consider adding
aria-labelor ensuring keyboard users can access this action when the item is focused.<button onClick={(e) => { e.stopPropagation(); onDelete(); }} className="p-1 text-red-500 hover:text-red-700 hover:bg-red-50 rounded flex-shrink-0" - title="Delete navigation item" + title="Delete navigation item" + aria-label={`Delete ${item.label}`} > <FaTrash size={12} /> </button>src/pages/api/admin/config/navigation/index.ts (1)
263-269: Consider limiting the size ofitemIdsarray to prevent abuse.Large arrays could cause performance issues. A reasonable upper bound (e.g., 100 items) would prevent potential DoS vectors.
const { communityId, itemIds } = request; + // Prevent excessively large reorder requests + if (itemIds.length > 100) { + return errorResponse(res, "Too many items in reorder request (max 100)"); + } + // Get current config const currentConfig = await getCommunityConfig(communityId);src/app/[navSlug]/[[...tabName]]/page.tsx (1)
83-89: Removeundefined as any(unnecessary + masks type issues)On Line 84,
createNavPageSpaceDataalready acceptsstring | undefined, so you can just passundefinedand keep type-safety.- undefined as any, // Will be set by client if pending + undefined, // Will be set by client if pendingsrc/common/components/organisms/Navigation.tsx (1)
150-152: Community ID derivation frombrand.nameis fragileOn Line 150-151,
brand.name.toLowerCase()can introduce spaces/punctuation and isn’t a stable identifier. Prefer a dedicatedcommunityIdfield from config (or slugify here similarly to the store) to avoid mismatches across environments.src/app/(spaces)/PublicSpace.tsx (1)
149-166: Consider downgrading expected “pending space” load failures fromconsole.errorOn Line 160-162 you note this is expected for pending spaces, but it still emits
console.error, which will look like real breakage in logs.src/common/data/stores/app/navigation/navigationStore.ts (2)
146-170: Use a collision-resistant nav item idOn Line 148,
nav-${Date.now()}can collide (fast clicks, multiple tabs). Since you already rely oncrypto.randomUUID()(Line 162), consider using it forNavigationItem.idtoo.
396-399: Preferrouter.refresh()overwindow.location.reload()in Next appReload works, but it’s a hard refresh. If feasible, refresh data via Next navigation APIs from the component layer after
commitChanges()resolves.src/pages/api/space/registry/index.ts (1)
62-83: De-duplicate fid parsing/validation
validateSpaceRegistrationFieldsallowsfidas string/number (Line 66-68, 81-83) and thenregisterNewSpacere-parses/validates again (Line 190-200). Consider normalizingfidonce (e.g.,const fid = coerceFid(registration.fid)) to keep behavior consistent.Also applies to: 190-205
src/common/data/stores/app/space/spaceStore.ts (3)
1495-1496: Inconsistent timestamp creation method.Line 1495 uses
new Date().toISOString()while the rest of the file consistently usesmoment().toISOString(). Consider usingmoment()for consistency.const registration = { spaceId, spaceName: spaceId, // Use spaceId as name (navPage spaces don't have a separate name) identityPublicKey: identity.rootKeys.publicKey, spaceType: SPACE_TYPES.NAV_PAGE, - timestamp: new Date().toISOString(), + timestamp: moment().toISOString(), };
1531-1544: Consider surfacing partial commit failures.The current implementation silently continues when individual tab commits fail. While this resilience is good, callers won't know if some tabs failed. Consider returning a result object with success/failure status for each tab, or at minimum, accumulating errors to throw at the end.
+ // Track failures + const failures: { tabName: string; error: unknown }[] = []; + // Commit each tab for (const tabName of localSpace.order) { if (localSpace.tabs[tabName]) { try { await get().space.commitSpaceTabToDatabase(spaceId, tabName, network); } catch (e) { console.error(`Failed to commit tab ${tabName} for space ${spaceId}:`, e); - // Continue with other tabs + failures.push({ tabName, error: e }); } } } // Commit tab order await get().space.commitSpaceOrderToDatabase(spaceId, network as EtherScanChainName); + + if (failures.length > 0) { + console.warn(`commitAllSpaceChanges completed with ${failures.length} tab failures`); + }
822-840: Consider propagatingspaceTypewhen loading remote space data.When initializing space entries in
loadSpaceTab,spaceTypeis not populated. If the remote data or space registry provides type information, consider including it here for consistency with registration flows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/app/(spaces)/PublicSpace.tsx(7 hunks)src/app/[navSlug]/[[...tabName]]/NavPageSpace.tsx(1 hunks)src/app/[navSlug]/[[...tabName]]/page.tsx(3 hunks)src/common/components/organisms/Navigation.tsx(6 hunks)src/common/components/organisms/NavigationEditItem.tsx(1 hunks)src/common/data/stores/app/index.tsx(3 hunks)src/common/data/stores/app/navigation/navigationStore.ts(1 hunks)src/common/data/stores/app/space/spaceStore.ts(21 hunks)src/common/lib/hooks/useIsAdmin.ts(1 hunks)src/pages/api/admin/config/navigation/index.ts(1 hunks)src/pages/api/space/registry/index.ts(7 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-11-28T21:31:10.654Z
Learnt from: CR
Repo: Nounspace/nounspace.ts PR: 0
File: docs/DEVELOPMENT/AGENTS.md:0-0
Timestamp: 2025-11-28T21:31:10.654Z
Learning: Applies to docs/DEVELOPMENT/src/**/store*.{ts,tsx} : Model Zustand stores with clearly typed `StoreState` + `StoreActions`, default state objects, and mutative updates inside `set` callbacks
Applied to files:
src/common/data/stores/app/index.tsxsrc/common/data/stores/app/navigation/navigationStore.tssrc/common/data/stores/app/space/spaceStore.ts
📚 Learning: 2025-06-05T19:55:02.560Z
Learnt from: willyogo
Repo: Nounspace/nounspace.ts PR: 1118
File: src/common/fidgets/FidgetWrapper.tsx:160-175
Timestamp: 2025-06-05T19:55:02.560Z
Learning: In the Nounspace.ts codebase, fidget edit mode icons in FidgetWrapper.tsx require an extremely high z-index (999999) to prevent being covered by the tab bar. Lower z-index values like 1050 break this fix, indicating other UI elements use very high z-index values.
Applied to files:
src/app/[navSlug]/[[...tabName]]/page.tsx
📚 Learning: 2025-06-05T19:55:02.560Z
Learnt from: willyogo
Repo: Nounspace/nounspace.ts PR: 1118
File: src/common/fidgets/FidgetWrapper.tsx:160-175
Timestamp: 2025-06-05T19:55:02.560Z
Learning: In the Nounspace.ts codebase, reducing the z-index of fidget edit mode icons from 999999 to 1050 breaks the fix that prevents them from being covered by UI elements. The high z-index value of 999999 appears to be necessary despite seeming excessive.
Applied to files:
src/app/[navSlug]/[[...tabName]]/page.tsx
📚 Learning: 2025-11-28T21:31:10.654Z
Learnt from: CR
Repo: Nounspace/nounspace.ts PR: 0
File: docs/DEVELOPMENT/AGENTS.md:0-0
Timestamp: 2025-11-28T21:31:10.654Z
Learning: Review PUBLIC_SPACES_PATTERN.md documentation when touching shared/public space layouts, publishing tools, or visibility rules
Applied to files:
src/app/[navSlug]/[[...tabName]]/NavPageSpace.tsxsrc/app/(spaces)/PublicSpace.tsx
📚 Learning: 2025-11-28T21:31:10.654Z
Learnt from: CR
Repo: Nounspace/nounspace.ts PR: 0
File: docs/DEVELOPMENT/AGENTS.md:0-0
Timestamp: 2025-11-28T21:31:10.654Z
Learning: Review SPACE_ARCHITECTURE.md documentation for any work on space loading, persistence, layout editing, or cross-device behaviours
Applied to files:
src/app/[navSlug]/[[...tabName]]/NavPageSpace.tsxsrc/app/(spaces)/PublicSpace.tsxsrc/common/data/stores/app/space/spaceStore.ts
📚 Learning: 2025-11-28T21:31:10.654Z
Learnt from: CR
Repo: Nounspace/nounspace.ts PR: 0
File: docs/DEVELOPMENT/AGENTS.md:0-0
Timestamp: 2025-11-28T21:31:10.654Z
Learning: Applies to docs/DEVELOPMENT/src/**/*.{ts,tsx} : Follow the atomic design hierarchy for UI components: organize components into `atoms`, `molecules`, `organisms`, `templates`, and `pages`
Applied to files:
src/common/components/organisms/Navigation.tsx
🧬 Code graph analysis (8)
src/common/data/stores/app/index.tsx (1)
src/common/data/stores/app/navigation/navigationStore.ts (2)
NavigationStore(72-72)createNavigationStoreFunc(95-416)
src/common/lib/hooks/useIsAdmin.ts (1)
src/common/lib/hooks/useCurrentSpaceIdentityPublicKey.ts (1)
useCurrentSpaceIdentityPublicKey(3-5)
src/common/data/stores/app/navigation/navigationStore.ts (5)
src/config/systemConfig.ts (2)
NavigationItem(167-175)NavigationConfig(155-160)src/common/data/stores/createStore.ts (2)
StoreSet(51-51)StoreGet(52-52)src/common/data/stores/app/index.tsx (1)
AppStore(43-56)src/common/types/spaceData.ts (1)
SPACE_TYPES(7-13)src/common/lib/signedFiles.ts (1)
signSignable(54-62)
src/app/(spaces)/PublicSpace.tsx (3)
src/common/types/spaceData.ts (1)
isTokenSpace(98-100)src/constants/etherscanChainIds.ts (1)
EtherScanChainName(1-1)src/config/index.ts (1)
INITIAL_SPACE_CONFIG_EMPTY(94-94)
src/pages/api/space/registry/index.ts (3)
src/common/types/spaceData.ts (2)
SpaceTypeValue(16-16)SPACE_TYPES(7-13)src/app/(spaces)/Space.tsx (1)
SpaceConfig(37-49)src/common/lib/signedFiles.ts (1)
isSignable(29-41)
src/common/data/stores/app/space/spaceStore.ts (5)
src/common/types/spaceData.ts (2)
SpaceTypeValue(16-16)SPACE_TYPES(7-13)src/pages/api/space/registry/[spaceId]/index.ts (1)
UnsignedUpdateTabOrderRequest(22-28)src/pages/api/space/registry/index.ts (1)
SpaceRegistration(22-37)src/common/lib/signedFiles.ts (1)
signSignable(54-62)src/constants/etherscanChainIds.ts (1)
EtherScanChainName(1-1)
src/common/components/organisms/NavigationEditItem.tsx (2)
src/config/systemConfig.ts (1)
NavigationItem(167-175)src/common/lib/utils/mergeClasses.ts (1)
mergeClasses(4-6)
src/common/components/organisms/Navigation.tsx (4)
src/common/lib/theme/UserThemeProvider.tsx (1)
useUserTheme(14-57)src/common/lib/hooks/useIsAdmin.ts (1)
useIsAdmin(16-27)src/common/lib/hooks/useNotificationBadgeText.ts (1)
useNotificationBadgeText(15-61)src/common/components/organisms/NavigationEditItem.tsx (1)
NavigationEditItem(22-103)
🪛 Biome (2.1.2)
src/app/[navSlug]/[[...tabName]]/NavPageSpace.tsx
[error] 107-107: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 119-119: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🔇 Additional comments (20)
src/common/data/stores/app/index.tsx (3)
35-38: LGTM!The import follows the established pattern for other store slices (AccountStore, SpaceStore, ChatStore, etc.).
51-51: LGTM!The
navigation: NavigationStorefield correctly extends the AppStore type to include the new navigation subsystem.
68-68: Correct: Navigation store is intentionally not persisted.The navigation store is initialized but not included in
partialize(lines 108-114). This is appropriate since navigation edit state is ephemeral (edit mode, local items, selection) and should not survive page reloads. Based on learnings, this follows the Zustand store pattern with clearly typed state and actions.src/common/lib/hooks/useIsAdmin.ts (1)
16-27: LGTM!The hook implementation is clean and correctly memoized. It properly:
- Guards against missing identity keys
- Uses appropriate dependencies in the useMemo array
- Returns a boolean for straightforward consumption
src/common/components/organisms/NavigationEditItem.tsx (1)
34-41: LGTM!The rename handler correctly validates by trimming whitespace and checking for actual changes before propagating the update.
src/pages/api/admin/config/navigation/index.ts (2)
171-174: LGTM!Good defensive check for duplicate item IDs before insertion, preventing data integrity issues.
90-94: Await rollback operations to ensure cleanup completes.The rollback delete operation is not awaited, which could leave orphaned data if the response returns before cleanup finishes. While failures here are rare, awaiting ensures consistency.
if (tabError) { // Rollback space registration - await supabase.from("spaceRegistrations").delete().eq("spaceId", spaceId); + await supabase.from("spaceRegistrations").delete().eq("spaceId", spaceId); return { error: `Failed to create initial tab: ${tabError.message}` }; }The current code does await, but the rollback at lines 115-116 runs two operations that could be parallelized:
if (orderError) { // Rollback - await supabase.storage.from("spaces").remove([`${spaceId}/tabs/${initialTabName}`]); - await supabase.from("spaceRegistrations").delete().eq("spaceId", spaceId); + await Promise.all([ + supabase.storage.from("spaces").remove([`${spaceId}/tabs/${initialTabName}`]), + supabase.from("spaceRegistrations").delete().eq("spaceId", spaceId), + ]); return { error: `Failed to create tab order: ${orderError.message}` }; }Likely an incorrect or invalid review comment.
src/app/[navSlug]/[[...tabName]]/NavPageSpace.tsx (1)
48-60: LGTM!The
isNavPageSpaceEditablehelper function is a clean, pure function that correctly checks admin status by comparing the user's identity key against the admin list.src/common/components/organisms/Navigation.tsx (1)
524-543: Reorder wiring looks consistent (ids end-to-end)
valuesare ids (Line 526-528) andNavigationEditItemusesReorder.Item value={item.id}(per provided snippet), so the store receivingstring[]is coherent.src/app/(spaces)/PublicSpace.tsx (2)
389-438: Good: explicitcurrentSpaceIdguards on tab mutationsThe early returns (Line 390-405, 421-437) avoid accidental mutations when viewing an unregistered/pending space.
319-341: Good: propagatespaceTypeintosaveLocalSpaceTabPassing
spacePageData.spaceType(Lines 338, 366) makes the persisted space type explicit. The parameter is correctly used in the store to set or initializespaceTypeon local space entries, ensuring NAV_PAGE flows and other space types remain consistent.src/pages/api/space/registry/index.ts (1)
299-307: Response normalization looks consistent with nullable fieldsThe mapping to explicit
null(Line 300-307) makes the API response shape predictable for clients.src/app/[navSlug]/[[...tabName]]/page.tsx (1)
16-20: No action needed. The module at@/common/data/database/supabase/clients/serverexportscreateSupabaseServerClientas both a named export (line 7) and a default export (line 9), so both import styles are valid and widely used throughout the codebase without issues.Likely an incorrect or invalid review comment.
src/common/data/stores/app/space/spaceStore.ts (7)
8-8: LGTM on type imports and interface additions.The
SpaceTypeValueandSPACE_TYPESimports, unifiedSpaceRegistrationtype, and optionalspaceTypefield inCachedSpaceare well-integrated with the existing type system.Also applies to: 17-18, 101-102
218-245: Good defensive guards for unregistered spaces.The early-return guards for unregistered spaces prevent runtime errors. The TODO at line 236 correctly acknowledges that auto-registration for non-NAV_PAGE types needs future implementation.
284-351: LGTM onsaveLocalSpaceTabimplementation.The refactored logic properly handles:
- New space initialization with theme setup
spaceTypepropagation and defensive setting- Order array management for new tabs
The captured
isNewSpaceandisNewTabflags before thesetcallback correctly preserve the pre-mutation state.
720-786: LGTM oncommitSpaceOrderToDatabaseguards.The added guards for unregistered spaces are consistent with
commitSpaceTabToDatabase. Using the capturedlocalSpacevariable throughout the function is more efficient than repeatedget()calls.
984-990: LGTM on registration functions withspaceTypepropagation.The unified
SpaceRegistrationtype usage withOmit<..., "signature">is consistent acrossregisterSpaceFid,registerChannelSpace,registerSpaceContract, andregisterProposalSpace. ThespaceTypefield is properly set in bothlocalSpacesandremoteSpacesentries.Also applies to: 1012-1014, 1096-1103, 1127-1128
1462-1468: LGTM onremoveLocalSpaceimplementation.The implementation correctly cleans up all space state. Note that the method name could be
removeSpacesince it removes fromlocalSpaces,editableSpaces, andremoteSpaces, though this is a minor naming consideration.
130-135: All call sites forsaveLocalSpaceTabhave been updated with the newspaceTypeparameter. The breaking change is complete and no missing implementations were found.
| if (isNotFound) { | ||
| return <SpaceNotFound />; | ||
| } | ||
|
|
||
| // If we found a pending space but spaceData doesn't have the spaceId, update it | ||
| const effectiveSpaceData = useMemo(() => { | ||
| if (pendingSpaceId && !spaceData.spaceId) { | ||
| return { | ||
| ...spaceData, | ||
| spaceId: pendingSpaceId, | ||
| }; | ||
| } | ||
| return spaceData; | ||
| }, [spaceData, pendingSpaceId]); | ||
|
|
||
| // Add isEditable and spacePageUrl logic on the client side | ||
| // PublicSpace will automatically use localSpaces data if available (via getCurrentSpaceConfig) | ||
| const spaceDataWithClientSideLogic = useMemo(() => ({ | ||
| ...effectiveSpaceData, | ||
| spacePageUrl: (tabName: string) => { | ||
| // No query params needed - server doesn't need to know about pending | ||
| return `/${effectiveSpaceData.navSlug}/${encodeURIComponent(tabName)}`; | ||
| }, | ||
| // NavPage editability is based on admin keys, not FID | ||
| // We ignore the currentUserFid parameter since we use identity keys instead | ||
| isEditable: (_currentUserFid: number | undefined) => | ||
| isNavPageSpaceEditable( | ||
| effectiveSpaceData.adminIdentityPublicKeys, | ||
| currentUserIdentityPublicKey | ||
| ), | ||
| }), [effectiveSpaceData, currentUserIdentityPublicKey]); |
There was a problem hiding this comment.
Fix: Hooks called after early return violate Rules of Hooks.
The useMemo calls at lines 107 and 119 are placed after the early return at line 102, violating React's Rules of Hooks. Hooks must be called unconditionally and in the same order on every render.
Restructure to call all hooks before any early returns:
// Initialize frame ready state for mini app support
useEffect(() => {
if (!isFrameReady) setFrameReady();
}, [isFrameReady, setFrameReady]);
+ // If we found a pending space but spaceData doesn't have the spaceId, update it
+ const effectiveSpaceData = useMemo(() => {
+ if (pendingSpaceId && !spaceData.spaceId) {
+ return {
+ ...spaceData,
+ spaceId: pendingSpaceId,
+ };
+ }
+ return spaceData;
+ }, [spaceData, pendingSpaceId]);
+
+ // Add isEditable and spacePageUrl logic on the client side
+ const spaceDataWithClientSideLogic = useMemo(() => ({
+ ...effectiveSpaceData,
+ spacePageUrl: (tabName: string) => {
+ return `/${effectiveSpaceData.navSlug}/${encodeURIComponent(tabName)}`;
+ },
+ isEditable: (_currentUserFid: number | undefined) =>
+ isNavPageSpaceEditable(
+ effectiveSpaceData.adminIdentityPublicKeys,
+ currentUserIdentityPublicKey
+ ),
+ }), [effectiveSpaceData, currentUserIdentityPublicKey]);
+
// If space is not found, show not found component
if (isNotFound) {
return <SpaceNotFound />;
}
- // If we found a pending space but spaceData doesn't have the spaceId, update it
- const effectiveSpaceData = useMemo(() => {
- if (pendingSpaceId && !spaceData.spaceId) {
- return {
- ...spaceData,
- spaceId: pendingSpaceId,
- };
- }
- return spaceData;
- }, [spaceData, pendingSpaceId]);
-
- // Add isEditable and spacePageUrl logic on the client side
- // PublicSpace will automatically use localSpaces data if available (via getCurrentSpaceConfig)
- const spaceDataWithClientSideLogic = useMemo(() => ({
- ...effectiveSpaceData,
- spacePageUrl: (tabName: string) => {
- // No query params needed - server doesn't need to know about pending
- return `/${effectiveSpaceData.navSlug}/${encodeURIComponent(tabName)}`;
- },
- // NavPage editability is based on admin keys, not FID
- // We ignore the currentUserFid parameter since we use identity keys instead
- isEditable: (_currentUserFid: number | undefined) =>
- isNavPageSpaceEditable(
- effectiveSpaceData.adminIdentityPublicKeys,
- currentUserIdentityPublicKey
- ),
- }), [effectiveSpaceData, currentUserIdentityPublicKey]);
-
return (
<PublicSpace
spacePageData={spaceDataWithClientSideLogic}
tabName={tabName}
/>
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (isNotFound) { | |
| return <SpaceNotFound />; | |
| } | |
| // If we found a pending space but spaceData doesn't have the spaceId, update it | |
| const effectiveSpaceData = useMemo(() => { | |
| if (pendingSpaceId && !spaceData.spaceId) { | |
| return { | |
| ...spaceData, | |
| spaceId: pendingSpaceId, | |
| }; | |
| } | |
| return spaceData; | |
| }, [spaceData, pendingSpaceId]); | |
| // Add isEditable and spacePageUrl logic on the client side | |
| // PublicSpace will automatically use localSpaces data if available (via getCurrentSpaceConfig) | |
| const spaceDataWithClientSideLogic = useMemo(() => ({ | |
| ...effectiveSpaceData, | |
| spacePageUrl: (tabName: string) => { | |
| // No query params needed - server doesn't need to know about pending | |
| return `/${effectiveSpaceData.navSlug}/${encodeURIComponent(tabName)}`; | |
| }, | |
| // NavPage editability is based on admin keys, not FID | |
| // We ignore the currentUserFid parameter since we use identity keys instead | |
| isEditable: (_currentUserFid: number | undefined) => | |
| isNavPageSpaceEditable( | |
| effectiveSpaceData.adminIdentityPublicKeys, | |
| currentUserIdentityPublicKey | |
| ), | |
| }), [effectiveSpaceData, currentUserIdentityPublicKey]); | |
| // Initialize frame ready state for mini app support | |
| useEffect(() => { | |
| if (!isFrameReady) setFrameReady(); | |
| }, [isFrameReady, setFrameReady]); | |
| // If we found a pending space but spaceData doesn't have the spaceId, update it | |
| const effectiveSpaceData = useMemo(() => { | |
| if (pendingSpaceId && !spaceData.spaceId) { | |
| return { | |
| ...spaceData, | |
| spaceId: pendingSpaceId, | |
| }; | |
| } | |
| return spaceData; | |
| }, [spaceData, pendingSpaceId]); | |
| // Add isEditable and spacePageUrl logic on the client side | |
| // PublicSpace will automatically use localSpaces data if available (via getCurrentSpaceConfig) | |
| const spaceDataWithClientSideLogic = useMemo(() => ({ | |
| ...effectiveSpaceData, | |
| spacePageUrl: (tabName: string) => { | |
| // No query params needed - server doesn't need to know about pending | |
| return `/${effectiveSpaceData.navSlug}/${encodeURIComponent(tabName)}`; | |
| }, | |
| // NavPage editability is based on admin keys, not FID | |
| // We ignore the currentUserFid parameter since we use identity keys instead | |
| isEditable: (_currentUserFid: number | undefined) => | |
| isNavPageSpaceEditable( | |
| effectiveSpaceData.adminIdentityPublicKeys, | |
| currentUserIdentityPublicKey | |
| ), | |
| }), [effectiveSpaceData, currentUserIdentityPublicKey]); | |
| // If space is not found, show not found component | |
| if (isNotFound) { | |
| return <SpaceNotFound />; | |
| } | |
| return ( | |
| <PublicSpace | |
| spacePageData={spaceDataWithClientSideLogic} | |
| tabName={tabName} | |
| /> | |
| ); |
🧰 Tools
🪛 Biome (2.1.2)
[error] 107-107: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 119-119: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
| async function getDefaultTab(spaceId: string): Promise<string | null> { | ||
| try { | ||
| // Create Supabase client with checked credentials | ||
| const supabase = createClient(supabaseUrl, supabaseKey); | ||
|
|
||
| // Fetch tab order | ||
| const { data: tabOrderData, error: tabOrderError } = await supabase.storage | ||
| const { data: tabOrderData, error } = await createSupabaseServerClient() | ||
| .storage | ||
| .from('spaces') | ||
| .download(`${spaceId}/tabOrder`); | ||
|
|
||
| if (tabOrderError || !tabOrderData) { | ||
| console.warn(`Failed to load tabOrder for space ${spaceId}:`, tabOrderError); | ||
| if (error || !tabOrderData) { | ||
| return null; | ||
| } | ||
|
|
||
| const tabOrderFile = JSON.parse(await tabOrderData.text()) as SignedFile; | ||
| const tabOrderObj = JSON.parse(tabOrderFile.fileData) as { tabOrder: string[] }; | ||
| const tabOrder = tabOrderObj.tabOrder || []; | ||
|
|
||
| // Fetch each tab config | ||
| const tabs: Record<string, any> = {}; | ||
| for (const tabName of tabOrder) { | ||
| try { | ||
| const { data: tabData, error: tabError } = await supabase.storage | ||
| .from('spaces') | ||
| .download(`${spaceId}/tabs/${tabName}`); | ||
|
|
||
| if (tabError || !tabData) { | ||
| console.warn(`Failed to load tab ${tabName} for space ${spaceId}:`, tabError); | ||
| continue; | ||
| } | ||
|
|
||
| const tabFile = JSON.parse(await tabData.text()) as SignedFile; | ||
| const tabConfig = JSON.parse(tabFile.fileData); | ||
| tabs[tabName] = tabConfig; | ||
| } catch (error) { | ||
| console.warn(`Error parsing tab ${tabName} for space ${spaceId}:`, error); | ||
| } | ||
| } | ||
|
|
||
| if (Object.keys(tabs).length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| // Reconstruct NavPageConfig format | ||
| return { | ||
| defaultTab: tabOrder[0] || 'Home', | ||
| tabOrder, | ||
| tabs, | ||
| layout: { | ||
| defaultLayoutFidget: 'grid', | ||
| gridSpacing: 16, | ||
| theme: { | ||
| background: '#ffffff', | ||
| fidgetBackground: '#ffffff', | ||
| font: 'Inter', | ||
| fontColor: '#000000', | ||
| }, | ||
| }, | ||
| } as NavPageConfig; | ||
| } catch (error) { | ||
| console.error(`Error loading space ${spaceId} as page config:`, error); | ||
| const tabOrderJson = JSON.parse(await tabOrderData.text()); | ||
| return tabOrderJson.tabOrder?.[0] || null; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Harden tabOrder parsing to avoid accidental 404s
On Line 25-26, this assumes the downloaded JSON is an object with { tabOrder: string[] }. If storage contains a raw string[] (or other historical shape), tabOrderJson.tabOrder will be undefined and you’ll notFound() later. Consider supporting both shapes.
- const tabOrderJson = JSON.parse(await tabOrderData.text());
- return tabOrderJson.tabOrder?.[0] || null;
+ const parsed = JSON.parse(await tabOrderData.text());
+ const tabOrder = Array.isArray(parsed) ? parsed : parsed?.tabOrder;
+ return Array.isArray(tabOrder) ? tabOrder[0] ?? null : null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function getDefaultTab(spaceId: string): Promise<string | null> { | |
| try { | |
| // Create Supabase client with checked credentials | |
| const supabase = createClient(supabaseUrl, supabaseKey); | |
| // Fetch tab order | |
| const { data: tabOrderData, error: tabOrderError } = await supabase.storage | |
| const { data: tabOrderData, error } = await createSupabaseServerClient() | |
| .storage | |
| .from('spaces') | |
| .download(`${spaceId}/tabOrder`); | |
| if (tabOrderError || !tabOrderData) { | |
| console.warn(`Failed to load tabOrder for space ${spaceId}:`, tabOrderError); | |
| if (error || !tabOrderData) { | |
| return null; | |
| } | |
| const tabOrderFile = JSON.parse(await tabOrderData.text()) as SignedFile; | |
| const tabOrderObj = JSON.parse(tabOrderFile.fileData) as { tabOrder: string[] }; | |
| const tabOrder = tabOrderObj.tabOrder || []; | |
| // Fetch each tab config | |
| const tabs: Record<string, any> = {}; | |
| for (const tabName of tabOrder) { | |
| try { | |
| const { data: tabData, error: tabError } = await supabase.storage | |
| .from('spaces') | |
| .download(`${spaceId}/tabs/${tabName}`); | |
| if (tabError || !tabData) { | |
| console.warn(`Failed to load tab ${tabName} for space ${spaceId}:`, tabError); | |
| continue; | |
| } | |
| const tabFile = JSON.parse(await tabData.text()) as SignedFile; | |
| const tabConfig = JSON.parse(tabFile.fileData); | |
| tabs[tabName] = tabConfig; | |
| } catch (error) { | |
| console.warn(`Error parsing tab ${tabName} for space ${spaceId}:`, error); | |
| } | |
| } | |
| if (Object.keys(tabs).length === 0) { | |
| return null; | |
| } | |
| // Reconstruct NavPageConfig format | |
| return { | |
| defaultTab: tabOrder[0] || 'Home', | |
| tabOrder, | |
| tabs, | |
| layout: { | |
| defaultLayoutFidget: 'grid', | |
| gridSpacing: 16, | |
| theme: { | |
| background: '#ffffff', | |
| fidgetBackground: '#ffffff', | |
| font: 'Inter', | |
| fontColor: '#000000', | |
| }, | |
| }, | |
| } as NavPageConfig; | |
| } catch (error) { | |
| console.error(`Error loading space ${spaceId} as page config:`, error); | |
| const tabOrderJson = JSON.parse(await tabOrderData.text()); | |
| return tabOrderJson.tabOrder?.[0] || null; | |
| } catch { | |
| return null; | |
| } | |
| } | |
| async function getDefaultTab(spaceId: string): Promise<string | null> { | |
| try { | |
| const { data: tabOrderData, error } = await createSupabaseServerClient() | |
| .storage | |
| .from('spaces') | |
| .download(`${spaceId}/tabOrder`); | |
| if (error || !tabOrderData) { | |
| return null; | |
| } | |
| const parsed = JSON.parse(await tabOrderData.text()); | |
| const tabOrder = Array.isArray(parsed) ? parsed : parsed?.tabOrder; | |
| return Array.isArray(tabOrder) ? tabOrder[0] ?? null : null; | |
| } catch { | |
| return null; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/app/[navSlug]/[[...tabName]]/page.tsx around lines 14 to 30, the parsing
assumes the downloaded JSON is { tabOrder: string[] } which can break if storage
contains a raw string[] or another shape; update the parsing to handle both
shapes by checking whether the parsed value is an array (use it directly) or an
object with a tabOrder array (use that), validate that the resulting value is an
array of strings before returning the first element, and keep the catch
returning null for any parse/validation errors.
| {/* Admin Edit Mode Header */} | ||
| {navEditMode && !shrunk && ( | ||
| <div className="mb-4 pb-2 border-b border-gray-200"> | ||
| <div className="flex items-center justify-between mb-2"> | ||
| <span className="text-sm font-semibold text-gray-600">Edit Navigation</span> | ||
| <div className="flex gap-1"> | ||
| <button | ||
| onClick={handleSaveNavChanges} | ||
| disabled={isSaving} | ||
| className="p-1.5 text-green-600 hover:bg-green-50 rounded" | ||
| title="Save changes" | ||
| > | ||
| <FaCheck size={14} /> | ||
| </button> | ||
| <button | ||
| onClick={handleCancelNavEdit} | ||
| className="p-1.5 text-gray-500 hover:bg-gray-100 rounded" | ||
| title="Cancel" | ||
| > | ||
| <FaXmark size={14} /> | ||
| </button> | ||
| </div> | ||
| </div> | ||
| <button | ||
| onClick={addNavItem} | ||
| className="flex items-center gap-2 w-full p-2 text-sm text-blue-600 hover:bg-blue-50 rounded-lg" | ||
| > | ||
| <FaPlus size={12} /> | ||
| <span>Add Page</span> | ||
| </button> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Disable “Cancel” (and “Add Page”) while saving to prevent state races
On Line 503-519, isSaving disables Save but not Cancel/Add; exiting edit mode while commitNavChanges() is inflight can leave local/pending spaces and nav state in an odd spot.
- <button
+ <button
onClick={handleCancelNavEdit}
+ disabled={isSaving}
className="p-1.5 text-gray-500 hover:bg-gray-100 rounded"
title="Cancel"
>- <button
+ <button
onClick={addNavItem}
+ disabled={isSaving}
className="flex items-center gap-2 w-full p-2 text-sm text-blue-600 hover:bg-blue-50 rounded-lg"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {/* Admin Edit Mode Header */} | |
| {navEditMode && !shrunk && ( | |
| <div className="mb-4 pb-2 border-b border-gray-200"> | |
| <div className="flex items-center justify-between mb-2"> | |
| <span className="text-sm font-semibold text-gray-600">Edit Navigation</span> | |
| <div className="flex gap-1"> | |
| <button | |
| onClick={handleSaveNavChanges} | |
| disabled={isSaving} | |
| className="p-1.5 text-green-600 hover:bg-green-50 rounded" | |
| title="Save changes" | |
| > | |
| <FaCheck size={14} /> | |
| </button> | |
| <button | |
| onClick={handleCancelNavEdit} | |
| className="p-1.5 text-gray-500 hover:bg-gray-100 rounded" | |
| title="Cancel" | |
| > | |
| <FaXmark size={14} /> | |
| </button> | |
| </div> | |
| </div> | |
| <button | |
| onClick={addNavItem} | |
| className="flex items-center gap-2 w-full p-2 text-sm text-blue-600 hover:bg-blue-50 rounded-lg" | |
| > | |
| <FaPlus size={12} /> | |
| <span>Add Page</span> | |
| </button> | |
| </div> | |
| )} | |
| {/* Admin Edit Mode Header */} | |
| {navEditMode && !shrunk && ( | |
| <div className="mb-4 pb-2 border-b border-gray-200"> | |
| <div className="flex items-center justify-between mb-2"> | |
| <span className="text-sm font-semibold text-gray-600">Edit Navigation</span> | |
| <div className="flex gap-1"> | |
| <button | |
| onClick={handleSaveNavChanges} | |
| disabled={isSaving} | |
| className="p-1.5 text-green-600 hover:bg-green-50 rounded" | |
| title="Save changes" | |
| > | |
| <FaCheck size={14} /> | |
| </button> | |
| <button | |
| onClick={handleCancelNavEdit} | |
| disabled={isSaving} | |
| className="p-1.5 text-gray-500 hover:bg-gray-100 rounded" | |
| title="Cancel" | |
| > | |
| <FaXmark size={14} /> | |
| </button> | |
| </div> | |
| </div> | |
| <button | |
| onClick={addNavItem} | |
| disabled={isSaving} | |
| className="flex items-center gap-2 w-full p-2 text-sm text-blue-600 hover:bg-blue-50 rounded-lg" | |
| > | |
| <FaPlus size={12} /> | |
| <span>Add Page</span> | |
| </button> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In src/common/components/organisms/Navigation.tsx around lines 489 to 520, the
Cancel and Add Page buttons are not disabled while isSaving is true which allows
exiting edit mode or adding items mid-commit and can cause state races; update
the Cancel and Add Page button elements to honor isSaving by adding the disabled
prop (disabled={isSaving}), prevent their onClick handlers from running when
isSaving (either by guarding at the top of the handler or returning early), and
add a visual disabled style (e.g., reduce opacity/change cursor) so they look
inactive while isSaving to prevent user interaction until commitNavChanges()
completes.
| reorderItems: (newOrder) => { | ||
| set((draft) => { | ||
| if (!draft.navigation.localItems) return; | ||
| const itemsById = new Map(draft.navigation.localItems.map(i => [i.id, i])); | ||
| const reordered = newOrder | ||
| .map(id => itemsById.get(id)) | ||
| .filter(Boolean) as NavigationItem[]; | ||
| draft.navigation.localItems = reordered; | ||
| }, "reorderNavItems"); | ||
| }, |
There was a problem hiding this comment.
Avoid dropping items if newOrder is incomplete
On Line 248-251, any id not present in newOrder gets silently removed from localItems. Safer: append missing items after the reordered subset.
const reordered = newOrder
.map(id => itemsById.get(id))
.filter(Boolean) as NavigationItem[];
- draft.navigation.localItems = reordered;
+ const missing = draft.navigation.localItems.filter(i => !newOrder.includes(i.id));
+ draft.navigation.localItems = [...reordered, ...missing];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| reorderItems: (newOrder) => { | |
| set((draft) => { | |
| if (!draft.navigation.localItems) return; | |
| const itemsById = new Map(draft.navigation.localItems.map(i => [i.id, i])); | |
| const reordered = newOrder | |
| .map(id => itemsById.get(id)) | |
| .filter(Boolean) as NavigationItem[]; | |
| draft.navigation.localItems = reordered; | |
| }, "reorderNavItems"); | |
| }, | |
| reorderItems: (newOrder) => { | |
| set((draft) => { | |
| if (!draft.navigation.localItems) return; | |
| const itemsById = new Map(draft.navigation.localItems.map(i => [i.id, i])); | |
| const reordered = newOrder | |
| .map(id => itemsById.get(id)) | |
| .filter(Boolean) as NavigationItem[]; | |
| const missing = draft.navigation.localItems.filter(i => !newOrder.includes(i.id)); | |
| draft.navigation.localItems = [...reordered, ...missing]; | |
| }, "reorderNavItems"); | |
| }, |
🤖 Prompt for AI Agents
In src/common/data/stores/app/navigation/navigationStore.ts around lines 244 to
253, the reorderItems implementation currently drops any items whose ids are not
present in newOrder; change it to preserve missing items by computing the
reordered subset from newOrder, then append any original localItems not included
in newOrder (preserving their original relative order), and assign that combined
list back to draft.navigation.localItems; keep the existing guard for missing
localItems and the set(...) call context string.
| // Now update the full navigation config with all items | ||
| const finalNavConfig: NavigationConfig = { | ||
| ...originalNavConfig, | ||
| items: localItems, | ||
| }; | ||
|
|
There was a problem hiding this comment.
Fix possible TS compile error: spreading NavigationConfig | null
On Line 367-370, originalNavConfig is NavigationConfig | null; object-spreading a nullable union typically fails under strictNullChecks (“Spread types may only be created from object types.”). Make it explicitly non-null.
- const finalNavConfig: NavigationConfig = {
- ...originalNavConfig,
- items: localItems,
- };
+ const baseNavConfig: NavigationConfig = originalNavConfig ?? { items: [] };
+ const finalNavConfig: NavigationConfig = {
+ ...baseNavConfig,
+ items: localItems,
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Now update the full navigation config with all items | |
| const finalNavConfig: NavigationConfig = { | |
| ...originalNavConfig, | |
| items: localItems, | |
| }; | |
| // Now update the full navigation config with all items | |
| const baseNavConfig: NavigationConfig = originalNavConfig ?? { items: [] }; | |
| const finalNavConfig: NavigationConfig = { | |
| ...baseNavConfig, | |
| items: localItems, | |
| }; | |
🤖 Prompt for AI Agents
In src/common/data/stores/app/navigation/navigationStore.ts around lines 366 to
371, object-spreading `originalNavConfig` which is typed `NavigationConfig |
null` will fail under strictNullChecks; ensure you only spread a non-null object
by replacing the spread with a non-null fallback (e.g. spread
`(originalNavConfig ?? {})`) or assert non-null after a prior guard (e.g. use
`originalNavConfig!` if safe), and ensure the resulting value is typed as
NavigationConfig (cast if necessary) so the finalNavConfig assignment compiles.
| // Validate type-specific required fields based on spaceType | ||
| function validateSpaceRegistrationFields(registration: SpaceRegistration): { valid: boolean; error?: string } { | ||
| const { spaceType } = registration; | ||
|
|
||
| // Update the type guard to handle SpaceRegistrationProposer | ||
| function isSpaceRegistrationProposer(maybe: unknown): maybe is SpaceRegistrationProposer { | ||
| return ( | ||
| isSpaceRegistration(maybe) && | ||
| maybe["spaceType"] === SPACE_TYPES.PROPOSAL && | ||
| typeof maybe["proposalId"] === "string" | ||
| ); | ||
| } | ||
| if (spaceType === SPACE_TYPES.PROFILE) { | ||
| if (typeof registration.fid !== "number" && typeof registration.fid !== "string") { | ||
| return { valid: false, error: "PROFILE space type requires 'fid' field" }; | ||
| } | ||
| } else if (spaceType === SPACE_TYPES.TOKEN) { | ||
| if (typeof registration.contractAddress !== "string") { | ||
| return { valid: false, error: "TOKEN space type requires 'contractAddress' field" }; | ||
| } | ||
| } else if (spaceType === SPACE_TYPES.PROPOSAL) { | ||
| if (typeof registration.proposalId !== "string") { | ||
| return { valid: false, error: "PROPOSAL space type requires 'proposalId' field" }; | ||
| } | ||
| } else if (spaceType === SPACE_TYPES.CHANNEL) { | ||
| if (typeof registration.channelId !== "string") { | ||
| return { valid: false, error: "CHANNEL space type requires 'channelId' field" }; | ||
| } | ||
| if (typeof registration.fid !== "number" && typeof registration.fid !== "string") { | ||
| return { valid: false, error: "CHANNEL space type requires 'fid' field" }; | ||
| } | ||
| } else if (spaceType === SPACE_TYPES.NAV_PAGE) { | ||
| // NAV_PAGE has no additional required fields beyond base | ||
| } | ||
| // Any other space type is accepted (future-proofing) | ||
|
|
||
| function isSpaceRegistrationChannel(maybe: unknown): maybe is SpaceRegistrationChannel { | ||
| return ( | ||
| isSpaceRegistration(maybe) && | ||
| maybe["spaceType"] === SPACE_TYPES.CHANNEL && | ||
| typeof maybe["channelId"] === "string" && | ||
| (typeof maybe["fid"] === "number" || typeof maybe["fid"] === "string") | ||
| ); | ||
| return { valid: true }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/pages/api/space/registry/index.ts | head -100Repository: Nounspace/nounspace.ts
Length of output: 4563
🏁 Script executed:
wc -l src/pages/api/space/registry/index.tsRepository: Nounspace/nounspace.ts
Length of output: 107
🏁 Script executed:
sed -n '100,300p' src/pages/api/space/registry/index.tsRepository: Nounspace/nounspace.ts
Length of output: 5768
🏁 Script executed:
rg -n "adminIdentityPublicKeys|admin\s*=|isAdmin|unauthorized|forbidden" src/pages/api/space/registry/index.ts -C3Repository: Nounspace/nounspace.ts
Length of output: 48
🏁 Script executed:
sed -n '230,270p' src/pages/api/space/registry/index.tsRepository: Nounspace/nounspace.ts
Length of output: 1491
🏁 Script executed:
rg -n "NAV_PAGE" src/ -t ts --type-list -C3 | head -80Repository: Nounspace/nounspace.ts
Length of output: 455
🏁 Script executed:
rg -n "NAV_PAGE" src/ -t ts -C3Repository: Nounspace/nounspace.ts
Length of output: 7064
🏁 Script executed:
rg -n "navigationConfig|nav.*page|admin" src/ -t ts | head -40Repository: Nounspace/nounspace.ts
Length of output: 3905
🏁 Script executed:
sed -n '75,95p' src/common/types/spaceData.tsRepository: Nounspace/nounspace.ts
Length of output: 727
🏁 Script executed:
rg -n "admin_identity_public_keys|navSlug|NAV_PAGE.*admin|modifySpace.*NAV_PAGE" src/ -t ts -A2 -B2 | head -80Repository: Nounspace/nounspace.ts
Length of output: 5167
🏁 Script executed:
rg -n "spaceRegistrations" src/supabase/database.d.ts -A30 | head -50Repository: Nounspace/nounspace.ts
Length of output: 1670
🏁 Script executed:
rg -n "GET.*spaceId|updateSpace.*NAV_PAGE|deleteSpace.*NAV_PAGE" src/pages/api/space/ -t ts -B2 -A5Repository: Nounspace/nounspace.ts
Length of output: 48
🏁 Script executed:
sed -n '1480,1500p' src/common/data/stores/app/space/spaceStore.tsRepository: Nounspace/nounspace.ts
Length of output: 954
🏁 Script executed:
rg -n "getSpace|useSpace.*NAV_PAGE|fetchSpace" src/pages/api/space/ -t ts -B2 -A10 | head -100Repository: Nounspace/nounspace.ts
Length of output: 1108
NAV_PAGE registration has no authorization check and allows any signed identity to register
Lines 236-239 show NAV_PAGE spaces skip authorization validation entirely. Combined with the auto-registration logic in spaceStore.ts, any user can register unlimited NAV_PAGE spaces without constraints.
The code comment states "authorization is handled at the navigation config level," but this is misleading—admin keys are stored in the separate community_configs table and are not enforced during NAV_PAGE space registration. Since NavPageSpacePageData includes an adminIdentityPublicKeys field, indicating NAV_PAGE spaces are intended to be admin-managed, the registration endpoint should validate that the requesting identity is an admin (via community_configs.admin_identity_public_keys) before allowing NAV_PAGE space creation.
🤖 Prompt for AI Agents
In src/pages/api/space/registry/index.ts around lines 61 to 90, NAV_PAGE
registrations currently skip authorization and therefore allow any signed
identity to register; update validation to enforce that the registering identity
is an admin for the target community by loading the community_config (by
communityId) and checking the request's identity public key exists in
community_config.admin_identity_public_keys; if the community config is missing
or the identity is not listed, return a validation error denying NAV_PAGE
registration. Use the same DB access/service utilities used elsewhere in this
file to fetch community_configs and perform the check, and keep existing
behavior for other space types.
j-paterson
force-pushed
the
adminEditor
branch
from
dfa7033 to
e44bc42
Compare
Summary by CodeRabbit
New Features
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.