Update community_configs fields, map additional fields to elements, implement domain->config routing, and add endpoint for registering community_configs #1625
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@j-paterson has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 5 minutes and 57 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds domain normalization and cached SystemConfig loading, a POST /api/community-config route, token-gating utilities/hooks (ERC20 + NFT), font/color theming support, runtime metadata/frame fetching, and updates components to use token-gates and new theming fields. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Middleware as Middleware
participant Registry as Registry
participant Cache as SystemConfigCache
participant DB as Supabase
Browser->>Middleware: HTTP request (Host header)
Middleware->>Middleware: normalizeDomain(host)
Middleware->>Cache: lookup(normalizedDomain)
alt cache hit
Cache-->>Middleware: cached config
else cache miss
Middleware->>Registry: getCommunityConfigForDomain(normalizedDomain)
Registry->>DB: query community_configs (by communityId)
DB-->>Registry: row
Registry->>Registry: transformRowToSystemConfig(row)
Registry->>Cache: write(normalizedDomain, SystemConfig)
Registry-->>Middleware: { communityId, config }
end
Middleware->>Browser: Respond (sets x-detected-domain header)
sequenceDiagram
participant Component
participant Hook as useTokenGate
participant Privy as Privy
participant Wagmi as Wagmi/useBalance
participant Alchemy as Alchemy NFT API
participant Store as AppStore
Component->>Hook: call useTokenGate(systemConfig)
Hook->>Privy: request wallet address
alt wallet connected
Hook->>Wagmi: fetch ERC20 balance for configured token
Wagmi-->>Hook: balance
par NFT checks
Hook->>Alchemy: query NFT holders for token A
Hook->>Alchemy: query NFT holders for token B
and
Alchemy-->>Hook: holdings
end
Hook->>Store: read hasNogs
Hook-->>Component: { hasEnoughTokens, hasNogs, gatingSatisfied, balanceAmount }
else no wallet
Hook-->>Component: defaults (not satisfied)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
willyogo
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/terms/page.tsx (1)
99-117: Duplicate "Contact and Support" content.The "Contact and Support" section is duplicated (lines 99-108 and lines 110-117 contain identical content). This appears to be an unintentional copy-paste.
🔎 Proposed fix: Remove duplicate section
* **Email/Other:** If an official support email or contact form is provided on the nounspace website, you can use that as well. (At the time of writing these Terms, the primary support channels are Discord and GitHub, as listed above.) - -Nounspace is a community-driven project, and we value our users. If you have any questions, concerns, or feedback about these Terms or the platform in general, please reach out to us. The best ways to contact us include: - -* **Discord:** We have an official nounspace Discord community (chat server) where the team and community members are active. Feel free to join and ask questions or raise concerns. - -* **GitHub:** Since nounspace is open source, you can also open an issue on our GitHub repository if you encounter bugs or have questions about the code. (For general Terms or account issues, Discord or email might get a faster response, but we do monitor GitHub issues as well.) - -* **Email/Other:** If an official support email or contact form is provided on the nounspace website, you can use that as well. (At the time of writing these Terms, the primary support channels are Discord and GitHub, as listed above.) - We are here to help and to ensure that your experience on nounspace is positive.src/common/components/organisms/NogsGateButton.tsx (1)
164-198: Stale closure bug:nftTokensmissing from dependency array.The
checkForNogscallback callsisHoldingNogs, which reads fromnftTokensstate. However,nftTokensis not in the dependency array, so ifnftTokensupdates after the initial render,checkForNogswill still reference the oldisHoldingNogsclosure that sees empty/stalenftTokens.🔎 Proposed fix
}, [ user, nogsTimeoutTimer, nogsRecheckCountDownTimer, nogsRecheckTimerLength, setNogsRecheckCountDown, setNogsRecheckTimerLength, setNogsShouldRecheck, setNogsTimeoutTimer, setHasNogs, + nftTokens, ]);
🧹 Nitpick comments (7)
supabase/seed.sql (1)
165-180: Minor: Inconsistent color format across communities.Clanker uses
rgba()format while other communities usergb(). Both are valid CSS, but for consistency, consider standardizing on one format across all community configs.src/app/api/community-config/route.ts (1)
64-82: Consider validating payload structure before database insertion.The config fields are cast directly to database types without schema validation. Malformed data could cause runtime issues or corrupt the database. Consider using a schema validation library (e.g., Zod) to validate the structure of
brandConfig,assetsConfig, etc.🔎 Example with Zod validation
import { z } from 'zod'; const BrandConfigSchema = z.object({ displayName: z.string(), description: z.string(), miniAppTags: z.array(z.string()), }); // Validate before insertion const brandConfigResult = BrandConfigSchema.safeParse(brandConfig); if (!brandConfigResult.success) { return NextResponse.json( { success: false, error: 'Invalid brand_config structure', details: brandConfigResult.error }, { status: 400 } ); }src/fidgets/farcaster/components/CreateCast.tsx (1)
370-370: Dependency array references a mutable ref.currentvalue.Using
editorContentRef.currentin the dependency array is problematic—React won't detect changes to.current. The effect runs only on mount/unmount because the ref object itself doesn't change, which may be intentional here, but the dependency is misleading.🔎 Proposed fix
return () => { el.removeEventListener("paste", handler as any); }; - }, [editorContentRef.current]); + }, []); // Effect depends on ref availability, not its current valuesrc/constants/spaceToken.ts (1)
9-18: Async function performs only synchronous operations.
getSpaceContractAddr()is markedasyncbut only validates and returns a constant. If the async signature is for future-proofing (e.g., dynamic config loading), this is acceptable. Otherwise, consider making it synchronous to avoid unnecessary Promise wrapping.If async is not required for future use
-export async function getSpaceContractAddr(): Promise<Address> { +export function getSpaceContractAddr(): Address { if (!isAddress(DEFAULT_SPACE_CONTRACT_ADDR)) { throw new Error( "Invalid default SPACE contract address configured. Expected a checksummed 0x-prefixed address.", ); } return DEFAULT_SPACE_CONTRACT_ADDR; }src/common/components/organisms/NogsGateButton.tsx (2)
137-148: Avoidas anytype assertion.The
token.network as anybypasses type safety. SincenftTokensstate is typed as{ address: string; network?: CommunityTokenNetwork }[], the network should already be compatible withmapNetworkToAlchemy.🔎 Proposed fix
- const alchemyNetwork = mapNetworkToAlchemy(token.network as any); + const alchemyNetwork = mapNetworkToAlchemy(token.network);
240-253: Parallel NFT checks could hit rate limits.With multiple NFT tokens,
Promise.allfires all Alchemy API requests simultaneously. If the number of configured NFT tokens grows, this could trigger rate limiting.Consider adding a sequential check with early exit (similar to
isHoldingNogs), or limit concurrency for larger token lists.src/config/loaders/registry.ts (1)
55-58: Consider consolidating Vercel preview handling intoDOMAIN_TO_COMMUNITY_MAP.The inline check for
.vercel.appdeployments containing 'nounspace' introduces a special case that could be harder to discover. Moving this logic to the explicit mapping at the top of the file would improve discoverability.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
docs/SYSTEMS/CONFIGURATION/ARCHITECTURE_OVERVIEW.md(1 hunks)middleware.ts(1 hunks)scripts/seed.ts(6 hunks)src/app/api/community-config/route.ts(1 hunks)src/app/layout.tsx(4 hunks)src/app/terms/page.tsx(1 hunks)src/common/components/atoms/reorderable-tab.tsx(2 hunks)src/common/components/organisms/MobileHeader.tsx(4 hunks)src/common/components/organisms/MobileNavbar.tsx(4 hunks)src/common/components/organisms/Navigation.tsx(7 hunks)src/common/components/organisms/NogsChecker.tsx(1 hunks)src/common/components/organisms/NogsGateButton.tsx(4 hunks)src/common/lib/hooks/useUIColors.ts(2 hunks)src/common/lib/theme/BackgroundGenerator.tsx(2 hunks)src/common/lib/utils/fontUtils.ts(1 hunks)src/common/lib/utils/tokenGates.ts(1 hunks)src/config/clanker/clanker.brand.ts(0 hunks)src/config/clanker/clanker.community.ts(1 hunks)src/config/example/example.brand.ts(0 hunks)src/config/example/example.community.ts(1 hunks)src/config/index.ts(2 hunks)src/config/loaders/registry.ts(3 hunks)src/config/nouns/nouns.brand.ts(0 hunks)src/config/nouns/nouns.community.ts(1 hunks)src/config/systemConfig.ts(3 hunks)src/constants/metadata.ts(3 hunks)src/constants/nogs.ts(1 hunks)src/constants/spaceToken.ts(1 hunks)src/fidgets/farcaster/components/CreateCast.tsx(3 hunks)src/supabase/database.d.ts(1 hunks)supabase/seed.sql(5 hunks)
💤 Files with no reviewable changes (3)
- src/config/nouns/nouns.brand.ts
- src/config/example/example.brand.ts
- src/config/clanker/clanker.brand.ts
🧰 Additional context used
🧬 Code graph analysis (12)
src/app/api/community-config/route.ts (2)
src/common/data/database/supabase/clients/server.ts (1)
createSupabaseServerClient(7-7)src/supabase/database.d.ts (1)
Database(11-1004)
src/common/lib/utils/tokenGates.ts (4)
src/config/systemConfig.ts (3)
CommunityErc20Token(6-12)CommunityNftToken(14-20)CommunityTokenNetwork(4-4)src/config/index.ts (1)
loadSystemConfig(37-71)src/constants/spaceToken.ts (1)
getSpaceContractAddr(10-18)src/constants/nogs.ts (1)
getNogsContractAddr(7-9)
src/fidgets/farcaster/components/CreateCast.tsx (3)
src/config/index.ts (1)
CommunityTokenNetwork(78-78)src/config/systemConfig.ts (1)
CommunityTokenNetwork(4-4)src/common/lib/utils/tokenGates.ts (2)
getGateTokens(12-48)getChainForNetwork(50-55)
src/common/lib/hooks/useUIColors.ts (1)
src/common/lib/utils/fontUtils.ts (1)
extractFontFamilyFromUrl(1-11)
src/common/components/organisms/Navigation.tsx (1)
src/common/lib/hooks/useUIColors.ts (1)
useUIColors(15-49)
middleware.ts (1)
src/config/loaders/registry.ts (2)
normalizeDomain(86-93)resolveCommunityConfig(205-221)
src/config/systemConfig.ts (1)
src/config/index.ts (3)
CommunityTokenNetwork(78-78)CommunityNftToken(77-77)CommunityTokensConfig(79-79)
src/common/lib/theme/BackgroundGenerator.tsx (3)
src/config/index.ts (1)
CommunityTokenNetwork(78-78)src/config/systemConfig.ts (1)
CommunityTokenNetwork(4-4)src/common/lib/utils/tokenGates.ts (2)
getGateTokens(12-48)getChainForNetwork(50-55)
src/common/components/organisms/NogsGateButton.tsx (4)
src/config/systemConfig.ts (1)
CommunityTokenNetwork(4-4)src/common/lib/utils/tokenGates.ts (3)
getGateTokens(12-48)getChainForNetwork(50-55)mapNetworkToAlchemy(57-62)src/pages/api/signerRequests.ts (1)
AlchemyIsHolderOfContract(70-72)src/constants/urls.ts (1)
ALCHEMY_API(7-19)
src/app/layout.tsx (1)
src/common/lib/utils/fontUtils.ts (1)
extractFontFamilyFromUrl(1-11)
src/config/loaders/registry.ts (2)
src/supabase/database.d.ts (1)
Database(11-1004)src/common/data/database/supabase/clients/server.ts (1)
createSupabaseServerClient(7-7)
src/constants/metadata.ts (2)
middleware.ts (1)
config(43-55)src/constants/app.ts (1)
WEBSITE_URL(1-6)
🔇 Additional comments (43)
src/config/nouns/nouns.community.ts (3)
17-34: Token addresses verified and correct.Both addresses have been confirmed against official blockchain explorers:
- Nouns NFT (0x9c8ff314c9bc7f6e59a9d9225fb22946427edc03) is the official contract on Ethereum mainnet
- SPACE token (0x48c6740bcf807d6c47c864faeea15ed4da3910ab) is correctly deployed on Base network
No action required.
7-35: Verify removed fields aren't breaking references across the codebase.Significant fields have been removed from the nouns community configuration:
urls.twitter,urls.github,urls.forumsocial.discord,social.twitter- Entire
contractsblock (previously contained nouns, auctionHouse, space, nogs)No references to these removed properties exist in the codebase. The only file consuming
nounsCommunity(src/config/nouns/nouns.explore.ts) correctly uses the newtokensconfiguration andsocial.farcaster, both of which remain intact.
16-16: Empty governance object is intentional and safe—no action required.The
governance: {}configuration is valid. Thegovernancefield is optional in theCommunityConfigtype, and all properties within it (snapshotSpaceandnounishGov) are also optional. No code in the codebase accesses these properties; theNounishGovernancefidget uses its own settings object, not the community config's governance field. This empty object is consistent across all community configurations in the project and poses no safety concerns.Likely an incorrect or invalid review comment.
docs/SYSTEMS/CONFIGURATION/ARCHITECTURE_OVERVIEW.md (1)
200-206: LGTM!The documentation table updates accurately reflect the new configuration fields being added to the system: mini-app tags for brand config, governance identifiers and tokens for community config, and font colors/font URL for UI config. These align well with the broader UI theming and token gating changes in this PR.
supabase/seed.sql (2)
98-114: LGTM!The Nouns community configuration is well-structured with the new UI theming fields (
fontColor,castButtonFontColor,urlfor fonts,backgroundColor) and properly configured token gate data. The nestedcastButtonobject provides consistent styling configuration.
137-142: Placeholder token addresses are appropriate for example community.The example community uses obvious placeholder addresses (
0x1234...) which is suitable for test/example configurations. The UI config structure matches the other communities correctly.src/common/lib/utils/tokenGates.ts (2)
22-48: LGTM!The fallback mechanism correctly constructs legacy SPACE and NOGS tokens when no community-configured tokens exist. Using
Promise.allfor concurrent address resolution is efficient, and the conditional NFT token inclusion based onnogsAddresstruthiness is clean.
50-55: LGTM!Clean mapping from internal network identifiers to viem
Chainobjects with sensiblebasedefault.src/common/lib/utils/fontUtils.ts (1)
1-11: Consider edge case: multiple font families in URL.Google Fonts allows multiple font families to be requested by separating the names with a pipe character (|), and the CSS2 API supports multiple families via repeated
family=parameters. The current implementation only extracts the first family. If the function is intended for single-family URLs only, document this constraint; otherwise, consider handling pipe-separated families or multiple family parameters.src/app/terms/page.tsx (1)
17-17: The 1,111 ERC20 threshold is correctly implemented in the gating logic. Line 17 ofNogsGateButton.tsxdefinesconst MIN_SPACE_TOKENS_FOR_UNLOCK = 1111;and this constant is used in the balance check (lines 92-100) to verify users hold sufficient tokens. The terms documentation accurately reflects the implementation.src/config/systemConfig.ts (3)
6-12: LGTM! Optionalnamefield added for display purposes.Adding optional
namefields to bothCommunityErc20TokenandCommunityNftTokenprovides flexibility for human-readable token identification without breaking existing configurations.Also applies to: 14-20
37-50: LGTM! UI theming configuration extended.The optional fields (
url,fontColor,castButtonFontColor,backgroundColor) complement the required color properties and enable per-community font/color customization.
103-117: Optional interface properties require consumers to use defensive access patterns.While making
social,governance, andtokensoptional aligns the interface with the actual runtime values in provided configs, ensure all consuming code handles potential undefined values. TypeScript will enforce type safety at compile time ifstrictNullChecksis enabled, but verify that configuration access patterns use optional chaining (?.) where needed.src/common/components/atoms/reorderable-tab.tsx (1)
65-72: LGTM! Consistent theming viauseUIColors.Applying
fontColorandfontFamilyfrom the centralized UI colors hook ensures consistent styling across navigation surfaces and aligns with the broader theming approach in this PR.src/config/index.ts (2)
1-7: LGTM! Clean type exports.Importing
SystemConfigas a value (for runtime use) alongside type-only imports for token configuration types is correct. This enables proper tree-shaking while exposing the necessary types.
73-80: LGTM! Public API expanded for token types.Exporting the token-related types (
CommunityErc20Token,CommunityNftToken,CommunityTokenNetwork,CommunityTokensConfig) enables consumers to properly type their token configurations.src/common/components/organisms/MobileHeader.tsx (2)
137-144: LGTM! Consistent button theming applied.The cast button properly uses
castButtonFontColorfor both the button color and icon, withfontFamilyfor consistent typography. The hover/active state handlers maintain the interactive feedback.Also applies to: 148-173
175-201: LGTM! Login button styled with primary colors.The login button appropriately uses
primaryColorwith corresponding hover/active states, maintaining visual consistency with the theming system.src/constants/metadata.ts (1)
36-54: LGTM! Metadata fields aligned with updated BrandConfig.The migration from
brand.name/brand.taglinetobrand.displayName/brand.descriptioncorrectly reflects the simplifiedBrandConfiginterface.src/fidgets/farcaster/components/CreateCast.tsx (2)
65-66: LGTM!The imports align correctly with the new token gate utilities. The type import for
CommunityTokenNetworkis properly scoped.
396-409: Approve the balance query refactor with a note on edge case behavior.The logic correctly uses the loaded
erc20Tokenfor balance queries. Whenerc20Tokenisnull:
tokendefaults tozeroAddress, which queries native token balance (not ERC-20).query.enabledisfalse, so the query won't run until token is loaded.This is acceptable since
enabled: !!erc20Tokenprevents the query from executing prematurely. The decimals fallback chain (result.data.decimals ?? erc20Token?.decimals ?? 18) is robust.src/app/layout.tsx (3)
11-11: LGTM!The font utility import and font stack construction are well-implemented with proper fallbacks to system fonts.
Also applies to: 124-128
139-147: LGTM!The CSS variable injection via inline styles is safe—React escapes values appropriately. The type assertion pattern
["--ns-nav-font" as string]is a reasonable workaround for TypeScript's CSSProperties type limitations.
58-65: LGTM!The metadata updates correctly use
displayNamefor user-facing text and properly populate the Farcaster frame action fields.middleware.ts (1)
14-24: LGTM on the async refactor!The middleware correctly awaits the async community resolution. The
normalizeDomaincall beforeresolveCommunityConfigensures consistent domain handling. The relevant code snippet shows thatresolveCommunityConfiginternally callsnormalizeDomainagain, but this is harmless (idempotent).src/config/example/example.community.ts (1)
7-35: LGTM!The example community config cleanup is appropriate. The simplified structure with an empty
governanceobject and retainedtokensblock aligns with the PR's goal of removing unused fields. Thesatisfies CommunityConfigassertion ensures type compatibility.src/constants/nogs.ts (1)
1-13: LGTM!The refactor to environment-driven defaults is clean and maintains backward compatibility:
- Environment variable priority chain is correct (
NEXT_PUBLIC_for client, then server-side, then hardcoded fallback).- The async function signature is preserved for API compatibility even though it now returns immediately.
- The
Promise.resolve()wrapper forNOGS_CONTRACT_ADDRmaintains the Promise type for existing consumers.src/common/components/organisms/MobileNavbar.tsx (2)
34-35: Clean refactor to CSS variable-based theming.The switch from
activeColor/inactiveColortofontColor/fontFamilywith opacity-based selection state (1 vs 0.7) simplifies the styling logic while maintaining visual distinction between selected and unselected tabs. The inline style approach correctly overrides the Tailwind classes.Also applies to: 43-44, 78-86
312-313: CSS variable defaults with proper fallback chain.The fallback chain
var(--ns-nav-font-color, #0f172a)and the font family fallbackvar(--ns-nav-font, var(--font-sans, Inter, system-ui, -apple-system, sans-serif))ensure graceful degradation when CSS variables are not defined.src/common/lib/theme/BackgroundGenerator.tsx (2)
59-77: Verify behavior when no ERC20 tokens are configured.When
tokens.erc20Tokensis empty,erc20Tokenremainsnull, the balance query is disabled (enabled: !!erc20Token), andspaceLoadingwill befalseafter initial load. This meansuserHoldEnoughSpacedefaults tofalseand the banner will show if!hasNogs.Confirm this is the intended behavior—if no tokens are configured, users without NOGS cannot generate backgrounds. If the feature should be unrestricted when no gating token is configured, consider adding a fallback or feature flag.
72-85: Balance query and formatting logic looks correct.The query correctly waits for
erc20Tokento be loaded before enabling, and the decimal fallback chain (result.data.decimals ?? erc20Token?.decimals ?? 18) provides sensible defaults. ThegetChainForNetworkutility properly maps the network to a chain ID.src/common/components/organisms/Navigation.tsx (2)
101-101: Clean integration of UI colors hook.The
useUIColorshook provides centralized color/font values, and thenavTextStyleobject efficiently bundles them for reuse across multiple elements. The cast button colors correctly derive fromuiColors.castButtonwith a separatecastButtonFontColorfor text contrast.Also applies to: 110-119
313-313: Consistent navTextStyle application across navigation elements.Applying
navTextStyletoNavItem,NavButton, and thenavcontainer ensures consistent font styling throughout the navigation. This is a well-structured approach.Also applies to: 342-342, 372-372
src/common/lib/hooks/useUIColors.ts (2)
22-28: CSS variable values are cached and won't update if variables change.The
useMemodependency is[systemConfig], so CSS variable values read viagetComputedStyleare captured once and won't recompute if the CSS variables change at runtime (e.g., dynamic theme switching). If CSS variables are set once on page load based on the community config, this is acceptable. Otherwise, consider adding a mechanism to trigger recomputation.
30-30: Font family derivation with proper fallback chain.The logic correctly prioritizes: parsed font family from URL → CSS variable → default system fonts. The fallback chain ensures graceful degradation when font configuration is incomplete.
Also applies to: 35-37
scripts/seed.ts (2)
344-357: UI config enrichment aligns with theming system.The new
ui_configfields (fontColor,castButtonFontColor,url,backgroundColor) correctly populate the values expected byuseUIColors. The consistent use of Inter font and matching color schemes across communities ensures a cohesive experience.Also applies to: 435-448, 553-566
266-266: Empty governance objects reduce config surface.Setting
governance: {}for all communities aligns with the PR objective of simplifying the public config surface. If governance features are needed later, the structure is in place.Also applies to: 394-394, 493-493
src/constants/spaceToken.ts (1)
20-22: No action needed.SPACE_CONTRACT_ADDRis not used anywhere in the codebase—the search found zero usages outside its definition. The code imports and usesgetSpaceContractAddr()function instead. The constant exists only as a legacy export for backward compatibility with no actual callers that need updating.Likely an incorrect or invalid review comment.
src/config/clanker/clanker.community.ts (1)
7-28: LGTM!The governance simplification to an empty object aligns with the PR's objective to remove unused fields. The
satisfiesassertions ensure type safety is maintained.src/supabase/database.d.ts (1)
186-224: Schema addition looks correct.The new
community_configstable follows the existing patterns in this file. A few observations:
- Using
Jsonfor config fields provides flexibility but sacrifices compile-time type safety on the stored data.- The
community_idis a required string on insert, which aligns with the domain-based resolution inregistry.ts.Consider adding a comment or documentation indicating what shape is expected for each JSON config field (e.g.,
brand_configschema) to help future maintainers.src/common/components/organisms/NogsGateButton.tsx (1)
85-100: LGTM on the ERC20 balance check logic.The
useBalancehook is properly guarded withquery.enabled, and the decimals fallback chain is sensible.src/config/loaders/registry.ts (2)
33-34: In-memory cache implementation is reasonable.The cache with TTL provides good protection against navigation bursts. Note that in serverless deployments (Vercel Edge, Lambda), the cache won't be shared across instances, but it still provides value for rapid successive requests within the same instance lifecycle.
Also applies to: 118-138
205-221: LGTM on the public API.The
resolveCommunityConfigfunction provides a clean interface for domain-to-config resolution with appropriate warning logging for unresolved domains.
| export async function POST(request: NextRequest): Promise<Response> { | ||
| try { | ||
| const body = await request.json(); | ||
| const incomingConfig: IncomingCommunityConfig | undefined = body?.community_config; | ||
|
|
||
| if (!incomingConfig || typeof incomingConfig !== 'object') { | ||
| return NextResponse.json( | ||
| { success: false, error: 'Missing community_config payload' }, | ||
| { status: 400 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Critical: Missing authentication and authorization.
This endpoint allows unauthenticated users to create or overwrite any community configuration by specifying a community_id. This is a significant security vulnerability that could allow:
- Hijacking existing communities by overwriting their configs
- Creating spam/malicious community entries
- Data corruption
Add authentication (e.g., verify the request is from an authenticated user) and authorization (e.g., verify the user is an admin of the specified community).
🔎 Suggested approach
export async function POST(request: NextRequest): Promise<Response> {
try {
// Add authentication check
const supabase = createSupabaseServerClient();
const { data: { user }, error: authError } = await supabase.auth.getUser();
if (authError || !user) {
return NextResponse.json(
{ success: false, error: 'Authentication required' },
{ status: 401 }
);
}
const body = await request.json();
const incomingConfig = body?.community_config;
// ... existing validation ...
// Add authorization check - verify user can modify this community
const { data: existingConfig } = await supabase
.from('community_configs')
.select('community_id')
.eq('community_id', communityId)
.single();
if (existingConfig) {
// Verify user has permission to update existing config
// (implement your authorization logic here)
}
// ... rest of the handler ...
} catch (error) {
// ...
}
}🤖 Prompt for AI Agents
In src/app/api/community-config/route.ts around lines 18 to 28 the handler
accepts a community_config payload without any authentication or authorization,
allowing unauthenticated users to create/overwrite community configs; fix by
adding an auth check (e.g., createSupabaseServerClient and call auth.getUser())
and return 401 if no authenticated user, then validate the incoming payload
including community_id, and perform an authorization check against your DB
(e.g., query memberships/communities to confirm the authenticated user is an
admin or has permission to create/modify that community); if the user is not
authorized return 403, otherwise proceed with the existing create/update logic
and surface auth/db errors in the response/logs.
There was a problem hiding this comment.
Will add auth before deploying to prod
There was a problem hiding this comment.
@willyogo, thanks for acknowledging this! Since this is a critical security issue, would you like me to create a GitHub issue to track adding authentication and authorization before production deployment? This would help ensure it doesn't get overlooked.
There was a problem hiding this comment.
sure, that would be great! thanks coderabbit
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Create GitHub issue to track authentication requirement
gh issue create \
--repo blankdotspace/space-system \
--title "Add authentication and authorization to /api/community-config endpoint" \
--body "## Description
The POST /api/community-config endpoint currently accepts requests without authentication or authorization checks. This creates a critical security vulnerability that could allow:
- Unauthenticated users to create or overwrite community configurations
- Hijacking existing communities
- Data corruption
## Action Required
Add authentication and authorization checks to the endpoint before deploying to production.
## Context
- Endpoint: \`src/app/api/community-config/route.ts\`
- Identified in PR: https://github.com/blankdotspace/space-system/pull/1625
- Discussion: https://github.com/blankdotspace/space-system/pull/1625#discussion_r2636386868
- Requested by: @willyogo
## Priority
🔴 Critical - Must be resolved before production deployment" \
--assignee willyogo
Length of output: 57
@willyogo, I've created issue #1633 to track adding authentication and authorization to the /api/community-config endpoint: #1633
The issue has been assigned to you and marked as critical priority. This will help ensure the security measures are in place before production deployment. 🔒
|
@j-paterson got this into a good working state! I can successfully
Also removed some unused config fields and added and mapped some missing fields that the launchpad outputs. Going to create another branch on top of this to make sure each variable is correctly mapped throughout the app, but would love your review on it and coderabbit's feedback so far (down to run walk through it together too) |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/config/loaders/registry.ts (1)
167-170: Cachingnullon transient errors still present.This concern was raised in a previous review. When a Supabase error occurs (network timeout, temporary unavailability), all candidates are cached as
nullfor 60 seconds, which can extend outages. Consider not caching on error or using a shorter TTL for error cases.
🧹 Nitpick comments (3)
scripts/seed.ts (1)
344-357: Consider renamingurltofontUrlfor clarity.The
urlproperty inui_configholds a Google Fonts URL, but the generic name could be confusing. Consider renaming it tofontUrlorfontStylesheetUrlto make its purpose self-documenting.🔎 Suggested naming improvement
fontColor: 'rgb(15, 23, 42)', castButtonFontColor: '#ffffff', - url: 'https://fonts.googleapis.com/css2?family=Inter:wght@400;500;600;700&display=swap', + fontUrl: 'https://fonts.googleapis.com/css2?family=Inter:wght@400;500;600;700&display=swap', backgroundColor: 'rgb(255, 255, 255)',This same change would apply to lines 441, 559 for the other community configs.
src/config/index.ts (1)
75-92: Solid fallback strategy, but consider structured error logging.The fallback to
DEFAULT_COMMUNITY_IDprevents runtime crashes when a community config fails to load. However, logging the rawerrorobject may expose stack traces or sensitive details in production logs. Consider logging onlyerror instanceof Error ? error.message : String(error).🔎 Suggested improvement for error logging
if (communityId && communityId !== DEFAULT_COMMUNITY_ID) { console.warn( `Falling back to default community after failed load for "${communityId}".`, - error + error instanceof Error ? error.message : String(error) );src/config/loaders/registry.ts (1)
197-213: Minor: Domain is normalized twice.
resolveCommunityConfignormalizes the domain on line 200, then passes it togetCommunityConfigForDomain, which callsgetCommunityIdCandidatesthat normalizes again on line 36. This is harmless (idempotent) but slightly redundant.🔎 Option to avoid double normalization
Either remove normalization from
resolveCommunityConfigand letgetCommunityIdCandidateshandle it, or pass a flag/use an internal variant that skips re-normalization.export async function resolveCommunityConfig( domain: string ): Promise<{ communityId: string; config: CommunityConfigRow } | null> { - const normalizedDomain = normalizeDomain(domain); - if (!normalizedDomain) { + if (!domain?.trim()) { return null; } - const resolution = await getCommunityConfigForDomain(normalizedDomain); + const resolution = await getCommunityConfigForDomain(domain);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
scripts/seed.ts(9 hunks)src/config/index.ts(3 hunks)src/config/loaders/index.ts(1 hunks)src/config/loaders/registry.ts(2 hunks)src/config/loaders/utils.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/config/loaders/utils.ts (2)
src/config/loaders/registry.ts (1)
DEFAULT_COMMUNITY_ID(17-17)src/config/loaders/index.ts (1)
getCommunityIdFromHeaders(19-19)
src/config/loaders/registry.ts (2)
src/supabase/database.d.ts (1)
Database(11-1004)src/common/data/database/supabase/clients/server.ts (1)
createSupabaseServerClient(7-7)
src/config/index.ts (2)
src/config/loaders/utils.ts (2)
getDomainFromContext(70-99)getCommunityIdFromHeaders(47-58)src/config/loaders/registry.ts (1)
DEFAULT_COMMUNITY_ID(17-17)
🔇 Additional comments (9)
scripts/seed.ts (1)
249-266: LGTM!The community_id change to
nounspace.comaligns with the new domain-based routing strategy, and the simplifiedgovernance: {}removes unused configuration fields as intended.src/config/loaders/index.ts (1)
19-19: LGTM!The new
getCommunityIdFromHeadersexport properly extends the public API surface to support the middleware-resolved community ID pattern.src/config/index.ts (1)
46-58: LGTM!The parallel fetching of domain and middleware-resolved community ID is efficient. The fallback chain (middleware header → domain resolution → default) provides robust community identification.
src/config/loaders/utils.ts (2)
39-58: LGTM!Well-documented helper that safely reads the middleware-set
x-community-idheader. The dynamic import pattern and try/catch gracefully handle non-request contexts (static generation), returningundefinedconsistently with other functions in this module.
30-34: LGTM!The build-time fallback behavior is now clearly documented and uses
DEFAULT_COMMUNITY_IDconsistently, ensuring predictable behavior during static generation.src/config/loaders/registry.ts (4)
79-86: LGTM!The
normalizeDomainfunction handles common variations robustly: port stripping, lowercasing, andwww.prefix removal. The empty string return for invalid input is a safe default.
35-70: LGTM!The candidate generation logic implements a clear priority order with proper deduplication. The localhost subdomain extraction supports local development testing workflows effectively.
145-190: Efficient batch resolution with proper priority handling.The single
.in()query fetches all candidates in one round-trip, and the iteration preserves the priority order established bygetCommunityIdCandidates. The cache updates for both hits and misses reduce subsequent DB calls.
27-33: Cache lacks size bounds but likely acceptable.The in-memory cache has no maximum size, but since community_id values are limited in practice, unbounded growth is unlikely. In serverless/edge environments, the cache may reset between requests anyway.
Consider if you're running in a long-lived server process with many domains, whether a cache size limit or LRU eviction would be warranted.
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/config/loaders/registry.ts (1)
84-98: Past review comment about runtime validation still applies.The type assertions
as unknown as SystemConfig['...']bypass compile-time checks. While field presence is validated (lines 223-240, 282-298), there's no validation of the nested JSON structure. If the database contains malformed JSON (e.g., missing nested properties or incorrect types), runtime errors will occur when downstream components access expected properties.Consider adding runtime validation using a schema validator (e.g., Zod) or document that callers must handle potential structure mismatches.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/config/loaders/registry.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: j-paterson
Repo: blankdotspace/space-system PR: 1625
File: src/common/lib/utils/tokenGates.ts:12-20
Timestamp: 2025-12-29T18:25:25.957Z
Learning: Gate tokens in src/common/lib/utils/tokenGates.ts are currently fetched redundantly from 3 client components (CreateCast.tsx, BackgroundGenerator.tsx, NogsGateButton.tsx) with no caching, and should be refactored to use a cached API route with a shared client hook to reduce redundant server calls.
🧬 Code graph analysis (1)
src/config/loaders/registry.ts (5)
src/supabase/database.d.ts (1)
Database(11-1010)src/config/index.ts (1)
SystemConfig(89-89)src/config/shared/themes.ts (1)
themes(8-8)src/config/loaders/index.ts (2)
getCommunityConfigForDomain(20-20)loadSystemConfigById(20-20)src/common/data/database/supabase/clients/server.ts (1)
createSupabaseServerClient(7-7)
🔇 Additional comments (8)
src/config/loaders/registry.ts (8)
1-7: LGTM!The imports and default community ID constant are well-structured and properly exported for use throughout the module.
8-20: LGTM!The domain mapping configuration is clear and well-documented. The comment correctly notes that Vercel preview deployments are handled separately through pattern matching.
22-38: LGTM!The cache implementation is well-structured with appropriate types and a reasonable 60-second TTL for configuration data.
67-81: LGTM!The domain normalization logic is thorough and handles edge cases correctly, including port stripping, case normalization, and www prefix removal.
100-123: LGTM!The cache helpers correctly implement a three-state model (undefined = not cached, null = cached miss, value = hit) with automatic cleanup of expired entries.
126-173: LGTM!The domain-based config resolution implements a clear priority system with appropriate caching and fallback logic. The function correctly avoids redundant fallback attempts when the primary community is already the default.
179-249: LGTM!The error handling correctly distinguishes between transient errors and legitimate not-found cases, avoiding aggressive null-caching that could suppress valid entries during temporary outages. The validation checks all required fields as expected. Past review comments have been properly addressed.
251-307: LGTM!The function appropriately throws errors instead of returning null, which is the correct behavior when loading a config by a known community ID. The validation and caching logic matches
tryLoadCommunityConfig, ensuring consistency.
src/config/loaders/registry.ts
Outdated
| /** | ||
| * Resolve community ID from domain. | ||
| * Simple priority: special mapping → domain as-is → default fallback | ||
| */ | ||
| function resolveCommunityIdFromDomain(domain: string): string { | ||
| const normalizedDomain = normalizeDomain(domain); | ||
|
|
||
| // Check special domain mappings first (highest priority) | ||
| if (domain in DOMAIN_TO_COMMUNITY_MAP) { | ||
| return DOMAIN_TO_COMMUNITY_MAP[domain]; | ||
| if (!normalizedDomain) { | ||
| return DEFAULT_COMMUNITY_ID; | ||
| } | ||
|
|
||
| // Handle Vercel preview deployments (e.g., nounspace.vercel.app, branch-nounspace.vercel.app) | ||
| // All Vercel preview deployments should point to nouns community | ||
| // Priority 1: Handle Vercel preview deployments (e.g., nounspace.vercel.app, branch-nounspace.vercel.app) | ||
| // All Vercel preview deployments should point to nounspace.com community | ||
| // Match any .vercel.app domain (preview deployments have random branch names) | ||
| if (domain.endsWith('.vercel.app')) { | ||
| return 'nouns'; | ||
| return DEFAULT_COMMUNITY_ID; | ||
| } | ||
|
|
||
| // Priority 2: Special domain mappings (from DOMAIN_TO_COMMUNITY_MAP) | ||
| if (normalizedDomain in DOMAIN_TO_COMMUNITY_MAP) { | ||
| return DOMAIN_TO_COMMUNITY_MAP[normalizedDomain]; | ||
| } | ||
|
|
||
| // Support localhost subdomains for local testing | ||
| // e.g., example.localhost:3000 -> example | ||
| if (domain.includes('localhost')) { | ||
| const parts = domain.split('.'); | ||
| if (parts.length > 1 && parts[0] !== 'localhost') { | ||
| // Has subdomain before localhost | ||
| return parts[0]; | ||
| // Priority 3: Domain as community ID (e.g., example.nounspace.com → example.nounspace.com) | ||
| // Priority 4: Default fallback (handled by caller if domain lookup fails) | ||
| return normalizedDomain; | ||
| } |
There was a problem hiding this comment.
Fix inconsistent domain checking for Vercel deployments.
Line 53 checks domain.endsWith('.vercel.app') but should use normalizedDomain for consistency. The domain is normalized on line 44, but the original domain parameter is used for the Vercel check. This inconsistency could cause Vercel preview deployments with port numbers (e.g., example.vercel.app:3000) or mixed case (e.g., EXAMPLE.VERCEL.APP) to not be recognized correctly.
🔎 Proposed fix
// Priority 1: Handle Vercel preview deployments (e.g., nounspace.vercel.app, branch-nounspace.vercel.app)
// All Vercel preview deployments should point to nounspace.com community
// Match any .vercel.app domain (preview deployments have random branch names)
- if (domain.endsWith('.vercel.app')) {
+ if (normalizedDomain.endsWith('.vercel.app')) {
return DEFAULT_COMMUNITY_ID;
}🤖 Prompt for AI Agents
In src/config/loaders/registry.ts around lines 39 to 65, the Vercel preview
check uses the original domain variable which can miss normalized variants
(ports, casing); replace the condition to use
normalizedDomain.endsWith('.vercel.app') so the normalized value (already
computed and validated) is used for matching Vercel deployments, keeping the
check before the special mapping branch.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/config/loaders/registry.ts (1)
53-55: Critical: Use normalized domain for Vercel deployment check.Line 53 checks
domain.endsWith('.vercel.app')but should usenormalizedDomain(computed on line 44) for consistency. The originaldomainparameter may contain port numbers (e.g.,example.vercel.app:3000) or mixed case (e.g.,EXAMPLE.VERCEL.APP), which would cause the check to fail or behave inconsistently.Note: This issue was flagged in a previous review as critical and marked as addressed in commit b8d56ca, but the fix is not present in the current code. This may indicate a regression or that the commit was not included in this PR.
🔎 Proposed fix
// Priority 1: Handle Vercel preview deployments (e.g., nounspace.vercel.app, branch-nounspace.vercel.app) // All Vercel preview deployments should point to nounspace.com community // Match any .vercel.app domain (preview deployments have random branch names) - if (domain.endsWith('.vercel.app')) { + if (normalizedDomain.endsWith('.vercel.app')) { return DEFAULT_COMMUNITY_ID; }
🧹 Nitpick comments (1)
src/config/index.ts (1)
79-81: Remove redundant development environment check.Lines 79-81 check
process.env.NODE_ENV === 'development'inside a block that already verified this condition on line 77. The inner check is unnecessary.🔎 Proposed fix
// Priority 3: Development override if (process.env.NODE_ENV === 'development' && process.env.NEXT_PUBLIC_TEST_COMMUNITY) { const devCommunityId = process.env.NEXT_PUBLIC_TEST_COMMUNITY; - if (process.env.NODE_ENV === 'development') { - console.log(`✅ Loading config for community: ${devCommunityId} (dev override)`); - } + console.log(`✅ Loading config for community: ${devCommunityId} (dev override)`); return await loadSystemConfigById(devCommunityId); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/config/index.tssrc/config/loaders/index.tssrc/config/loaders/registry.tssrc/config/loaders/utils.ts
💤 Files with no reviewable changes (1)
- src/config/loaders/utils.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/config/index.ts (2)
src/config/loaders/index.ts (2)
loadSystemConfigById(19-19)getCommunityConfigForDomain(19-19)src/config/loaders/registry.ts (3)
loadSystemConfigById(275-331)getCommunityConfigForDomain(137-172)DEFAULT_COMMUNITY_ID(6-6)
🔇 Additional comments (2)
src/config/loaders/registry.ts (2)
201-224: Verified: Transient error handling is correct.The error handling logic correctly distinguishes between legitimate "not found" errors (PGRST116) and transient errors. Transient errors do not cache
null, allowing subsequent requests to retry. This addresses the concern raised in a previous review.
240-260: Verified: All required fields are validated.The validation logic correctly checks all four required fields (
brand_config,assets_config,community_config,fidgets_config), addressing the concern from a previous review about missing validation forcommunity_configandfidgets_config.
…fig loading functions - Remove all automatic fallbacks to DEFAULT_COMMUNITY_ID - Bypass cache completely - all lookups query database directly - Consolidate tryLoadCommunityConfig and loadSystemConfigById into single core function loadCommunityConfigFromDatabase - Add comprehensive logging throughout config loading flow for debugging - Update error handling to throw informative errors instead of silent fallbacks
- Add 'export const dynamic = force-dynamic' to root layout - Remove build-time fallback metadata logic - Metadata now generated per-request based on actual domain/community config - Ensures metadata matches runtime config instead of build-time default
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/config/loaders/index.ts (1)
16-19: Redundant explicit export.Line 17 already uses
export * from './registry', which exports all named exports from the registry module includinggetCommunityConfigForDomain,loadSystemConfigById, andDEFAULT_COMMUNITY_ID. The explicit re-export on line 19 is redundant.🔎 Proposed fix
export * from './types'; export * from './registry'; export * from './runtimeLoader'; -export { getCommunityConfigForDomain, loadSystemConfigById, DEFAULT_COMMUNITY_ID } from './registry';src/app/layout.tsx (1)
124-125: Consider sanitizing color values from the database.
navFontColorandcastButtonFontColorare injected directly into inline styles from database-sourced config. While the current threat model assumes admin-controlled data, malicious values could potentially break styling or (in edge cases) be used for CSS-based attacks.Consider adding a simple validation function to ensure these are valid CSS color values (hex, rgb, or named colors).
🔎 Example validation approach
const CSS_COLOR_REGEX = /^(#[0-9a-fA-F]{3,8}|rgb\(|rgba\(|hsl\(|hsla\(|[a-z]+)$/i; function validateCssColor(color: string | undefined, fallback: string): string { if (!color) return fallback; // Basic check - allows hex, rgb/rgba/hsl/hsla functions, and named colors if (CSS_COLOR_REGEX.test(color.trim())) { return color.trim(); } console.warn('Invalid CSS color value rejected:', color); return fallback; } // Usage: const navFontColor = validateCssColor(systemConfig.ui?.fontColor, "#0f172a"); const castButtonFontColor = validateCssColor(systemConfig.ui?.castButtonFontColor, "#ffffff");src/config/index.ts (1)
82-89: Minor: Redundant environment check.The
NODE_ENV === 'development'check on line 85 is redundant since it's already checked on line 83.🔎 Proposed fix
// Priority 3: Development override if (process.env.NODE_ENV === 'development' && process.env.NEXT_PUBLIC_TEST_COMMUNITY) { const devCommunityId = process.env.NEXT_PUBLIC_TEST_COMMUNITY; - if (process.env.NODE_ENV === 'development') { - console.log(`✅ Loading config for community: ${devCommunityId} (dev override)`); - } + console.log(`✅ Loading config for community: ${devCommunityId} (dev override)`); return await loadSystemConfigById(devCommunityId); }src/config/loaders/registry.ts (1)
30-36: Unused cache infrastructure.The
SYSTEM_CONFIG_CACHE,SystemConfigCacheEntry,readSystemConfigCache, andwriteSystemConfigCacheare defined but never used—getCommunityConfigForDomainandloadCommunityConfigFromDatabaseboth explicitly bypass the cache. Consider removing this dead code or implementing the caching if it's intended for future use.🔎 Option 1: Remove unused cache code
-/** - * Cache entry for SystemConfig lookups. - * - * Keeps a short-lived in-memory cache to reduce Supabase round-trips when the - * same community is requested repeatedly (e.g., during navigation or asset loads). - * Caches the final SystemConfig (transformed and ready to use). - */ -type SystemConfigCacheEntry = { - expiresAt: number; - value: SystemConfig | null; -}; - -const SYSTEM_CONFIG_CACHE = new Map<string, SystemConfigCacheEntry>(); -const SYSTEM_CONFIG_CACHE_TTL_MS = 60_000; ... -/** - * Attempt to read a cached SystemConfig entry if it has not expired. - */ -function readSystemConfigCache(communityId: string): SystemConfig | null | undefined { - const entry = SYSTEM_CONFIG_CACHE.get(communityId); - if (!entry) return undefined; - - if (entry.expiresAt > Date.now()) { - return entry.value; - } - - SYSTEM_CONFIG_CACHE.delete(communityId); - return undefined; -} - -/** - * Store a SystemConfig value in the cache with a short TTL. - */ -function writeSystemConfigCache(communityId: string, value: SystemConfig | null) { - SYSTEM_CONFIG_CACHE.set(communityId, { - expiresAt: Date.now() + SYSTEM_CONFIG_CACHE_TTL_MS, - value, - }); -}Also applies to: 95-118
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
middleware.tssrc/app/api/test-logs/route.tssrc/app/layout.tsxsrc/config/index.tssrc/config/loaders/index.tssrc/config/loaders/registry.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-29T18:25:25.957Z
Learnt from: j-paterson
Repo: blankdotspace/space-system PR: 1625
File: src/common/lib/utils/tokenGates.ts:12-20
Timestamp: 2025-12-29T18:25:25.957Z
Learning: Gate tokens in src/common/lib/utils/tokenGates.ts are currently fetched redundantly from 3 client components (CreateCast.tsx, BackgroundGenerator.tsx, NogsGateButton.tsx) with no caching, and should be refactored to use a cached API route with a shared client hook to reduce redundant server calls.
Applied to files:
src/app/layout.tsx
🧬 Code graph analysis (4)
middleware.ts (1)
src/config/loaders/registry.ts (1)
normalizeDomain(69-76)
src/app/layout.tsx (3)
src/config/index.ts (1)
loadSystemConfig(27-100)src/constants/app.ts (1)
WEBSITE_URL(1-6)src/common/lib/utils/fontUtils.ts (1)
extractFontFamilyFromUrl(1-11)
src/config/loaders/registry.ts (4)
src/config/loaders/index.ts (3)
DEFAULT_COMMUNITY_ID(19-19)getCommunityConfigForDomain(19-19)loadSystemConfigById(19-19)src/supabase/database.d.ts (1)
Database(11-1010)src/config/shared/themes.ts (1)
themes(8-8)src/common/data/database/supabase/clients/server.ts (1)
createSupabaseServerClient(7-7)
src/config/index.ts (1)
src/config/loaders/registry.ts (2)
loadSystemConfigById(263-276)getCommunityConfigForDomain(132-168)
🔇 Additional comments (11)
src/app/api/test-logs/route.ts (1)
1-2: LGTM!Standard Next.js import for API routes.
middleware.ts (1)
12-35: LGTM!The middleware simplification is well-designed. Deferring community resolution to server components avoids unnecessary database queries for static assets and API routes. The use of
console.errorfor Vercel log visibility is appropriate.src/app/layout.tsx (2)
13-40: LGTM!The stylesheet URL validation properly implements defense-in-depth by requiring HTTPS and checking against a trusted host allowlist. This addresses the previous security concern about CSS injection.
46-103: LGTM!The dynamic metadata generation correctly uses runtime config to populate Open Graph, icons, and Farcaster frame metadata. The
force-dynamicexport ensures this runs per-request rather than at build time.src/config/index.ts (1)
27-100: LGTM!The refactored
loadSystemConfigfunction has clear priority-based resolution with explicit error messages when configs are missing. Removing automatic fallbacks aligns with the PR objective to ensure each domain gets its intended config.src/config/loaders/registry.ts (6)
43-60: LGTM!The
resolveCommunityIdFromDomainfunction has clear priority logic with proper error handling when domain normalization fails. The comment on line 58 mentions "Priority 3" but there's no Priority 2—consider updating the comment for clarity (minor nit).
69-76: LGTM!The
normalizeDomainfunction correctly handles edge cases: empty input, port stripping, lowercase conversion, andwww.prefix removal.
83-93: Verify runtime behavior of type assertions.The type assertions (
as unknown as SystemConfig['...']) bypass compile-time type checking. While required field validation exists (lines 231-248), there's no validation of the JSON structure within each field. Ifbrand_configexists but has incorrect nested properties, runtime errors will occur when downstream components access expected properties.This was noted in a past review with a suggestion to add Zod validation. For MVP this may be acceptable, but consider documenting this limitation or adding runtime validation in a follow-up.
199-218: LGTM!Good improvement distinguishing between legitimate "not found" errors (PGRST116) and transient/unknown errors. This prevents caching failures on transient issues and provides clear error messages for debugging.
230-248: LGTM!Comprehensive validation of all required fields (
brand_config,assets_config,community_config,fidgets_config) with clear error messages. This addresses the previous review comment about incomplete validation.
263-276: LGTM!The
loadSystemConfigByIdfunction provides a clean interface for loading config by explicit ID with proper error handling when config is not found.
- If x-detected-domain header is not set by middleware, read host header directly - Normalize host header to get domain as fallback - Add detailed logging to debug header issues - Ensures domain detection works even if middleware doesn't set header
- Delete middleware.ts - no longer needed - Read host header directly in loadSystemConfig() instead of relying on middleware header - Simplify domain detection by removing middleware layer - Update error messages to remove middleware references - Host header works directly, making middleware unnecessary
- Remove all console.error debug/informational logs - Keep only actual error logs (failures, not found, transient errors) - Clean up entry point logs, domain resolution logs, database query logs - Remove success messages and cache bypass messages - Only log when something goes wrong, not for normal operation
- Delete ARCHITECTURE_SIMPLIFICATION.md planning file - Delete test-logs API endpoint used for debugging - Clean up temporary files added during issue investigation
- Re-enable cache checks in getCommunityConfigForDomain and loadSystemConfigById - Add cache check in loadCommunityConfigFromDatabase before database query - Write successful config loads to cache with 60 second TTL - Cache stores transformed SystemConfig objects, not raw database rows - Does not cache null values to allow immediate retries for transient errors
- Fall back to DEFAULT_COMMUNITY_ID when domain config not found - Fall back to DEFAULT_COMMUNITY_ID when no domain/communityId provided - Explicit communityId still throws error if not found (no fallback) - Ensures app always has a config to use, even if specific domain config is missing
Summary
Updating community_configs fields and mapping additional fields to elements:
Implemented domain->config routing:
Add endpoint for registering community_configs:
Testing
Codex Task
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.