Feature/stack companion #769
Conversation
This is a merge commit the virtual branches in your workspace. Due to GitButler managing multiple virtual branches, you cannot switch back and forth between git branches and virtual branches easily. If you switch to another branch, GitButler will need to be reinitialized. If you commit on this branch, GitButler will throw it away. Here are the branches that are currently applied: - dashboard_docs (refs/gitbutler/dashboard_docs) branch head: 7202731 - apps/dashboard/src/app/api/sso/featurebase/route.ts - apps/dashboard/src/app/api/feature-request/route.ts - apps/dashboard/.env - docker/readme.md - apps/dashboard/src/app/sso/featurebase/page.tsx - apps/dashboard/src/components/stack-companion.tsx - apps/backend/.env - apps/dashboard/src/components/stack-companion/changelog-widget.tsx - apps/dashboard/src/components/stack-companion/feature-request-board.tsx - apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsx For more information about what we're doing here, check out our docs: https://docs.gitbutler.com/features/virtual-branches/integration-branch
- Add user ID as primary identifier for feature requests to handle email changes - Store user ID in customInputValues for post creation - Add changelog widget component with Next.js Image optimization - Fix React hooks exhaustive-deps warnings with useCallback - Remove deprecated featurebase SSO route files
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
|
😱 Found 1 issue. Time to roll up your sleeves! 😱 🗒️ View all ignored comments in this repo
Need help? Join our Discord for support! |
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsx
Outdated
Show resolved
Hide resolved
…rule for this in settings anywhere thats causing it
There was a problem hiding this comment.
Greptile Summary
This PR introduces a new Stack Companion feature to the dashboard application, adding a collapsible sidebar with documentation, feature requests, and changelog functionality. The implementation includes:
- A new StackCompanion component that serves as a fixed right-side panel
- Integration with Featurebase for feature request management and changelogs
- New API endpoints for handling feature requests
- Environment configuration updates to support the new features
- UI components for feature request board and changelog display
The changes span both frontend and backend, with proper attention to authentication, state management, and user experience. The implementation follows existing patterns and includes responsive design considerations.
Confidence score: 3 /5
- This PR introduces significant new functionality that could impact user experience if not properly tested
- Several security concerns exist around XSS vulnerabilities and input validation
- The following files need particular attention:
changelog-widget.tsx: Usage of dangerouslySetInnerHTMLfeature-requests/route.tsx: Potential scalability issues with upvote fetchingfeature-request-board.tsx: Proper error handling needs review
11 files reviewed, 7 comments
Edit PR Review Bot Settings | Greptile
apps/dashboard/src/components/stack-companion/feature-request-board.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/src/components/stack-companion/feature-request-board.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
apps/dashboard/src/components/stack-companion/changelog-widget.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/src/components/stack-companion/feature-request-board.tsx
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/backend/.env (1)
69-69: Past feedback addressed: correct key name and typo fixedThe variable name
STACK_FEATUREBASE_API_KEYand “featurebase” spelling match prior suggestions. Thanks for aligning with earlier feedback.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
apps/backend/.env(1 hunks)apps/dashboard/package.json(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/dashboard/package.json
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsx
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
apps/backend/.env
[warning] 69-69: [UnorderedKey] The STACK_FEATUREBASE_API_KEY key should go before the STACK_FREESTYLE_API_KEY key
[warning] 69-69: [ValueWithoutQuotes] This value needs to be surrounded in quotes
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: docker
- GitHub Check: setup-tests
- GitHub Check: lint_and_build (latest)
- GitHub Check: restart-dev-and-test
- GitHub Check: docker
- GitHub Check: build (22.x)
- GitHub Check: claude-review
- GitHub Check: all-good
- GitHub Check: build (22.x)
- GitHub Check: Security Check
🔇 Additional comments (1)
apps/backend/.env (1)
69-69: No client-side references to STACK_FEATUREBASE_API_KEY detectedRipgrep searches for
process.env.STACK_FEATUREBASE_API_KEYand anyNEXT_PUBLIC_STACK_FEATUREBASE_API_KEYalias returned no matches across the repo. No further changes required—this secret remains server-only.
|
Claude finished @madster456's task —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
packages/stack-shared/src/utils/featurebase.tsx (1)
23-28: Add timeout and retry logic for API callsAll fetch requests in this file lack timeout configuration and retry logic for transient failures, which could lead to hanging requests or unnecessary failures.
🧹 Nitpick comments (1)
packages/stack-shared/src/utils/featurebase.tsx (1)
225-225: Improve fallback name generationUsing the email local part as a fallback name could expose email addresses in the UI if Featurebase displays usernames publicly. Consider using a more generic fallback.
- name: stackAuthUser.displayName || stackAuthUser.primaryEmail?.split('@')[0] || 'User', + name: stackAuthUser.displayName || `User ${stackAuthUser.id.slice(-6)}`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/stack-shared/src/utils/featurebase.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/stack-shared/src/utils/featurebase.tsx (2)
packages/stack-shared/src/utils/errors.tsx (1)
StackAssertionError(69-85)packages/stack-shared/src/utils/env.tsx (1)
getEnvVariable(16-58)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: restart-dev-and-test
- GitHub Check: build (22.x)
- GitHub Check: setup-tests
- GitHub Check: all-good
- GitHub Check: claude-review
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: lint_and_build (latest)
- GitHub Check: docker
- GitHub Check: Security Check
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/dashboard/package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: lint_and_build (latest)
- GitHub Check: restart-dev-and-test
- GitHub Check: docker
- GitHub Check: setup-tests
- GitHub Check: all-good
- GitHub Check: claude-review
- GitHub Check: docker
- GitHub Check: Security Check
This comment was marked as outdated.
This comment was marked as outdated.
|
Claude finished @madster456's task —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/stack-shared/src/utils/featurebase.tsx (2)
92-95: Fix inconsistent error handling patternThe error handling here suppresses all errors with
console.errorand returnsnull, while other similar functions (findFeaturebaseUserById,createFeaturebaseUser,updateFeaturebaseUser) consistently throwStackAssertionErrorfor failures. This creates inconsistent behavior for callers.Apply consistent error handling:
} catch (error) { - console.error('Error finding Featurebase user by email:', error); - return null; + if (error instanceof StackAssertionError) { + throw error; + } + throw new StackAssertionError("Failed to find Featurebase user by email", { cause: error }); }
23-28: Add timeout and retry logic for API callsAll fetch requests lack timeout configuration and retry logic for transient failures. Consider adding these for better resilience in production environments.
Example implementation using AbortController for timeout:
async function fetchWithTimeout(url: string, options: RequestInit, timeoutMs = 10000): Promise<Response> { const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), timeoutMs); try { const response = await fetch(url, { ...options, signal: controller.signal, }); return response; } finally { clearTimeout(timeout); } }Then replace fetch calls:
- const response = await fetch(`https://do.featurebase.app/v2/organization/identifyUser?id=${stackAuthUserId}`, { + const response = await fetchWithTimeout(`https://do.featurebase.app/v2/organization/identifyUser?id=${stackAuthUserId}`, {Also applies to: 64-64, 103-115, 138-145
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/middleware.ts(0 hunks)packages/stack-shared/src/utils/featurebase.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- docs/middleware.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/stack-shared/src/utils/featurebase.tsx (2)
packages/stack-shared/src/utils/errors.tsx (1)
StackAssertionError(69-85)packages/stack-shared/src/utils/env.tsx (1)
getEnvVariable(16-58)
🪛 Biome (2.1.2)
packages/stack-shared/src/utils/featurebase.tsx
[error] 164-164: This code will never be reached ...
... because either this statement will throw an exception, ...
... this statement will return from the function, ...
... this statement will throw an exception, ...
... or this statement will throw an exception beforehand
(lint/correctness/noUnreachable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: claude-review
- GitHub Check: all-good
- GitHub Check: docker
- GitHub Check: Security Check
🔇 Additional comments (2)
packages/stack-shared/src/utils/featurebase.tsx (2)
196-199: Good fix for error handling consistencyThe error handling now properly throws a
StackAssertionErrorinstead of silently allowing undefined email to be returned. This ensures the function contract is maintained and callers receive consistent error behavior.
224-224: Verify email is never undefinedThe function signature promises to return
email: string, but ifensuredEmailremains undefined after the fallback email creation fails, this would violate the contract. However, the fix in lines 196-199 properly throws an error, ensuring this function never returns undefined email.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/stack-shared/src/utils/featurebase.tsx (3)
92-95: Fix inconsistent error handlingThe error handling pattern here differs from the other helper functions. This function logs errors and returns
null, whilefindFeaturebaseUserByIdre-throwsStackAssertionErrorand wraps other errors.This was already identified in previous reviews. The inconsistent error handling should be fixed to match the pattern used in
findFeaturebaseUserById.
102-131: Add try-catch wrapper for network failuresThe fetch call is not wrapped in a try-catch block, which means network failures (DNS errors, timeouts, connection issues) will result in unhandled promise rejections rather than consistent
StackAssertionErrorresponses.This issue was already identified in previous reviews. The fetch call should be wrapped in a try-catch block following the same pattern as the other helper functions.
137-165: Add try-catch wrapper for network failuresSimilar to
createFeaturebaseUser, this function lacks proper error handling for network failures that occur before the response is received.This issue was already identified in previous reviews. The fetch call should be wrapped in a try-catch block to handle network failures consistently.
🧹 Nitpick comments (1)
packages/stack-shared/src/utils/featurebase.tsx (1)
175-178: Consider adding JSDoc parameter documentationThe function has good JSDoc documentation for behavior, but could benefit from parameter documentation for better IDE support.
/** * Get or create a Featurebase user based on Stack Auth user data. * This function ensures that: * 1. We never change a user's email address on Featurebase * 2. We use Stack Auth user ID as the primary identifier * 3. We handle email conflicts by using fallback emails * 4. We update profile information when needed + * + * @param stackAuthUser - The Stack Auth user data to sync + * @param options - Optional configuration including API key override + * @returns Promise resolving to userId and email of the Featurebase user */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/stack-shared/src/utils/featurebase.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/stack-shared/src/utils/featurebase.tsx (2)
packages/stack-shared/src/utils/errors.tsx (1)
StackAssertionError(69-85)packages/stack-shared/src/utils/env.tsx (1)
getEnvVariable(16-58)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: build (22.x)
- GitHub Check: setup-tests
- GitHub Check: build (22.x)
- GitHub Check: restart-dev-and-test
- GitHub Check: lint_and_build (latest)
- GitHub Check: all-good
- GitHub Check: docker
- GitHub Check: docker
- GitHub Check: claude-review
- GitHub Check: Security Check
🔇 Additional comments (3)
packages/stack-shared/src/utils/featurebase.tsx (3)
188-199: Good fix for error handlingThe error handling in this section has been improved compared to previous versions. The function now properly throws a
StackAssertionErrorwhen fallback email creation fails, ensuring that callers receive a defined email or an error.
229-241: Validate user creation logicThe logic for creating new users looks correct:
- Uses primary email if available, otherwise fallback email
- Checks for email conflicts and uses fallback if needed
- Generates a reasonable display name from available data
The fallback display name logic using
primaryEmail?.split('@')[0]is a good approach.
222-225: VerifyensuredEmailis always defined before returnPlease double-check that in every execution path
ensuredEmailis either set to a non-empty string or the function exits with an error—there should be no way to hit thereturn { … email: ensuredEmail }(lines 222–225) withensuredEmailstillundefined. In particular:
- Review the
try { … } catch { … }block around lines 197–199 to ensure any failure in generating or fetching the fallback email always throws.- If there’s an
if (!ensuredEmail)guard, confirm it either assigns a valid default or throws before reaching the return.- Run through edge cases (e.g. existing user has no email and fallback generator returns an empty string) to make sure they’re all caught.
This comment was marked as outdated.
This comment was marked as outdated.
|
Claude finished @madster456's task —— View job Code Review for Stack Companion FeatureI've conducted a comprehensive code review of this PR. Here's my analysis plan: Todo List:
SummaryThis PR introduces a comprehensive Stack Companion feature with a collapsible sidebar containing documentation, feature requests, changelog, and support sections. The implementation includes both frontend React components and backend API routes for managing feature requests through Featurebase integration. Major Issues Found🔴 Critical Security Issues
🟠 Moderate Issues
🟡 Minor Issues
Code Quality Assessment✅ Good Practices
❌ Areas for Improvement
Security AnalysisConcerns Identified:
Recommendations:
Performance ConsiderationsIssues:
Optimizations:
Test Coverage❌ MAJOR ISSUE: No tests found for any of the new functionality:
Critical Recommendation: Add comprehensive test coverage before merging: # Suggested test structure
/tests/
├── components/stack-companion.test.tsx
├── components/feature-request-board.test.tsx
├── api/feature-requests.test.ts
└── e2e/stack-companion-flow.test.tsFinal RecommendationsBefore Merging:
Post-Merge Improvements:
Code Quality Score: 6.5/10Strengths: Well-architected, good UX design, proper React patterns The feature adds significant value but needs security and performance improvements before production deployment. |
Important
Introduces Stack Companion with a right-side panel for docs, feature requests, changelog, and support, along with a new feedback form and improved feature request handling.
StackCompanioncomponent for right-side panel with Docs, Feature Requests, Changelog, and Support insidebar-layout.tsxandstack-companion.tsx.FeedbackFormcomponent infeedback-form.tsxwith success/error states and contact links.GET,POST, andupvoteroutes inroute.tsxand[featureRequestId]/upvote/route.tsxfor feature requests with SSO and upvote syncing.FeatureRequestBoardcomponent infeature-request-board.tsxfor managing feature requests.ChangelogWidgetcomponent inchangelog-widget.tsxto display recent updates.version-check.tsand updatesVersionAlerterinversion-alerter.tsx.featurebase-attachments.cominnext.config.mjs.FeedbackDialoganddocs/middleware.ts.This description was created by
for 8baf5e1. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Improvements
Removals