feat: user profiles with timezone storage#2486
Conversation
🦋 Changeset detectedLatest commit: 6659469 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
bc014be to
66b0924
Compare
66b0924 to
2cffa12
Compare
…d upsertUserProfile
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
16862d0 to
58d8b92
Compare
There was a problem hiding this comment.
PR Review Summary
(0) New Issues | (1) Pending | Risk: Low ✅
✅ Delta Review: Docs Build Fix
The delta since the last automated review (4938b99e2) is a single commit (109051b37) that fixes the docs build by registering the new User Profile OpenAPI tag.
| File | Change | Assessment |
|---|---|---|
agents-api/src/openapi.ts:40 |
Added 'User Profile': 'Operations for managing user profile' |
✅ Correct — Required for OpenAPI spec generation |
agents-docs/scripts/generate-openapi-docs.ts:51 |
Added 'User Profile': 'LuUser' |
✅ Correct — Required for docs icon mapping |
Why this was needed: The docs generator (generate-openapi-docs.ts) validates that all tags in TagToDescription have corresponding icons in TitleToIcon (lines 96-99). Without this change, the build would fail with Unknown icon for tag "User Profile".
🕐 Pending Recommendations (1)
The following item from prior reviews remains unaddressed:
- 🟠
systemNo documentation for user profile feature — API endpoints (GET/PUT /api/users/{userId}/profile), profile settings page (/{tenantId}/profile), and timezone behavior should be documented
📋 Feature Status Summary
All prior Critical and Major issues have been addressed:
| Issue | Status |
|---|---|
🔴 Missing sessionAuth() middleware |
✅ Fixed |
| 🟠 Loading state renders blank page | ✅ Fixed (skeleton added) |
🟠 Human feedback: move to afterAcceptInvitation |
✅ Fixed |
| 🟠 Missing test coverage | ✅ Fixed (215 lines of tests) |
| 🟡 Changeset | ✅ Added |
| 🟡 Toast notifications for save feedback | ✅ Added |
| 🟡 OpenAPI tag registration | ✅ Fixed (this delta) |
✅ APPROVE
Summary: The delta correctly registers the User Profile tag in both the OpenAPI spec and the docs icon mapping, fixing the docs build. The feature implementation is complete with proper authentication, authorization tests, loading UX, toast feedback, and now correct OpenAPI documentation generation. The only remaining item is user-facing documentation, which is polish rather than a blocker. Ship it! 🚀
Discarded (0)
No discarded items in this delta review.
Reviewers (0)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
| Total | 0 | 0 | 0 | 0 | 0 | 1 | 0 |
Note: Trivial delta review — direct assessment without dispatching specialized reviewers. The 2-line change correctly fixes the docs build by registering the new OpenAPI tag.
There was a problem hiding this comment.
PR Review Summary
(0) New Issues | (1) Pending | Risk: Low ✅
✅ Delta Review: OpenAPI Snapshot Update
The delta since the last automated review (109051b37) is a single commit (359c126fa) that updates the OpenAPI snapshot file.
| File | Change | Assessment |
|---|---|---|
agents-api/__snapshots__/openapi.json |
+4 lines — added User Profile tag entry |
✅ Expected — Auto-generated snapshot reflecting the openapi.ts changes from the previous commit |
This is a trivial, mechanical change with no review findings.
🕐 Pending Recommendations (1)
The following item from prior reviews remains unaddressed:
- 🟠
systemNo documentation for user profile feature — API endpoints (GET/PUT /api/users/{userId}/profile), profile settings page (/{tenantId}/profile), and timezone behavior should be documented
📋 Feature Status Summary
All prior Critical and Major issues have been addressed:
| Issue | Status |
|---|---|
🔴 Missing sessionAuth() middleware |
✅ Fixed |
| 🟠 Loading state renders blank page | ✅ Fixed (skeleton added) |
🟠 Human feedback: move to afterAcceptInvitation |
✅ Fixed |
| 🟠 Missing test coverage | ✅ Fixed (215 lines of tests) |
| 🟡 Changeset | ✅ Added |
| 🟡 Toast notifications for save feedback | ✅ Added |
| 🟡 OpenAPI tag registration | ✅ Fixed |
| 🟡 OpenAPI snapshot | ✅ Updated (this delta) |
✅ APPROVE
Summary: Trivial delta — just an OpenAPI snapshot update reflecting the tag registration from the previous commit. The feature implementation is complete with proper authentication, authorization tests, loading UX, toast feedback, and correct OpenAPI documentation generation. The only remaining item is user-facing documentation, which is polish rather than a blocker. Ship it! 🚀
Discarded (0)
No discarded items in this delta review.
Reviewers (0)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
| Total | 0 | 0 | 0 | 0 | 0 | 1 | 0 |
Note: Trivial delta review — no specialized reviewers dispatched. The 4-line change is an auto-generated OpenAPI snapshot update with no substantive code changes.
There was a problem hiding this comment.
PR Review Summary
(1) New Issue | (1) Pending | Risk: Medium
✅ Delta Review: Backfill Migration Added
The delta since the last automated review (359c126fa) is a single commit (6d410995e) that adds a significant architectural change:
| Change | Assessment |
|---|---|
| Remove GET auto-creation logic | |
| Add migration backfill INSERT | ✅ Correct — Ensures existing users have profiles |
This addresses @robert-inkeep's question about backfilling by ensuring all existing users get a profile row during migration deployment.
🔴❗ Critical (1) ❗🔴
🔴 1) userProfile.test.ts:70-85 Stale test will fail — tests removed auto-creation behavior
Issue: The Auto-creation describe block tests that GET auto-creates profiles when none exist. This behavior was removed in this delta — GET now returns 404 instead.
Why: This test will fail on CI. The test expects:
res.statusto be200→ now404upsertUserProfileMock.toHaveBeenCalled()→ no longer true
Fix: Update the test to verify the new 404 behavior instead of auto-creation.
Refs:
Inline Comments:
- 🔴 Critical:
userProfile.test.ts:85Stale test tests removed auto-creation behavior
🟡 Minor (1) 🟡
🟡 1) 0019_easy_bedlam.sql:24 Missing newline at EOF
Issue: File doesn't end with a newline character.
Inline Comments:
- 🟡 Minor:
0019_easy_bedlam.sql:24Missing newline at end of file
💭 Consider (1) 💭
💭 1) 0019_easy_bedlam.sql:15 ID format inconsistency (cosmetic)
Issue: Migration generates hex-based IDs (0-9a-f) while app uses nanoid (0-9a-z). Functionally correct but visually different.
Inline Comments:
- 💭 Consider:
0019_easy_bedlam.sql:15ID format consistency with application code
🕐 Pending Recommendations (1)
The following item from prior reviews remains unaddressed:
- 🟠
systemNo documentation for user profile feature — API endpoints, profile settings page, and timezone behavior should be documented
🚫 REQUEST CHANGES
Summary: The delta adds a necessary backfill migration addressing @robert-inkeep's question, but introduces a breaking test — the Auto-creation test block now tests behavior that was removed. This will cause CI failures. The test needs to be updated to verify the new 404 behavior before merging. The backfill approach is architecturally sound (migration for existing users + afterAcceptInvitation hook for new users).
Discarded (0)
No discarded items in this delta review.
Reviewers (0)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
| Total | 0 | 1 | 1 | 0 | 3 | 1 | 0 |
Note: Small-scope delta review performed directly by orchestrator without dispatching specialized reviewers. The critical finding (stale test) was identified through code inspection.
| }); | ||
| const body = await res.json(); | ||
| expect(body.userId).toBe('test-user-123'); | ||
| }); |
There was a problem hiding this comment.
🔴 CRITICAL: Stale test — Auto-creation describe block tests removed behavior
Issue: This test verifies that GET auto-creates a profile when none exists (expects 200 status and upsertUserProfileMock to be called). However, the delta commit backfill in migration script removed this auto-creation logic from the GET endpoint — it now throws a 404 error instead.
Why: This test will fail after the delta change. The test expects:
res.statusto be200→ actual behavior is now404upsertUserProfileMock.toHaveBeenCalledWith(...)→ no longer called
Fix: Update the test to reflect the new behavior:
describe('Missing profile', () => {
it('should return 404 when no profile exists', async () => {
getUserProfileMock.mockResolvedValue(null);
const res = await userProfileRoutes.request('/test-user-123/profile');
expect(res.status).toBe(404);
const body = await res.json();
expect(body.error.message).toContain('User profile not found');
expect(upsertUserProfileMock).not.toHaveBeenCalled();
});
});Refs:
| FROM "user" u | ||
| WHERE NOT EXISTS ( | ||
| SELECT 1 FROM "user_profile" up WHERE up."user_id" = u."id" | ||
| ); No newline at end of file |
There was a problem hiding this comment.
🟡 Minor: Missing newline at end of file
Issue: The file doesn't end with a newline character, which triggers POSIX-compliance warnings in some editors and diff tools.
Fix: Add a newline after the closing ; on line 24.
| --> statement-breakpoint | ||
| INSERT INTO "user_profile" ("id", "user_id", "timezone", "attributes", "created_at", "updated_at") | ||
| SELECT | ||
| substr(replace(gen_random_uuid()::text, '-', ''), 1, 21), |
There was a problem hiding this comment.
💭 Consider: ID format consistency with application code
Issue: The backfill generates IDs using substr(replace(gen_random_uuid()::text, '-', ''), 1, 21) which produces hexadecimal strings (0-9, a-f). The application's generateId() uses nanoid with a different alphabet (a-z, 0-9).
Why: While both formats work correctly (text columns, 21 chars, unique), backfilled profiles will have visually different ID patterns than profiles created through the application. This is a cosmetic inconsistency, not a functional issue.
Options:
- Accept as-is — IDs are functionally correct; mixing formats is harmless
- Create a PostgreSQL function matching the app's nanoid alphabet for consistency (more complex)
Recommend option 1 unless there's a specific need for ID format consistency across all records.
Refs:
There was a problem hiding this comment.
PR Review Summary
(0) New Issues | (1) Pending | Risk: Low ✅
✅ Delta Review: Prior Feedback Addressed
The delta since the last automated review (6d410995e) is a single commit (3457c3a63) that fixes both issues raised in the prior review:
| Prior Issue | Status | Implementation |
|---|---|---|
| 🔴 Stale test tests removed auto-creation behavior | ✅ Fixed | Test renamed from Auto-creation to Missing profile, now verifies 404 response and upsertUserProfileMock is not called |
| 🟡 Missing newline at EOF | ✅ Fixed | Migration file now ends with newline at line 25 |
Test verification:
- ✅ Correct status assertion:
expect(res.status).toBe(404) - ✅ Correct error message check:
expect(body.error.message).toContain('User profile not found') - ✅ Correct mock assertion:
expect(upsertUserProfileMock).not.toHaveBeenCalled()
🕐 Pending Recommendations (1)
The following item from prior reviews remains unaddressed:
- 🟠
systemNo documentation for user profile feature — API endpoints (GET/PUT /api/users/{userId}/profile), profile settings page (/{tenantId}/profile), and timezone behavior should be documented
📋 Feature Status Summary
All prior Critical and Major issues have been addressed:
| Issue | Status |
|---|---|
🔴 Missing sessionAuth() middleware |
✅ Fixed |
| 🔴 Stale test for auto-creation behavior | ✅ Fixed (this delta) |
| 🟠 Loading state renders blank page | ✅ Fixed (skeleton added) |
🟠 Human feedback: move to afterAcceptInvitation |
✅ Fixed |
| 🟠 Missing test coverage | ✅ Fixed (210 lines of tests) |
| 🟡 Changeset | ✅ Added |
| 🟡 Toast notifications for save feedback | ✅ Added |
| 🟡 OpenAPI tag registration | ✅ Fixed |
| 🟡 Missing newline at EOF | ✅ Fixed (this delta) |
| 💭 ID format consistency in migration | Accepted as-is (cosmetic) |
✅ APPROVE
Summary: The delta successfully addresses both issues from the prior review — the stale test now correctly verifies 404 behavior, and the migration file has the required trailing newline. The feature implementation is complete with proper authentication, authorization tests, loading UX, toast feedback, backfill migration, and correct OpenAPI documentation generation. The only remaining item is user-facing documentation, which is polish rather than a blocker. Ship it! 🚀
Discarded (0)
No discarded items in this delta review.
Reviewers (0)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
| Total | 0 | 0 | 0 | 0 | 0 | 1 | 0 |
Note: Trivial delta review — no specialized reviewers dispatched. The 2-file, 6-line change directly addresses prior feedback with no new issues.
There was a problem hiding this comment.
PR Review Summary
(0) New Issues | (1) Pending | Risk: Low ✅
✅ Delta Review: Main Merge Only
The delta since the last automated review (3457c3a63) consists of 3 commits — a merge from main plus the 2 commits it brought in:
| Commit | Description |
|---|---|
3488fd68e |
Merge branch 'main' into feat/user-profiles |
9f5635adf |
docs: Document tool approval denial reason propagation (#2520) |
31b5e8ba4 |
Fix tool approval denial reason not propagating (#2500) |
User-profile feature code: No changes. The 8 files in the delta are all related to tool approval denial reason propagation — completely unrelated to the user-profile feature.
🕐 Pending Recommendations (1)
The following item from prior reviews remains unaddressed:
- 🟠
systemNo documentation for user profile feature — API endpoints (GET/PUT /api/users/{userId}/profile), profile settings page (/{tenantId}/profile), and timezone behavior should be documented
📋 Feature Status Summary
All prior Critical and Major issues have been addressed:
| Issue | Status |
|---|---|
🔴 Missing sessionAuth() middleware |
✅ Fixed |
| 🔴 Stale test for auto-creation behavior | ✅ Fixed |
| 🟠 Loading state renders blank page | ✅ Fixed (skeleton added) |
🟠 Human feedback: move to afterAcceptInvitation |
✅ Fixed |
| 🟠 Missing test coverage | ✅ Fixed (210 lines of tests) |
| 🟠 Backfill migration for existing users | ✅ Fixed |
| 🟡 Changeset | ✅ Added |
| 🟡 Toast notifications for save feedback | ✅ Added |
| 🟡 OpenAPI tag registration | ✅ Fixed |
| 🟡 Missing newline at EOF | ✅ Fixed |
✅ APPROVE
Summary: No user-profile code changes in this delta — only a main merge bringing in unrelated tool approval denial reason propagation work. The feature implementation is complete with proper authentication, authorization tests, loading UX, toast feedback, backfill migration, and correct OpenAPI documentation generation. The only remaining item is user-facing documentation, which is polish rather than a blocker. Ship it! 🚀
Discarded (0)
No discarded items in this delta review.
Reviewers (0)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
| Total | 0 | 0 | 0 | 0 | 0 | 1 | 0 |
Note: Trivial delta review — no specialized reviewers dispatched. The delta contains only a main merge with no user-profile code changes.
There was a problem hiding this comment.
PR Review Summary
(0) New Issues | (1) Pending | Risk: Low ✅
✅ Delta Review: Main Merge Only
The delta since the last automated review (3488fd68e) consists of 2 commits — a merge from main plus a trigger page UI optimization PR:
| Commit | Description |
|---|---|
665946937 |
Merge branch 'main' into feat/user-profiles |
19e16cc9f |
Trigger page UI optimizations (#2253) |
User-profile feature code: No changes. The 12 files in the delta are all related to trigger page UI optimizations (Suspense boundaries, RSC components, nuqs for tab switching) — completely unrelated to the user-profile feature.
🕐 Pending Recommendations (1)
The following item from prior reviews remains unaddressed:
- 🟠
systemNo documentation for user profile feature — API endpoints (GET/PUT /api/users/{userId}/profile), profile settings page (/{tenantId}/profile), and timezone behavior should be documented
📋 Feature Status Summary
All prior Critical and Major issues have been addressed:
| Issue | Status |
|---|---|
🔴 Missing sessionAuth() middleware |
✅ Fixed |
| 🔴 Stale test for auto-creation behavior | ✅ Fixed |
| 🟠 Loading state renders blank page | ✅ Fixed (skeleton added) |
🟠 Human feedback: move to afterAcceptInvitation |
✅ Fixed |
| 🟠 Missing test coverage | ✅ Fixed (210 lines of tests) |
| 🟠 Backfill migration for existing users | ✅ Fixed |
| 🟡 Changeset | ✅ Added |
| 🟡 Toast notifications for save feedback | ✅ Added |
| 🟡 OpenAPI tag registration | ✅ Fixed |
| 🟡 Missing newline at EOF | ✅ Fixed |
✅ APPROVE
Summary: No user-profile code changes in this delta — only a main merge bringing in unrelated trigger page UI optimizations (Suspense, RSC patterns, nuqs for client-side tabs). The feature implementation is complete with proper authentication, authorization tests, loading UX, toast feedback, backfill migration, and correct OpenAPI documentation generation. The only remaining item is user-facing documentation, which is polish rather than a blocker. Ship it! 🚀
Discarded (0)
No discarded items in this delta review.
Reviewers (0)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
| Total | 0 | 0 | 0 | 0 | 0 | 1 | 0 |
Note: Trivial delta review — no specialized reviewers dispatched. The delta contains only a main merge with no user-profile code changes.
|
🔎💬 Inkeep AI search and chat service is syncing content for source 'Inkeep Agent Framework Docs' |
|
No documentation updates required for this PR. This feature introduces new user profile functionality (timezone storage, profile settings page, API endpoints) that isn't covered by any existing documentation. If documentation for user profiles is desired in the future, it would need to be created as net-new content rather than updates to existing pages. |
Summary
user_profiletable (one-to-one with Better Auth'susertable) withtimezone(IANA string, nullable) andattributes(jsonb, extensible catch-all)user.create.afterhook on registration/[tenantId]/settings/profilewith searchable timezone pickerTest plan
🤖 Generated with Claude Code