feat: medium-priority polish (#59, #60, #62) + fix auth error envelope#67
Merged
Conversation
…elope - mobile: SettingsScreen (profile + sign-out) wired into AppNavGraph (#59) - web: dashboard page fetches /api/v1/me, ProfileCard Server Component (#60) - backend: /debug/pprof/* routes gated by LocalNetworkOnly() (#62) - fix: FirebaseAuth middleware now returns nested error envelope {"error":{"code":"UNAUTHORIZED","message":"..."}} instead of flat string so mobile ApiErrorResponse can deserialise 401 responses correctly - docs: routing.md, middleware.md, data-fetching.md, architecture.md, compose-conventions.md all updated; web/docs/routing.md stale font fixed
github-actions
Bot
added
area: backend
|
Warning Review limit reached
More reviews will be available in 46 minutes and 20 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe PR updates backend auth error responses and pprof routing, adds a mobile settings screen and navigation path, introduces web profile fetching and dashboard rendering, and marks template status items complete. ChangesBackend transport updates
Mobile settings flow
Web profile dashboard flow
Template status note
Sequence Diagram(s)Backend pprof access flow: sequenceDiagram
participant Client
participant Handler
participant LocalNetworkOnly
participant netHttpPprof
Handler->>LocalNetworkOnly: wrap /debug/pprof group
Client->>Handler: GET /debug/pprof/heap
Handler->>LocalNetworkOnly: enforce local-network access
alt loopback/private IP
LocalNetworkOnly->>netHttpPprof: dispatch heap handler
netHttpPprof-->>Client: 200 OK
else public IP
LocalNetworkOnly-->>Client: 403 Forbidden
end
Web dashboard profile flow: sequenceDiagram
participant DashboardPage
participant fetchUserProfile
participant GoBackend
participant ProfileCard
DashboardPage->>DashboardPage: build fallbackProfile from session
DashboardPage->>fetchUserProfile: request current-user profile
fetchUserProfile->>GoBackend: GET /api/v1/me with X-User-Id
GoBackend-->>fetchUserProfile: { data: UserProfile }
fetchUserProfile-->>DashboardPage: resolved profile
alt fetchUserProfile fails
DashboardPage->>ProfileCard: render fallbackProfile
else fetchUserProfile succeeds
DashboardPage->>ProfileCard: render backend profile
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/docs/middleware.md (1)
100-100: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale FirebaseAuth error-body doc — update to the nested envelope.
This PR changed both unauthorized branches in
auth.goto return{"error":{"code":"...","message":"..."}}, but this line still documents the flat{"error": "..."}body. Update it so the docs match the new envelope the mobile client parses.📝 Suggested doc update
-- On failure (missing header, malformed header, or token verification error): aborts with `401 Unauthorized` and a JSON body `{"error": "..."}`. +- On failure (missing header, malformed header, or token verification error): aborts with `401 Unauthorized` and a JSON body `{"error":{"code":"UNAUTHORIZED","message":"..."}}`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/docs/middleware.md` at line 100, The unauthorized response documentation is stale and still describes a flat error body, while auth.go now returns the nested envelope parsed by the mobile client. Update the middleware docs entry for the failure path to match the new JSON shape used by the unauthorized branches in auth.go, referencing the FirebaseAuth middleware behavior and the returned error envelope.
🧹 Nitpick comments (3)
web/app/(dashboard)/__tests__/dashboard.test.tsx (1)
56-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBrittle class assertion.
toContain('mono')would also match unintended tokens; asserting the exactfont-monotoken is more robust.- expect(uidEl.className).toContain('mono') + expect(uidEl.className).toContain('font-mono')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/`(dashboard)/__tests__/dashboard.test.tsx around lines 56 - 57, The test in the dashboard suite uses a brittle class-name assertion that can match unintended tokens; update the expectation on uidEl to verify the exact font-mono token instead of a partial mono substring. Locate the assertion around uidEl in the dashboard test and make it assert the precise monospace class so the check is specific and stable.web/app/(dashboard)/dashboard/page.tsx (1)
24-29: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider logging the swallowed fetch error.
The bare
catch {}discards the error entirely, so backend outages on the profile path become invisible in server logs. Aconsole.error(or your structured logger) preserves observability without changing the fallback behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/`(dashboard)/dashboard/page.tsx around lines 24 - 29, The fetchUserProfile fallback in dashboard/page.tsx is swallowing the error in a bare catch, so backend failures on the profile path are not visible in logs. Update the try/catch around fetchUserProfile(fallbackProfile.uid) to capture the thrown error and log it with console.error or the existing structured logger before assigning fallbackProfile, while keeping the current fallback behavior unchanged.web/docs/data-fetching.md (1)
151-151: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTypo in rule text:
.res.json()should be.json().-- Always assert `res.ok` before calling `.res.json()`. +- Always assert `res.ok` before calling `.json()`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/docs/data-fetching.md` at line 151, The rule text in the data fetching docs has a typo: the response parsing method is written as .res.json() instead of .json(). Update the sentence in the affected documentation copy to use the correct method name so the guidance matches the actual Response API.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/internal/transport/handlers/routes.go`:
- Around line 59-70: The pprof protection in LocalNetworkOnly is bypassable
because it trusts gin.Context.ClientIP(), which can be influenced by
X-Forwarded-For when the Gin engine has no trusted proxy restriction. Fix this
by tightening proxy trust in the routes setup for the engine (for example via
SetTrustedProxies) or by changing LocalNetworkOnly to base the internal-network
check on c.Request.RemoteAddr instead of ClientIP(). Update the routes.go pprof
group and the middleware symbol accordingly so the restricted debug handlers
cannot be reached through spoofed forwarded headers.
In
`@mobile/app/src/androidTest/java/com/company/template/settings/SettingsScreenTest.kt`:
- Around line 50-62: The null-state test is matching a shared dash placeholder
text twice, so `onNodeWithText("—")` becomes ambiguous in
`settingsScreen_nullDisplayNameAndEmail_showsDashes`. Update this test to verify
the number of matching nodes instead of asserting a single node, using the
existing `composeTestRule` and `SettingsScreen` setup. Keep the placeholder
check tied to the two null fields by counting all nodes with the dash text and
asserting the expected total. If needed, add the matching Compose testing import
for the count-based assertion.
In `@web/app/`(dashboard)/dashboard/page.tsx:
- Around line 17-29: The profile lookup in dashboard/page.tsx should not call
fetchUserProfile when session.user?.id is missing, because fallbackProfile.uid
can be empty and causes an ambiguous backend request. Update the logic around
fallbackProfile and the fetchUserProfile call to guard on a non-empty uid first,
and only attempt the fetch when a real user id exists; otherwise keep using
fallbackProfile directly.
In `@web/docs/routing.md`:
- Around line 55-65: The directory-tree fenced block in the routing docs is
missing a language identifier, so update the Markdown fence used in the app
routes overview to specify text for the tree snippet. Locate the fenced block
that shows the app/(auth) and app/(dashboard) structure and change the opening
fence to a language-tagged one so it satisfies MD040 while keeping the content
unchanged.
In `@web/lib/user-profile.ts`:
- Around line 40-41: The user profile fetch in userProfile currently returns
body.data without verifying the ApiEnvelope shape, so a successful response with
missing or empty data can leak undefined into ProfileCard. Add a minimal
presence check in userProfile after res.json() to confirm body and body.data are
present before returning, and throw or reject through the existing try/catch
path when the envelope is invalid so callers fall back cleanly.
---
Outside diff comments:
In `@backend/docs/middleware.md`:
- Line 100: The unauthorized response documentation is stale and still describes
a flat error body, while auth.go now returns the nested envelope parsed by the
mobile client. Update the middleware docs entry for the failure path to match
the new JSON shape used by the unauthorized branches in auth.go, referencing the
FirebaseAuth middleware behavior and the returned error envelope.
---
Nitpick comments:
In `@web/app/`(dashboard)/__tests__/dashboard.test.tsx:
- Around line 56-57: The test in the dashboard suite uses a brittle class-name
assertion that can match unintended tokens; update the expectation on uidEl to
verify the exact font-mono token instead of a partial mono substring. Locate the
assertion around uidEl in the dashboard test and make it assert the precise
monospace class so the check is specific and stable.
In `@web/app/`(dashboard)/dashboard/page.tsx:
- Around line 24-29: The fetchUserProfile fallback in dashboard/page.tsx is
swallowing the error in a bare catch, so backend failures on the profile path
are not visible in logs. Update the try/catch around
fetchUserProfile(fallbackProfile.uid) to capture the thrown error and log it
with console.error or the existing structured logger before assigning
fallbackProfile, while keeping the current fallback behavior unchanged.
In `@web/docs/data-fetching.md`:
- Line 151: The rule text in the data fetching docs has a typo: the response
parsing method is written as .res.json() instead of .json(). Update the sentence
in the affected documentation copy to use the correct method name so the
guidance matches the actual Response API.
🪄 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: b071c0ae-1d75-4621-8e77-6d63d380f2f1
📒 Files selected for processing (20)
TEMPLATE_STATUS.mdbackend/docs/middleware.mdbackend/docs/routing.mdbackend/internal/transport/handlers/pprof_handler_test.gobackend/internal/transport/handlers/routes.gobackend/internal/transport/middleware/auth.gobackend/internal/transport/middleware/auth_test.gomobile/app/src/androidTest/java/com/company/template/settings/SettingsScreenTest.ktmobile/app/src/main/java/com/company/template/home/HomeScreen.ktmobile/app/src/main/java/com/company/template/navigation/AppNavGraph.ktmobile/app/src/main/java/com/company/template/settings/SettingsScreen.ktmobile/docs/architecture.mdmobile/docs/compose-conventions.mdweb/app/(dashboard)/__tests__/dashboard.test.tsxweb/app/(dashboard)/dashboard/ProfileCard.tsxweb/app/(dashboard)/dashboard/page.tsxweb/docs/data-fetching.mdweb/docs/routing.mdweb/lib/user-profile.test.tsweb/lib/user-profile.ts
- security: LocalNetworkOnly() now uses RemoteAddr instead of ClientIP() to prevent X-Forwarded-For spoofing bypass; added spoofing regression test - mobile test: use onAllNodesWithText to handle two dash nodes in null state - web: skip backend fetch when uid is empty to avoid ambiguous round-trip - web: validate body.data presence before returning from fetchUserProfile - docs: add "text" language tag to fenced code block in web/docs/routing.md
This was referenced Jun 25, 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
SettingsScreen— stateless composable showing displayName, email, avatar initial, and a full-width error-coloured sign-out button; wired intoAppNavGraphviaROUTE_SETTINGS; SettingsTextButtonadded toHomeScreenGET /api/v1/meserver-side, unwraps{ data: ... }envelope, rendersProfileCard(display name, email, uid); falls back to NextAuth session data if backend is unreachable/debug/pprof/*routes registered in Gin withLocalNetworkOnly()at group level — always gated, 7 endpoints viagin.WrapF, excluded from Swaggerbackend/docs/streams.mdalready documents Redis consumers as opt-inBug fix (found during testing)
FirebaseAuthmiddleware was returning a flat string error body:{"error": "missing or invalid Authorization header"}The mobile
ApiErrorResponseexpects the standard nested envelope:{"error": {"code": "UNAUTHORIZED", "message": "..."}}Updated both error paths in
auth.go; added a body format assertion to the middleware test to prevent regression.Docs updated
backend/docs/routing.md— pprof routes table + note about Swagger exclusionbackend/docs/middleware.md—LocalNetworkOnlyconsumers listweb/docs/routing.md— fixed stale font name (Manrope, not Geist Sans); added real route groupsweb/docs/data-fetching.md— backend fetch utility pattern, envelope unwrapping, fallback patternmobile/docs/architecture.md— ROUTE_SETTINGS, navigation graph sectionmobile/docs/compose-conventions.md— stateless screen signature exampleTest plan
go vet ./...— cleango test ./internal/transport/middleware/...— all pass including new envelope format assertionpnpm lint— 0 errorspnpm build— clean./gradlew lint— BUILD SUCCESSFUL./gradlew test— BUILD SUCCESSFUL./gradlew connectedAndroidTest— requires emulatorSummary by CodeRabbit
New Features
Bug Fixes
Documentation