Add Google OAuth sessions and team access UI#8
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds Google OAuth and session-backed authentication, Prisma-backed sessions and team memberships, RBAC for team visibility and membership roles, auth/session HTTP endpoints and client session bootstrapping, access-management UI and GraphQL mutations, plus docs and environment/config updates. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser
participant AppUI
participant Server
participant Google
User->>AppUI: Click "Sign in with Google"
AppUI->>Server: GET /auth/google/start
Server->>Server: Generate state, build auth URL
Server-->>Browser: 302 redirect + oauth state cookie
Browser->>Google: GET authorization URL
Google-->>Browser: 302 redirect to /auth/google/callback?code=...
Browser->>Server: GET /auth/google/callback?code=...&state=...
Server->>Server: Validate state cookie
Server->>Google: POST /token (exchange code)
Google-->>Server: access_token
Server->>Google: GET /userinfo
Google-->>Server: user profile
Server->>Server: Upsert user, create session record
Server-->>Browser: 302 redirect to appOrigin + session cookie
Browser->>AppUI: Fetch session -> authenticated
sequenceDiagram
participant Browser
participant Server
participant AuthCache as AuthCache
participant Prisma
Browser->>Server: POST /graphql (cookies + headers)
Server->>AuthCache: lookup(Request)
alt cached
AuthCache-->>Server: RequestAuthentication
else not cached
Server->>Server: read session cookie
alt session present
Server->>Prisma: find session by tokenHash
Prisma-->>Server: session + user
Server->>AuthCache: cache session auth
else no session
Server->>Server: check Authorization header
alt valid token
Server->>AuthCache: cache token auth (trusted)
else
Server->>AuthCache: cache unauthenticated
end
end
AuthCache-->>Server: RequestAuthentication
end
Server->>Server: build GraphQL context (viewer, authMode, isTrustedSystem)
Server-->>Browser: GraphQL response (authorization applied)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a comprehensive authentication and authorization system, introducing Google OAuth support and session-based authentication via cookies. It establishes a Role-Based Access Control (RBAC) framework for teams and issues, including visibility settings and membership roles. The updates include database schema changes, new server-side auth routes, and a dedicated Access management page in the web UI. Feedback suggests refining the GraphQL mutation wrapper to ensure authorization errors are correctly propagated and improving the accuracy of error messages when team memberships are not found.
Comment on lines
1140
to
1142
| if (error instanceof GraphQLError || getExposedError(error) || isPrismaInvalidInputError(error)) { | ||
| return fallback; | ||
| } |
There was a problem hiding this comment.
The error handling in runMutation is too broad. The condition error instanceof GraphQLError will catch and suppress authorization errors (e.g., FORBIDDEN, NOT_FOUND) that should be propagated to the client. This prevents the UI from showing specific feedback about why an action failed (e.g. permission denied).
Exposed errors should be re-thrown to be handled by the GraphQL layer, while other specific, non-critical errors can be converted into a { success: false } payload.
if (getExposedError(error)) {
throw error;
}
if (isPrismaInvalidInputError(error)) {
return fallback;
}| }); | ||
|
|
||
| if (!membership) { | ||
| throw createNotFoundError(TEAM_NOT_FOUND_MESSAGE); |
There was a problem hiding this comment.
The error message here is misleading. If a teamMembership is not found, the error thrown is TEAM_NOT_FOUND_MESSAGE, which states that the team could not be found. This could be confusing for debugging or for the client.
It would be more accurate to throw an error indicating that the membership was not found. You might consider adding a new error constant like TEAM_MEMBERSHIP_NOT_FOUND_MESSAGE and using it here.
![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
🧹 Nitpick comments (7)
packages/web/src/lib/apollo.tsx (1)
174-178: Consider adding error handling for malformed GraphQL URLs.
getServerBaseUrl()will throw ifgetGraphqlUrl()returns a malformed URL. WhilegetGraphqlUrl()currently returns valid URLs in all branches, a defensive try-catch could prevent runtime errors if the URL validation logic changes.🛡️ Optional defensive implementation
export function getServerBaseUrl(): string { - const graphqlUrl = new URL(getGraphqlUrl()); - - return graphqlUrl.origin; + try { + const graphqlUrl = new URL(getGraphqlUrl()); + return graphqlUrl.origin; + } catch { + return 'http://localhost:4200'; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/lib/apollo.tsx` around lines 174 - 178, Add defensive error handling around the URL parsing in getServerBaseUrl: wrap the call to new URL(getGraphqlUrl()) in a try-catch, catch any TypeError or RangeError from a malformed URL, and return a safe fallback (e.g., empty string or a sensible default) or rethrow a more descriptive error; update getServerBaseUrl to reference getGraphqlUrl() inside the try block and ensure the catch logs or surfaces the failure clearly so callers know the origin could not be determined.packages/server/src/access-control.test.ts (1)
7-24: Assert that the bypass path never hitsteamMembership.findUnique().These tests only prove that
assertCanWriteTeam()resolves. They would still pass if the implementation started querying membership before short-circuiting, which is exactly the behavior this PR is trying to preserve for trusted/admin requests. Capture the mock and assertexpect(findUnique).not.toHaveBeenCalled().Also applies to: 26-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/access-control.test.ts` around lines 7 - 24, The test for assertCanWriteTeam should also verify the bypass short-circuits membership lookups: capture the mocked teamMembership.findUnique used in the test (the vi.fn() passed into the prisma stub) and add an assertion expect(findUnique).not.toHaveBeenCalled() after awaiting assertCanWriteTeam so the test fails if the implementation queries membership before short-circuiting; apply the same change to the other similar test block covering lines 26-48 that tests trusted/admin bypasses.packages/web/src/App.access.test.tsx (1)
101-129: Assert the mutation payloads, not just that the mocks were called.These assertions still pass if the UI sends the wrong
teamId, role, email, or removes the wrong membership, because the mocks resolve regardless of theirvariables. Verifying the call payloads would also remove the brittle positional lookup on Line 126.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/App.access.test.tsx` around lines 101 - 129, Update the test to assert the actual mutation payloads instead of only that the mocks were called: after the visibility change, verify teamUpdateAccess was called with the expected variables (e.g., { teamId: ..., visibility: 'PUBLIC' }); after adding the member, assert teamMembershipUpsert was called with the correct { teamId, email: 'editor@example.com', name: 'Editor User', role: 'EDITOR' }; and when removing, assert teamMembershipRemove was called with the correct membership identifier/variables rather than using a brittle positional button lookup—use the membership's unique label/text to find the specific Remove button or capture the membership id from the rendered item so you can assert the correct variables were passed to teamMembershipRemove.packages/web/src/lib/session.ts (1)
30-35: Consider returning success/failure status fromlogoutSession().The function silently ignores any errors from the logout request. Callers might want to know if the logout failed (e.g., to show a retry option or warning).
♻️ Optional: return a boolean indicating success
-export async function logoutSession(): Promise<void> { - await fetch(`${getServerBaseUrl()}/auth/logout`, { +export async function logoutSession(): Promise<boolean> { + try { + const response = await fetch(`${getServerBaseUrl()}/auth/logout`, { + credentials: 'include', + method: 'POST', + }); + return response.ok; + } catch { + return false; + } - credentials: 'include', - method: 'POST', - }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/lib/session.ts` around lines 30 - 35, The logoutSession function currently ignores errors and always resolves; update logoutSession to return a Promise<boolean> (or similar success flag) by awaiting fetch(`${getServerBaseUrl()}/auth/logout`, catching network exceptions, and returning false on error, and also checking response.ok (return true for 2xx, false otherwise). Make changes inside logoutSession (and update callers) to propagate the boolean result so callers can react to failures.packages/web/src/test/app-test-helpers.tsx (1)
63-92: Consider extracting duplicated mutation mock logic into a shared function.The mutation mock implementation (handling CommentCreate, CommentDelete, IssueDelete, TeamUpdateAccess, etc.) is duplicated between the hoisted mock (lines 63-91) and the
beforeEachblock (lines 171-198). Extract this to a shared factory to reduce maintenance burden.♻️ Suggested refactor
function createMutationMockImplementation() { return (document: unknown) => { const source = getDocumentSource(document); if (source.includes('mutation CommentCreate')) { return [vi.fn().mockResolvedValue({ data: { commentCreate: { success: true, comment: null } } })]; } // ... rest of the cases return [vi.fn()]; }; } // Then use in both places: const hoistedApolloMocks = vi.hoisted<ApolloMockSet>(() => ({ useQuery: vi.fn(), useMutation: vi.fn(createMutationMockImplementation()), })); // In beforeEach: apolloMocks.useMutation.mockImplementation(createMutationMockImplementation());Also applies to: 171-199
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/test/app-test-helpers.tsx` around lines 63 - 92, Extract the duplicated mutation mock logic used in useMutation into a shared factory function (e.g., createMutationMockImplementation) that returns the implementation callback which inspects getDocumentSource(document) and returns the appropriate vi.fn().mockResolvedValue(...) cases for 'mutation CommentCreate', 'mutation CommentDelete', 'mutation IssueDelete', 'mutation TeamUpdateAccess', 'mutation TeamMembershipUpsert', and 'mutation TeamMembershipRemove', defaulting to [vi.fn()]; then use that factory when creating the hoisted mock (vi.hoisted returning useMutation: vi.fn(createMutationMockImplementation())) and in your beforeEach by calling apolloMocks.useMutation.mockImplementation(createMutationMockImplementation()).packages/web/src/routes/AccessPage.tsx (1)
128-134: Remove unnecessaryasynckeyword fromupdateSelectedTeam.The function is declared
asyncbut doesn't useawaitinternally. This causes callers to unnecessarily await on calls to this function (e.g., lines 145, 170, 211, 253). The function could be synchronous.♻️ Suggested fix
- async function updateSelectedTeam(updater: (team: TeamSummary) => TeamSummary) { + function updateSelectedTeam(updater: (team: TeamSummary) => TeamSummary) { if (!selectedTeam) { return; } setTeams((currentTeams) => currentTeams.map((team) => (team.id === selectedTeam.id ? updater(team) : team))); }Then remove the
awaitfrom calls at lines 145, 170, 211, 253.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/routes/AccessPage.tsx` around lines 128 - 134, Remove the unnecessary async from updateSelectedTeam: change the declaration "async function updateSelectedTeam(updater: (team: TeamSummary) => TeamSummary)" to a plain synchronous function "function updateSelectedTeam(...)" since it does not use await; keep the existing setTeams logic intact. Then update all call sites that currently "await updateSelectedTeam(...)" to call it synchronously (remove the await) so callers no longer await a non-async operation; specifically update every usage of updateSelectedTeam in this module to call it without awaiting.packages/web/src/board/types.ts (1)
5-8: Keep access-page team data separate fromTeamSummary.Making
visibilityandmembershipsoptional on the shared board type weakens the query contract: the access UI can still type-check even if those fields disappear from its selection set. A dedicatedAccessTeamSummarykeeps board queries loose while making access-page requirements explicit.Also applies to: 108-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/board/types.ts` around lines 5 - 8, The shared board-related type currently makes visibility and memberships optional on TeamSummary which weakens query contracts; create a new AccessTeamSummary type that includes visibility: 'PRIVATE'|'PUBLIC' and memberships: { nodes: TeamMembershipSummary[] } (required), revert TeamSummary to not include those optional fields, and update the access-page types/usages to consume AccessTeamSummary instead of TeamSummary (also adjust the other occurrences mentioned around the same block 108-113 to use AccessTeamSummary).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/server/src/auth-routes.ts`:
- Around line 233-253: The respondJson function must mark sensitive auth
responses as non-cacheable: add a Cache-Control header (e.g.,
response.setHeader('Cache-Control', 'no-store')) before ending the response in
respondJson so /auth/session and /auth/logout cannot be cached by browsers or
intermediaries; place this call alongside the existing content-type and
Set-Cookie header logic in respondJson (ensure it executes regardless of
options.setCookie).
In `@packages/server/src/errors.ts`:
- Around line 55-60: The getExposedError function currently allows any
GraphQLError that includes an extensions.code; tighten this by implementing an
allowlist of permitted codes (e.g. ALLOWED_ERROR_CODES) and only return a
GraphQLError when error is an instance of GraphQLError and error.extensions.code
is one of those allowed codes; when returning, normalize the output to a new
GraphQLError that contains only a safe message and the canonical extensions.code
(no other extension fields or original stack), and otherwise return null. Ensure
you update getExposedError to reference the allowlist and perform normalization
before exposing the error.
In `@packages/server/src/google-oauth.ts`:
- Around line 76-104: The two external fetches in
exchangeGoogleCodeForUserProfile (the POST to GOOGLE_TOKEN_URL producing
tokenResponse and the GET to GOOGLE_USER_INFO_URL producing userInfoResponse)
lack timeouts; fix by passing an AbortSignal from AbortSignal.timeout(ms) into
each fetch options so both fetch calls will abort on timeout (choose an
appropriate ms value), ensuring timeout errors propagate to the existing
try/catch in handleGoogleCallback.
- Around line 134-176: When linking/updating Google accounts in google-oauth.ts,
add guards to prevent hijacking: before performing the prisma.user.update for
existingByEmail or existingBySubject, check (1) if
existingByEmail?.googleSubject is set and differs from profile.subject, reject
the login (throw or return an auth error) instead of overwriting googleSubject,
and (2) if both existingBySubject and existingByEmail are non-null but
existingBySubject.id !== existingByEmail.id, reject as a conflict instead of
updating either record. Apply these checks where existingBySubject and
existingByEmail are used and only call prisma.user.update when the validation
passes.
In `@packages/server/src/index.ts`:
- Around line 120-125: Replace the current catch handler that returns
error.message with a safe exposure flow: call getExposedError(error) inside the
catch for the auth-route promise and use its return value for the JSON response
(falling back to a generic "Internal server error" string if getExposedError
returns nothing); also log the original error server-side (e.g., console.error
or existing logger) before sending the response so internal details are recorded
but not echoed to the browser — apply this change to the catch block that sets
response.statusCode/response.setHeader/response.end.
In `@packages/server/src/schema.ts`:
- Around line 700-702: The thrown error message is incorrect: in the membership
check (the block that checks membership and currently throws
createNotFoundError(TEAM_NOT_FOUND_MESSAGE) when `membership` is falsy) replace
the wrong message with a dedicated membership-not-found constant. Add a new
exported constant MEMBERSHIP_NOT_FOUND_MESSAGE (e.g. in errors.ts alongside
TEAM_NOT_FOUND_MESSAGE) with the text "Team membership not found." and update
the resolver to call createNotFoundError(MEMBERSHIP_NOT_FOUND_MESSAGE) when
`membership` is missing.
In `@packages/server/src/session.ts`:
- Around line 33-52: The cookie-parsing logic currently calls decodeURIComponent
which can throw on malformed percent-escapes; update the reducer in the cookie
parsing function (the block that computes cookies[key] =
decodeURIComponent(value)) to wrap decodeURIComponent(value) in a try/catch and
skip that entry on failure (i.e., do not rethrow, just return cookies) so
malformed cookie values are ignored and parsing continues for the rest of the
Cookie header used by readCookieValue / auth/session/logout/google-callback.
In `@packages/web/src/App.tsx`:
- Around line 14-39: The UI wrongly treats session === null as "not configured"
while that value is used for both "loading" and "loaded empty"; update the
session loading logic in the useEffect that calls fetchSessionState (and related
render branches at lines ~61-90) to distinguish loading vs loaded: introduce a
boolean state like isSessionLoaded (or switch session initial value to undefined
and set to null on an unauthenticated response), set it to true in both the
.then and .catch handlers (after calling setSession or setSessionError), and
update the render logic to only show the "Google OAuth not configured" /
unauthenticated sign-in copy when isSessionLoaded is true (or when session ===
null after switching to undefined initial). Ensure you reference and update
session, setSession, sessionError, fetchSessionState, and the useEffect handler
accordingly.
In `@packages/web/src/lib/session.ts`:
- Around line 17-24: fetchSessionState assumes fetch and response.json() always
succeed; wrap the network call and JSON parsing in a try/catch, check
response.ok before parsing, and handle non-JSON or network errors by logging the
error and returning a safe fallback SessionState (e.g., an unauthenticated/empty
state). Update the function fetchSessionState to validate response.ok, guard
against response.json() throwing, and return a default SessionState on any error
so callers never get an unhandled exception.
In `@packages/web/src/styles/app.css`:
- Around line 65-69: The .app-shell__brand-group flex container currently forces
a single row causing overflow on narrow viewports; update its stylesheet so it
can wrap (e.g., add flex-wrap: wrap and adjust gap/align-items) and/or add a
media query at the 780px breakpoint to set .app-shell__brand-group { flex-wrap:
wrap; gap: 0.5rem; align-items: center; } so the brand and nav pills stack/wrap
on small screens alongside .app-shell__nav.
---
Nitpick comments:
In `@packages/server/src/access-control.test.ts`:
- Around line 7-24: The test for assertCanWriteTeam should also verify the
bypass short-circuits membership lookups: capture the mocked
teamMembership.findUnique used in the test (the vi.fn() passed into the prisma
stub) and add an assertion expect(findUnique).not.toHaveBeenCalled() after
awaiting assertCanWriteTeam so the test fails if the implementation queries
membership before short-circuiting; apply the same change to the other similar
test block covering lines 26-48 that tests trusted/admin bypasses.
In `@packages/web/src/App.access.test.tsx`:
- Around line 101-129: Update the test to assert the actual mutation payloads
instead of only that the mocks were called: after the visibility change, verify
teamUpdateAccess was called with the expected variables (e.g., { teamId: ...,
visibility: 'PUBLIC' }); after adding the member, assert teamMembershipUpsert
was called with the correct { teamId, email: 'editor@example.com', name: 'Editor
User', role: 'EDITOR' }; and when removing, assert teamMembershipRemove was
called with the correct membership identifier/variables rather than using a
brittle positional button lookup—use the membership's unique label/text to find
the specific Remove button or capture the membership id from the rendered item
so you can assert the correct variables were passed to teamMembershipRemove.
In `@packages/web/src/board/types.ts`:
- Around line 5-8: The shared board-related type currently makes visibility and
memberships optional on TeamSummary which weakens query contracts; create a new
AccessTeamSummary type that includes visibility: 'PRIVATE'|'PUBLIC' and
memberships: { nodes: TeamMembershipSummary[] } (required), revert TeamSummary
to not include those optional fields, and update the access-page types/usages to
consume AccessTeamSummary instead of TeamSummary (also adjust the other
occurrences mentioned around the same block 108-113 to use AccessTeamSummary).
In `@packages/web/src/lib/apollo.tsx`:
- Around line 174-178: Add defensive error handling around the URL parsing in
getServerBaseUrl: wrap the call to new URL(getGraphqlUrl()) in a try-catch,
catch any TypeError or RangeError from a malformed URL, and return a safe
fallback (e.g., empty string or a sensible default) or rethrow a more
descriptive error; update getServerBaseUrl to reference getGraphqlUrl() inside
the try block and ensure the catch logs or surfaces the failure clearly so
callers know the origin could not be determined.
In `@packages/web/src/lib/session.ts`:
- Around line 30-35: The logoutSession function currently ignores errors and
always resolves; update logoutSession to return a Promise<boolean> (or similar
success flag) by awaiting fetch(`${getServerBaseUrl()}/auth/logout`, catching
network exceptions, and returning false on error, and also checking response.ok
(return true for 2xx, false otherwise). Make changes inside logoutSession (and
update callers) to propagate the boolean result so callers can react to
failures.
In `@packages/web/src/routes/AccessPage.tsx`:
- Around line 128-134: Remove the unnecessary async from updateSelectedTeam:
change the declaration "async function updateSelectedTeam(updater: (team:
TeamSummary) => TeamSummary)" to a plain synchronous function "function
updateSelectedTeam(...)" since it does not use await; keep the existing setTeams
logic intact. Then update all call sites that currently "await
updateSelectedTeam(...)" to call it synchronously (remove the await) so callers
no longer await a non-async operation; specifically update every usage of
updateSelectedTeam in this module to call it without awaiting.
In `@packages/web/src/test/app-test-helpers.tsx`:
- Around line 63-92: Extract the duplicated mutation mock logic used in
useMutation into a shared factory function (e.g.,
createMutationMockImplementation) that returns the implementation callback which
inspects getDocumentSource(document) and returns the appropriate
vi.fn().mockResolvedValue(...) cases for 'mutation CommentCreate', 'mutation
CommentDelete', 'mutation IssueDelete', 'mutation TeamUpdateAccess', 'mutation
TeamMembershipUpsert', and 'mutation TeamMembershipRemove', defaulting to
[vi.fn()]; then use that factory when creating the hoisted mock (vi.hoisted
returning useMutation: vi.fn(createMutationMockImplementation())) and in your
beforeEach by calling
apolloMocks.useMutation.mockImplementation(createMutationMockImplementation()).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a6d78db3-b6f9-40d3-8734-5e1a9dd36a43
📒 Files selected for processing (29)
.env.exampleREADME.mddocs/milestones.mddocs/review/triage.mddocs/vision.mdpackages/server/prisma/schema.prismapackages/server/prisma/seed-helpers.tspackages/server/src/access-control.test.tspackages/server/src/access-control.tspackages/server/src/auth-routes.tspackages/server/src/auth.test.tspackages/server/src/auth.tspackages/server/src/environment.tspackages/server/src/errors.tspackages/server/src/google-oauth.tspackages/server/src/index.test.tspackages/server/src/index.tspackages/server/src/schema.tspackages/server/src/session.tspackages/web/src/App.access.test.tsxpackages/web/src/App.error-states.test.tsxpackages/web/src/App.tsxpackages/web/src/board/queries.tspackages/web/src/board/types.tspackages/web/src/lib/apollo.tsxpackages/web/src/lib/session.tspackages/web/src/routes/AccessPage.tsxpackages/web/src/styles/app.csspackages/web/src/test/app-test-helpers.tsx
Comment on lines
+65
to
+69
| .app-shell__brand-group { | ||
| display: flex; | ||
| align-items: center; | ||
| gap: 1rem; | ||
| } |
There was a problem hiding this comment.
Allow the nav brand group to wrap on small screens.
The 780px breakpoint only stacks .app-shell__nav; .app-shell__brand-group still stays on one row. With the added Access link, the brand + nav pills can overflow narrow viewports instead of wrapping.
Suggested change
.app-shell__brand-group {
display: flex;
align-items: center;
gap: 1rem;
+ flex-wrap: wrap;
}📝 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.
Suggested change
| .app-shell__brand-group { | |
| display: flex; | |
| align-items: center; | |
| gap: 1rem; | |
| } | |
| .app-shell__brand-group { | |
| display: flex; | |
| align-items: center; | |
| gap: 1rem; | |
| flex-wrap: wrap; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/styles/app.css` around lines 65 - 69, The
.app-shell__brand-group flex container currently forces a single row causing
overflow on narrow viewports; update its stylesheet so it can wrap (e.g., add
flex-wrap: wrap and adjust gap/align-items) and/or add a media query at the
780px breakpoint to set .app-shell__brand-group { flex-wrap: wrap; gap: 0.5rem;
align-items: center; } so the brand and nav pills stack/wrap on small screens
alongside .app-shell__nav.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This was referenced Apr 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
Summary by CodeRabbit
New Features
Documentation
Tests
Chores