Standardize Checklist and Note Identity on UUIDs - #561
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR migrates checklist and note identity from filename/category-based ids to UUIDs across routing, UI components, hooks, server actions, and REST APIs. Legacy category-path routes become redirect-only resolvers that map old paths to canonical Sequence Diagram(s)sequenceDiagram
participant Client
participant LegacyRoute as Legacy [...categoryPath] page
participant LegacyResolve as legacyResolve()
participant FileSystem as Markdown file lookup
participant UuidRoute as Canonical [uuid] page
participant ServerAction as getListById/getNoteById
Client->>LegacyRoute: GET /checklist/Category/old-id
LegacyRoute->>LegacyResolve: legacyResolve(mode, category, id)
LegacyResolve->>FileSystem: locate matching file, extract or stamp uuid
FileSystem-->>LegacyResolve: uuid
LegacyResolve-->>LegacyRoute: uuid
LegacyRoute-->>Client: permanentRedirect to /checklist/{uuid}
Client->>UuidRoute: GET /checklist/{uuid}
UuidRoute->>ServerAction: getListById(uuid)
ServerAction-->>UuidRoute: checklist record
UuidRoute-->>Client: render ChecklistClient
sequenceDiagram
participant Owner
participant ShareModal
participant useSharingTools
participant ShareOperations as share/operations.ts
participant ShareQueries as share/queries.ts
participant Recipient
Owner->>ShareModal: select user, set permissions
ShareModal->>useSharingTools: handlePermissionChange(itemUuid, user, perms)
useSharingTools->>ShareOperations: shareItem(mode, itemUuid, user, perms)
ShareOperations->>ShareOperations: write inline sharedWith YAML
ShareOperations->>Recipient: broadcast update and notify
useSharingTools->>ShareQueries: itemShares(itemUuid, itemType)
ShareQueries-->>useSharingTools: current grants, isPublic, inheritedFrom
useSharingTools-->>ShareModal: updated sharing state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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
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: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (12)
app/_server/actions/users/helpers.ts (1)
23-65: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftPer-user UUID owner lookups are still linear scans.
getUserByItemUuidstill walks every user and scans that user’s directory until it finds a match, so note/checklist owner resolution is O(users) filesystem I/O per lookup. This backsgetUserByNoteUuidandgetUserByChecklistUuid, so owner-less lookups in note/checklist/sharing paths will keep getting slower as the user list grows. Add a UUID→owner index or another direct lookup instead of scanning each user directory.🤖 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 `@app/_server/actions/users/helpers.ts` around lines 23 - 65, Replace the per-user directory scan in getUserByItemUuid with a direct UUID-to-owner lookup, using or adding an index that covers the requested itemType and returns the owning User without iterating through users. Preserve the existing Result<User> success/not-found behavior and error handling, and ensure getUserByNoteUuid and getUserByChecklistUuid continue using this optimized path.app/_hooks/useSidebar.tsx (1)
189-204: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStale
.id-based lookup breaks sidebar auto-expand for canonical UUID routes.
isItemSelectedabove (lines 171-180) was correctly updated to match byitem.uuidagainst the new/checklist/{uuid}//note/{uuid}routes, but this effect still extractsitemIdfrompathnameand looks items up by.id. Since the pathname's last segment is now the item'suuid,checklists.find((c) => c.id === itemId)/notes.find((n) => n.id === itemId)will no longer match, soexpandCategoryPathnever fires — the sidebar stops auto-expanding the parent category whenever a checklist/note is opened.🐛 Proposed fix
const itemId = pathname.split("/").pop(); let currentItem: Partial<Checklist> | Partial<Note> | undefined; if (mode === Modes.CHECKLISTS) { - currentItem = checklists.find((c) => c.id === itemId); + currentItem = checklists.find((c) => c.uuid === itemId); } else { - currentItem = notes.find((n) => n.id === itemId); + currentItem = notes.find((n) => n.uuid === itemId); }🤖 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 `@app/_hooks/useSidebar.tsx` around lines 189 - 204, Update the pathname-based lookup in the useEffect to compare the extracted route identifier with each item’s uuid, matching the isItemSelected behavior. Keep the existing checklist/note branching and call expandCategoryPath for the matched item’s category.app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx (1)
152-152: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStale
disabledcheck: compares againstitem.id, but the toggling state now storesitem.uuid.Since line 100 sets
isTogglingPintoitem.uuid!, this comparison againstitem.idwill essentially never be true (id ≠ uuid), so the pin/unpin menu item never shows as disabled while the toggle is in flight.🐛 Fix
- disabled: isTogglingPin === item.id, + disabled: isTogglingPin === item.uuid,🤖 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 `@app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx` at line 152, Update the disabled check in SidebarItem to compare isTogglingPin with item.uuid, matching the identifier assigned when the pin toggle starts, so the menu item is disabled during the in-flight operation.app/_components/FeatureComponents/Tags/TagHoverCard.tsx (1)
22-35: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard
list.uuidbefore navigating.checklistsisPartial<Checklist>[], solist.uuidcan be missing here; return early beforerouter.push(...)to avoid/checklist/undefined.🤖 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 `@app/_components/FeatureComponents/Tags/TagHoverCard.tsx` around lines 22 - 35, Guard list.uuid in handleChecklistClick before calling router.push; return early when it is missing, while preserving the existing event prevention and navigation behavior for valid checklist UUIDs.app/_utils/markdown-utils.tsx (1)
656-673: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPopulate
uuidfor path-based internal links
/note/and/checklist/links now store the last path segment initemIdand leaveuuidempty.InternalLinkComponentstill usesuuidfor metadata loading and preview selection, so these links fall back to generic text/Uncategorized, and the conversion fallback only works ifitemIdmatches the legacyid.🤖 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 `@app/_utils/markdown-utils.tsx` around lines 656 - 673, Update the path-based link handling in the href parsing logic to populate uuid from the final decoded segment for both /note/ and /checklist/ links, while retaining itemId as needed for conversion compatibility. Ensure InternalLinkComponent receives the identifier used for metadata loading and preview selection, without changing category parsing.app/_components/FeatureComponents/Home/Parts/NotesHome.tsx (1)
121-124: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
uuidfor pinned deduping
filteredRecentstill excludes pinned notes by deprecatedid. Match onuuidhere so the recent list doesn’t drift when slugs and UUIDs differ.🤖 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 `@app/_components/FeatureComponents/Home/Parts/NotesHome.tsx` around lines 121 - 124, Update the pinned-note deduplication in filteredRecent to compare note.uuid with the pinned entry’s uuid instead of the deprecated id fields, while preserving the existing filtering behavior.app/_components/FeatureComponents/Notes/Parts/TipTap/CustomExtensions/InternalLinkComponent.tsx (1)
132-158: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
/jotty/checklist links can’t fall back to the checklist lookup.
getNoteById()returnsundefinedon a miss, so thereturnafterawait _returnNote(...)exits before_returnChecklist(...)can run. A/jotty/{uuid}link for a checklist without loaded metadata will silently do nothing.🤖 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 `@app/_components/FeatureComponents/Notes/Parts/TipTap/CustomExtensions/InternalLinkComponent.tsx` around lines 132 - 158, Update the /jotty/ link handling around _returnNote and _returnChecklist so it only returns after a lookup successfully navigates. Since _returnNote can resolve without throwing when no note exists, make its success explicit before returning, then fall through to _returnChecklist for checklist UUIDs; preserve the existing fullItem routing path and warning behavior.app/_server/actions/note/crud.ts (2)
462-471: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUnguarded re-fetch in
deleteNote's catch handler.
getNoteById(uuid!)is called again inside thecatchblock with no surrounding try/catch. If it throws the same class of error that triggered this catch path in the first place, the exception propagates unhandled instead of returning{ error: "Failed to delete note" }.🛠️ Proposed fix
} catch (error) { const { uuid } = getFormData(formData, ["uuid"]); - const note = await getNoteById(uuid!); + const note = await getNoteById(uuid!).catch(() => null); await logContentEvent( "note_deleted", "note", uuid!, note?.title || "unknown", false ); return { error: "Failed to delete note" }; }🤖 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 `@app/_server/actions/note/crud.ts` around lines 462 - 471, Guard the getNoteById call in deleteNote’s catch handler so a re-fetch failure cannot escape the handler. Preserve the existing failure response, and ensure logContentEvent still receives the available note title or "unknown" when the lookup succeeds.
476-538: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winAdd a READ permission check before cloning notes and checklists.
cloneNoteandcloneChecklistresolve the source item from client-supplieduuid/userand write a copy without validatingPermissionTypes.READ. Any signed-in user who knows the UUID can duplicate a private item into their own space.🤖 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 `@app/_server/actions/note/crud.ts` around lines 476 - 538, Add a PermissionTypes.READ authorization check in cloneNote, and the corresponding cloneChecklist flow, immediately after resolving the source item and before creating or writing the clone. Validate access for the resolved source owner rather than trusting the client-supplied user value, and return the existing unauthorized/error result when read permission is missing; preserve cloning behavior for authorized sources.app/_server/actions/history/index.ts (1)
354-422: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftResolve note versions by path first, not only by UUID scan
getHistorycan still surface commits from before UUID frontmatter existed, butgetVersiononly matchesmetadata.uuid === noteUuidacross the tree. For legacy commits with no UUID field, that returns “Note version not found in commit” even though the entry is listed. Check the note’s current path first withgit show <commitHash>:<path>, then fall back to the UUID scan for renamed files.🤖 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 `@app/_server/actions/history/index.ts` around lines 354 - 422, Update getVersion to resolve the note’s current repository path at commitHash first, using git.show for that path and accepting it when the content is available, so legacy files without UUID metadata are supported. If the path lookup fails or does not identify the note, retain the existing mdFiles UUID scan to handle renamed files, and continue returning “Note version not found in commit” only after both approaches fail.app/api/checklists/[listId]/items/route.ts (1)
64-69: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSub-item ID uses deprecated
list.idinstead oflist.uuid.The inline sub-item creation path generates
id: \${list.id}-sub-${Date.now()}`using the deprecated file-sluglist.id, while both the server action (sub-items.tsline 76) and the client hook (useChecklist.tsxline 722) were updated in this PR to uselist.uuid`. This creates inconsistent sub-item ID prefixes depending on which code path created them, violating the PR's UUID-based identity objective.🐛 Proposed fix
const newSubItem: any = { - id: `${list.id}-sub-${Date.now()}`, + id: `${list.uuid}-sub-${Date.now()}`, text, completed: false, order: 0, };🤖 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 `@app/api/checklists/`[listId]/items/route.ts around lines 64 - 69, Update the inline sub-item creation in the handler creating newSubItem to build the ID prefix from list.uuid instead of the deprecated list.id, matching the UUID-based identity used by the server action and client hook.app/_server/actions/kanban/tempo.ts (1)
73-85: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winMissing permission check in
getTempoData.getListById(uuid)resolves the checklist owner from the UUID when no username is passed, so any authenticated user with a checklist UUID can read tempo data for that checklist. Add the samecheckUserPermission(..., PermissionTypes.VIEW)guard used elsewhere before returning entries.🤖 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 `@app/_server/actions/kanban/tempo.ts` around lines 73 - 85, In getTempoData, add a checkUserPermission guard using the authenticated user, the resolved list, and PermissionTypes.VIEW before returning tempo entries. Return the established unauthorized/error response when permission is denied, while preserving the existing authentication and list-not-found checks.
🧹 Nitpick comments (9)
app/_components/FeatureComponents/Notes/NotesPageClient.tsx (1)
51-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame duplicated pin-matching predicate as
ChecklistsPageClient.tsx.This mirrors the identical inline matcher repeated in
ChecklistsPageClient.tsx,useChecklistHome.tsx,useNotesHome.tsx, and_pinMatchesindashboard/index.ts. Extracting a single shared helper (e.g.isPinnedEntryinglobal-utils.ts) would remove this duplication across both notes and checklist surfaces at once.Also applies to: 171-175, 192-196, 212-216
🤖 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 `@app/_components/FeatureComponents/Notes/NotesPageClient.tsx` around lines 51 - 57, Extract the duplicated pinned-entry matching predicate into a shared helper such as isPinnedEntry in global-utils.ts, preserving matching by exact UUID or the final path segment. Replace the inline predicates in NotesPageClient filtering and the corresponding checklist, notes-home, and dashboard _pinMatches usages with this helper.app/_hooks/useChecklistHome.tsx (1)
94-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated pin-matching predicate (also within this same file).
getPinnedListsandisListPinnedreimplement the identicalentry === list.uuid || entry.split("/").pop() === list.uuidcheck, and the same logic is duplicated again acrossChecklistsPageClient.tsx,useNotesHome.tsx, and_pinMatchesindashboard/index.ts. A single shared helper would remove 6+ copies of this predicate.Also applies to: 168-171
🤖 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 `@app/_hooks/useChecklistHome.tsx` around lines 94 - 99, Extract the duplicated pin-matching predicate into a shared helper and update getPinnedLists and isListPinned to use it, preserving both direct UUID and trailing path-segment matches. Reuse the same helper in ChecklistsPageClient.tsx, useNotesHome.tsx, and dashboard/index.ts’s _pinMatches instead of maintaining local copies.app/_hooks/useNotesHome.tsx (1)
92-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated pin-matching predicate (5th+ occurrence).
Same predicate as flagged in
ChecklistsPageClient.tsx,NotesPageClient.tsx,useChecklistHome.tsx, and_pinMatchesindashboard/index.ts. Consolidating into one shared utility is the single highest-leverage cleanup across this migration.Also applies to: 128-131
🤖 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 `@app/_hooks/useNotesHome.tsx` around lines 92 - 98, The pin-matching predicate in the notes mapping logic is duplicated across multiple modules. Extract the shared entry-to-note matching behavior into one reusable utility, then update the mapping in useNotesHome and the corresponding usages in ChecklistsPageClient, NotesPageClient, useChecklistHome, and dashboard/index to call it instead of duplicating the predicate.app/_components/FeatureComponents/Checklists/ChecklistsPageClient.tsx (1)
58-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract duplicated pin-matching predicate.
The same
entry === list.uuid || entry.split("/").pop() === list.uuidcheck is repeated 4 times in this file (and duplicated again acrossNotesPageClient.tsx,useChecklistHome.tsx,useNotesHome.tsx, and_pinMatchesindashboard/index.ts). Centralizing it inapp/_utils/global-utils.ts(already imported here foritemHref) avoids drift if the matching rule ever needs to change.♻️ Proposed extraction
// app/_utils/global-utils.ts +export const isPinnedEntry = ( + entries: string[] | undefined, + uuid: string | undefined, +): boolean => + !!uuid && !!entries?.some((entry) => entry === uuid || entry.split("/").pop() === uuid);- const pinnedEntries = user?.pinnedLists || []; - filtered = filtered.filter((list) => - pinnedEntries.some( - (entry) => - entry === list.uuid || entry.split("/").pop() === list.uuid, - ), - ); + filtered = filtered.filter((list) => isPinnedEntry(user?.pinnedLists, list.uuid));Also applies to: 274-278, 294-298, 314-318
🤖 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 `@app/_components/FeatureComponents/Checklists/ChecklistsPageClient.tsx` around lines 58 - 64, Extract the repeated pin-matching predicate into a shared helper in app/_utils/global-utils.ts, then reuse that helper in ChecklistsPageClient filtering and all other local occurrences identified in this file. Update the corresponding implementations in NotesPageClient.tsx, useChecklistHome.tsx, useNotesHome.tsx, and dashboard/index.ts to use the centralized matcher while preserving the existing UUID and slash-delimited entry behavior.app/_components/FeatureComponents/Home/Parts/NotesHome.tsx (1)
134-139: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing
uuidtruthiness guard, inconsistent withTagsHome.tsx.
getNoteSharermatches purely onitem.uuid === note.uuid. The sibling implementation inTagsHome.tsxguards withitem.uuid && item.uuid === note.uuidto avoid a false match when both sides are falsy/undefined (e.g. un-migratedUserSharedItementries whereuuidis still optional). Align this implementation for consistency and defense-in-depth.🔧 Proposed fix
const getNoteSharer = (note: Note) => { const sharedItem = userSharedItems?.notes?.find( - (item) => item.uuid === note.uuid, + (item) => item.uuid && item.uuid === note.uuid, ); return sharedItem?.sharer; };🤖 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 `@app/_components/FeatureComponents/Home/Parts/NotesHome.tsx` around lines 134 - 139, Update getNoteSharer to require item.uuid to be truthy before comparing it with note.uuid, matching the guard used by the sibling TagsHome implementation and preventing matches for missing UUIDs.app/_components/FeatureComponents/Notes/Parts/NoteEditor/NoteEditorHeader.tsx (1)
207-233: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
handleEncryptionSuccessduplicateshandlePermanentDecryptionverbatim.Both handlers now build the exact same
FormData(title/content/category/uuid), callupdateNote, and reload on success — the bodies are byte-identical. Extract a shared helper to avoid drift.♻️ Proposed consolidation
+ const saveNoteContent = async (newContent: string) => { + const formData = new FormData(); + formData.append("title", title); + formData.append("content", newContent); + formData.append("category", category); + formData.append("uuid", note.uuid!); + + const result = await updateNote(formData); + + if (result.success && result.data) { + window.location.reload(); + } + }; + - const handlePermanentDecryption = async (newContent: string) => { - const formData = new FormData(); - formData.append("title", title); - formData.append("content", newContent); - formData.append("category", category); - formData.append("uuid", note.uuid!); - - const result = await updateNote(formData); - - if (result.success && result.data) { - window.location.reload(); - } - }; - - const handleEncryptionSuccess = async (newContent: string) => { - const formData = new FormData(); - formData.append("title", title); - formData.append("content", newContent); - formData.append("category", category); - formData.append("uuid", note.uuid!); - - const result = await updateNote(formData); - - if (result.success && result.data) { - window.location.reload(); - } - }; + const handlePermanentDecryption = saveNoteContent; + const handleEncryptionSuccess = saveNoteContent;🤖 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 `@app/_components/FeatureComponents/Notes/Parts/NoteEditor/NoteEditorHeader.tsx` around lines 207 - 233, Extract the duplicated update-and-reload logic from handlePermanentDecryption and handleEncryptionSuccess into a shared helper that accepts the new content and builds the same FormData fields, calls updateNote, and reloads on successful completion. Update both handlers to delegate to this helper without changing their existing behavior.app/api/checklists/[listId]/items/route.ts (1)
110-114: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
UNCATEGORIZEDconstant instead of string literal.The server actions (
status.ts,sub-items.ts) import and use theUNCATEGORIZEDconstant from@/app/_consts/notes, but this API route uses the raw string"Uncategorized". If the constant value ever changes, this path will silently diverge.♻️ Proposed fix
import { CHECKLISTS_FOLDER } from "`@/app/_consts/checklists`"; +import { UNCATEGORIZED } from "`@/app/_consts/notes`"; @@ -112,7 +113,7 @@ const filePath = path.join( ownerDir, - list.category || "Uncategorized", + list.category || UNCATEGORIZED, `${list.id}.md`, );🤖 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 `@app/api/checklists/`[listId]/items/route.ts around lines 110 - 114, Update the filePath construction to use the existing UNCATEGORIZED constant as the fallback category instead of the literal "Uncategorized"; preserve the current path.join behavior and import the constant from the established notes constants module.app/_hooks/useChecklist.tsx (1)
192-197: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
confirmDeleteListignoresdeleteListfailure.
deleteListis awaited but its return value is never checked. If deletion fails (e.g., permission denied, list not found),onDeleteis still called and the modal closes, leaving the UI inconsistent with server state.🛡️ Proposed fix
const confirmDeleteList = async () => { const formData = new FormData(); formData.append("uuid", localList.uuid || ""); - await deleteList(formData); - onDelete?.(localList.uuid || ""); - setShowDeleteModal(false); + const result = await deleteList(formData); + if (result.error) { + console.error("Failed to delete list:", result.error); + return; + } + onDelete?.(localList.uuid || ""); + setShowDeleteModal(false); };🤖 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 `@app/_hooks/useChecklist.tsx` around lines 192 - 197, Update confirmDeleteList to inspect the result of deleteList before continuing; only call onDelete and setShowDeleteModal(false) when deletion succeeds, while preserving the current failure state or handling failure without updating the UI as though deletion completed.app/_server/actions/checklist-item/sub-items.ts (1)
28-30: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd input validation for consistency with
status.ts.
uuid,parentId, andtextare read from FormData without validation. WhilegetListByIdreturning undefined catches missinguuid, the error message ("List not found") is less specific thanstatus.tswhich explicitly checksif (!uuid || !itemId). MissingparentIdortextproduce similarly generic errors downstream.🛡️ Proposed fix
const uuid = formData.get("uuid") as string; const parentId = formData.get("parentId") as string; const text = formData.get("text") as string; + if (!uuid || !parentId || !text) { + return { success: false, error: "uuid, parentId, and text are required" }; + } + const currentUser = await getUsername();🤖 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 `@app/_server/actions/checklist-item/sub-items.ts` around lines 28 - 30, Update the sub-item action’s FormData parsing around uuid, parentId, and text to explicitly validate all required values before downstream lookups or mutations, matching the validation pattern used by status.ts. Return the same specific invalid-input response or error convention used there, and only continue when all three values are present.
🤖 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
`@app/_components/FeatureComponents/Checklists/Parts/Common/ChecklistHeader.tsx`:
- Line 92: Update the tooltip title in the ChecklistHeader component to fall
back from checklist.uuid to checklist.id, preventing an undefined value when the
UUID is missing while preserving the existing Copy ID label.
In `@app/_components/FeatureComponents/Checklists/TasksPageClient.tsx`:
- Around line 56-62: Extract the repeated pinned-entry comparison into a shared
helper in global-utils.ts alongside itemHref, matching both the full UUID and
the final path segment. Replace all four checks in TasksPageClient.tsx and the
corresponding checks in KanbanPageClient.tsx with this helper, and reuse the
canonical pinned-matching behavior exposed by useChecklistHome.
In `@app/_components/FeatureComponents/Kanban/KanbanPageClient.tsx`:
- Around line 57-63: Extract the duplicated pinned-entry matching logic from
KanbanPageClient and TasksPageClient into one shared helper in the existing
global utilities area, then use that helper in both filtering paths. Preserve
matching for direct UUID entries, slash-delimited entries, and legacy pre-UUID
pinnedLists values, and remove the inline comparison logic from each page
client.
In
`@app/_components/FeatureComponents/Notes/Parts/NoteEditor/NoteEditorHeader.tsx`:
- Line 212: Replace the non-null uuid assertions in the form-data construction
near the note update flow with the existing empty-string fallback pattern used
by handleCopyId, the tooltip, and NoteHistoryModal. Apply this consistently to
both affected formData.append calls so unset note.uuid values append "" rather
than "undefined", while preserving the existing updateNote lookup flow.
In `@app/_components/FeatureComponents/Notes/Parts/UnifiedMarkdownRenderer.tsx`:
- Line 230: Guard the decodeURIComponent call assigning linkItemId in
UnifiedMarkdownRenderer so malformed URL segments cannot interrupt markdown
rendering. Safely catch decoding failures and fall back to the raw final path
segment, while preserving decoded values for valid links.
In `@app/_components/FeatureComponents/Sidebar/Parts/CategoryList.tsx`:
- Around line 136-138: Update the drag handler around moveNode to capture its
result, surface any returned error through the existing user feedback mechanism,
and skip router.refresh() when the move fails. Preserve refreshing only after a
successful move.
In `@app/_hooks/useNoteEditor.tsx`:
- Around line 369-376: Update confirmDelete so onDelete, router.refresh, onBack,
and setShowDeleteModal(false) run only when deleteNote completes successfully.
Inspect the result returned by deleteNote, preserve the existing post-delete
navigation flow for success, and leave the UI/navigation unchanged on failure.
In `@app/_server/actions/checklist-item/crud.ts`:
- Around line 48-49: Update updateItem and createItem to rehydrate the checklist
by uuid before any permission check or write. Use the fetched canonical
checklist exclusively for checkUserPermission and persistence/path construction,
ignoring client-supplied owner, category, and id values.
In `@app/_server/actions/checklist-item/reorder.ts`:
- Around line 16-28: Update reorderItems to explicitly validate that the
formData uuid is present and valid before calling getListById, matching the
validation behavior in archive.ts and unarchive.ts; return the same clear
input-validation error used by those sibling actions, while preserving the
existing list lookup for valid UUIDs.
In `@app/_server/actions/checklist/readers.ts`:
- Around line 32-45: Serialize the read–generate–write sequence in _stampUuid
per filePath using a mutex or equivalent write guard, so concurrent
metadata-only callers cannot stamp the same legacy checklist with different
UUIDs. Ensure each caller re-checks the file state under the guard before
generating and writing, while preserving the existing undefined return and
warning behavior on failure.
In `@app/_server/actions/note/readers.ts`:
- Around line 29-45: Remove UUID assignment from the read path, including the
_stampUuid calls used by metadataOnly/excerptLength reads. Move UUID backfilling
to a one-time migration, or serialize assignment through a shared locking
mechanism so concurrent reads cannot generate conflicting UUIDs; ensure every
returned Note still has a defined uuid even when persistence fails.
In `@app/_server/actions/sharing/permissions.ts`:
- Around line 76-97: The local ownership check in the permission flow must use
an anchored frontmatter UUID lookup instead of grepCheckUuidExists. Update the
logic around getUserByChecklistUuid/getUserByNoteUuid to reuse
grepFindFileByUuid or grepFindFileByField, preserving the existing boolean
return behavior while preventing UUIDs in arbitrary Markdown content from
granting access.
In `@app/_server/lib/legacy-lookup.ts`:
- Around line 19-79: Validate category and id before constructing paths in
_candidates, rejecting path separators, traversal segments, and other values
that are not single safe path components; enforce this for every caller,
including _findFile and legacyResolve. Add a containment check for candidate
paths under the selected user directory before _findFile accesses them, so
_uuidFor can only read or stamp files within that directory. Preserve valid
categorized and uncategorized legacy lookups while returning no match for
invalid inputs.
In `@app/`(loggedInRoutes)/checklist/[...categoryPath]/page.tsx:
- Around line 1-4: Update the legacy resolution call in this checklist page to
pass the signed-in username to legacyResolve, ensuring duplicate legacy slugs
resolve only within the current user’s account. Reuse the page’s existing
authenticated-user value and preserve the current redirect behavior.
In `@app/`(loggedInRoutes)/checklist/[uuid]/page.tsx:
- Around line 47-61: Update the checklist page flow around getCurrentUser so it
redirects immediately when no user record is returned, before deriving username
or calling getListById. Preserve the existing authenticated lookup and
hasContentAccess fallback behavior for valid users.
In `@app/`(loggedInRoutes)/note/[uuid]/page.tsx:
- Around line 33-50: Update NotePage’s legacy resolution flow to call
getCurrentUser() before legacyResolve, then pass the current user’s username to
scope the lookup to that user first. Preserve the existing redirect behavior,
and apply the same scoped-first/admin-fallback pattern used by the nearby
getNoteById calls so legacyResolve cannot select another user’s matching note.
In `@app/public/note/`[uuid]/page.tsx:
- Line 74: Review the access condition around isPubliclyShared, isOwner, and
isPrintView, then confirm the intended owner behavior before changing it. Remove
the redundant standalone isOwner term if owners should only access this public
view in print mode; otherwise simplify the condition to isPubliclyShared ||
isOwner and remove the ineffective isPrintView check.
---
Outside diff comments:
In `@app/_components/FeatureComponents/Home/Parts/NotesHome.tsx`:
- Around line 121-124: Update the pinned-note deduplication in filteredRecent to
compare note.uuid with the pinned entry’s uuid instead of the deprecated id
fields, while preserving the existing filtering behavior.
In
`@app/_components/FeatureComponents/Notes/Parts/TipTap/CustomExtensions/InternalLinkComponent.tsx`:
- Around line 132-158: Update the /jotty/ link handling around _returnNote and
_returnChecklist so it only returns after a lookup successfully navigates. Since
_returnNote can resolve without throwing when no note exists, make its success
explicit before returning, then fall through to _returnChecklist for checklist
UUIDs; preserve the existing fullItem routing path and warning behavior.
In `@app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx`:
- Line 152: Update the disabled check in SidebarItem to compare isTogglingPin
with item.uuid, matching the identifier assigned when the pin toggle starts, so
the menu item is disabled during the in-flight operation.
In `@app/_components/FeatureComponents/Tags/TagHoverCard.tsx`:
- Around line 22-35: Guard list.uuid in handleChecklistClick before calling
router.push; return early when it is missing, while preserving the existing
event prevention and navigation behavior for valid checklist UUIDs.
In `@app/_hooks/useSidebar.tsx`:
- Around line 189-204: Update the pathname-based lookup in the useEffect to
compare the extracted route identifier with each item’s uuid, matching the
isItemSelected behavior. Keep the existing checklist/note branching and call
expandCategoryPath for the matched item’s category.
In `@app/_server/actions/history/index.ts`:
- Around line 354-422: Update getVersion to resolve the note’s current
repository path at commitHash first, using git.show for that path and accepting
it when the content is available, so legacy files without UUID metadata are
supported. If the path lookup fails or does not identify the note, retain the
existing mdFiles UUID scan to handle renamed files, and continue returning “Note
version not found in commit” only after both approaches fail.
In `@app/_server/actions/kanban/tempo.ts`:
- Around line 73-85: In getTempoData, add a checkUserPermission guard using the
authenticated user, the resolved list, and PermissionTypes.VIEW before returning
tempo entries. Return the established unauthorized/error response when
permission is denied, while preserving the existing authentication and
list-not-found checks.
In `@app/_server/actions/note/crud.ts`:
- Around line 462-471: Guard the getNoteById call in deleteNote’s catch handler
so a re-fetch failure cannot escape the handler. Preserve the existing failure
response, and ensure logContentEvent still receives the available note title or
"unknown" when the lookup succeeds.
- Around line 476-538: Add a PermissionTypes.READ authorization check in
cloneNote, and the corresponding cloneChecklist flow, immediately after
resolving the source item and before creating or writing the clone. Validate
access for the resolved source owner rather than trusting the client-supplied
user value, and return the existing unauthorized/error result when read
permission is missing; preserve cloning behavior for authorized sources.
In `@app/_server/actions/users/helpers.ts`:
- Around line 23-65: Replace the per-user directory scan in getUserByItemUuid
with a direct UUID-to-owner lookup, using or adding an index that covers the
requested itemType and returns the owning User without iterating through users.
Preserve the existing Result<User> success/not-found behavior and error
handling, and ensure getUserByNoteUuid and getUserByChecklistUuid continue using
this optimized path.
In `@app/_utils/markdown-utils.tsx`:
- Around line 656-673: Update the path-based link handling in the href parsing
logic to populate uuid from the final decoded segment for both /note/ and
/checklist/ links, while retaining itemId as needed for conversion
compatibility. Ensure InternalLinkComponent receives the identifier used for
metadata loading and preview selection, without changing category parsing.
In `@app/api/checklists/`[listId]/items/route.ts:
- Around line 64-69: Update the inline sub-item creation in the handler creating
newSubItem to build the ID prefix from list.uuid instead of the deprecated
list.id, matching the UUID-based identity used by the server action and client
hook.
---
Nitpick comments:
In `@app/_components/FeatureComponents/Checklists/ChecklistsPageClient.tsx`:
- Around line 58-64: Extract the repeated pin-matching predicate into a shared
helper in app/_utils/global-utils.ts, then reuse that helper in
ChecklistsPageClient filtering and all other local occurrences identified in
this file. Update the corresponding implementations in NotesPageClient.tsx,
useChecklistHome.tsx, useNotesHome.tsx, and dashboard/index.ts to use the
centralized matcher while preserving the existing UUID and slash-delimited entry
behavior.
In `@app/_components/FeatureComponents/Home/Parts/NotesHome.tsx`:
- Around line 134-139: Update getNoteSharer to require item.uuid to be truthy
before comparing it with note.uuid, matching the guard used by the sibling
TagsHome implementation and preventing matches for missing UUIDs.
In `@app/_components/FeatureComponents/Notes/NotesPageClient.tsx`:
- Around line 51-57: Extract the duplicated pinned-entry matching predicate into
a shared helper such as isPinnedEntry in global-utils.ts, preserving matching by
exact UUID or the final path segment. Replace the inline predicates in
NotesPageClient filtering and the corresponding checklist, notes-home, and
dashboard _pinMatches usages with this helper.
In
`@app/_components/FeatureComponents/Notes/Parts/NoteEditor/NoteEditorHeader.tsx`:
- Around line 207-233: Extract the duplicated update-and-reload logic from
handlePermanentDecryption and handleEncryptionSuccess into a shared helper that
accepts the new content and builds the same FormData fields, calls updateNote,
and reloads on successful completion. Update both handlers to delegate to this
helper without changing their existing behavior.
In `@app/_hooks/useChecklist.tsx`:
- Around line 192-197: Update confirmDeleteList to inspect the result of
deleteList before continuing; only call onDelete and setShowDeleteModal(false)
when deletion succeeds, while preserving the current failure state or handling
failure without updating the UI as though deletion completed.
In `@app/_hooks/useChecklistHome.tsx`:
- Around line 94-99: Extract the duplicated pin-matching predicate into a shared
helper and update getPinnedLists and isListPinned to use it, preserving both
direct UUID and trailing path-segment matches. Reuse the same helper in
ChecklistsPageClient.tsx, useNotesHome.tsx, and dashboard/index.ts’s _pinMatches
instead of maintaining local copies.
In `@app/_hooks/useNotesHome.tsx`:
- Around line 92-98: The pin-matching predicate in the notes mapping logic is
duplicated across multiple modules. Extract the shared entry-to-note matching
behavior into one reusable utility, then update the mapping in useNotesHome and
the corresponding usages in ChecklistsPageClient, NotesPageClient,
useChecklistHome, and dashboard/index to call it instead of duplicating the
predicate.
In `@app/_server/actions/checklist-item/sub-items.ts`:
- Around line 28-30: Update the sub-item action’s FormData parsing around uuid,
parentId, and text to explicitly validate all required values before downstream
lookups or mutations, matching the validation pattern used by status.ts. Return
the same specific invalid-input response or error convention used there, and
only continue when all three values are present.
In `@app/api/checklists/`[listId]/items/route.ts:
- Around line 110-114: Update the filePath construction to use the existing
UNCATEGORIZED constant as the fallback category instead of the literal
"Uncategorized"; preserve the current path.join behavior and import the constant
from the established notes constants module.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 20a5feef-992b-4239-8d3e-53706fece4fa
📒 Files selected for processing (135)
app/(loggedInRoutes)/admin/checklist/[uuid]/page.tsxapp/(loggedInRoutes)/admin/note/[uuid]/page.tsxapp/(loggedInRoutes)/checklist/[...categoryPath]/page.tsxapp/(loggedInRoutes)/checklist/[uuid]/page.tsxapp/(loggedInRoutes)/note/[...categoryPath]/page.tsxapp/(loggedInRoutes)/note/[uuid]/page.tsxapp/_components/FeatureComponents/Admin/Parts/AdminContent.tsxapp/_components/FeatureComponents/Checklists/ChecklistsPageClient.tsxapp/_components/FeatureComponents/Checklists/Parts/ChecklistClient.tsxapp/_components/FeatureComponents/Checklists/Parts/Common/ChecklistHeader.tsxapp/_components/FeatureComponents/Checklists/Parts/Common/LastModifiedCreatedInfo.tsxapp/_components/FeatureComponents/Checklists/Parts/Simple/ChecklistBody.tsxapp/_components/FeatureComponents/Checklists/TasksPageClient.tsxapp/_components/FeatureComponents/Home/HomeClient.tsxapp/_components/FeatureComponents/Home/Parts/ChecklistHome.tsxapp/_components/FeatureComponents/Home/Parts/NotesHome.tsxapp/_components/FeatureComponents/Home/Parts/TagsHome.tsxapp/_components/FeatureComponents/Kanban/Kanban.tsxapp/_components/FeatureComponents/Kanban/KanbanCard.tsxapp/_components/FeatureComponents/Kanban/KanbanCardDetail.tsxapp/_components/FeatureComponents/Kanban/KanbanColumn.tsxapp/_components/FeatureComponents/Kanban/KanbanPageClient.tsxapp/_components/FeatureComponents/Kanban/TimeEntriesModal.tsxapp/_components/FeatureComponents/Notes/NoteClient.tsxapp/_components/FeatureComponents/Notes/NotesPageClient.tsxapp/_components/FeatureComponents/Notes/Parts/NoteEditor/NoteEditor.tsxapp/_components/FeatureComponents/Notes/Parts/NoteEditor/NoteEditorContent.tsxapp/_components/FeatureComponents/Notes/Parts/NoteEditor/NoteEditorHeader.tsxapp/_components/FeatureComponents/Notes/Parts/ReferencedBySection.tsxapp/_components/FeatureComponents/Notes/Parts/SwipeNavigationWrapper.tsxapp/_components/FeatureComponents/Notes/Parts/TipTap/CustomExtensions/InternalLinkComponent.tsxapp/_components/FeatureComponents/Notes/Parts/UnifiedMarkdownRenderer.tsxapp/_components/FeatureComponents/Profile/Parts/ConnectionsGraph/graph-data.tsapp/_components/FeatureComponents/Search/Parts/SearchResults.tsxapp/_components/FeatureComponents/Sidebar/Parts/CategoryList.tsxapp/_components/FeatureComponents/Sidebar/Parts/CategoryRenderer.tsxapp/_components/FeatureComponents/Sidebar/Parts/SharedItemsList.tsxapp/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsxapp/_components/FeatureComponents/Tags/TagHoverCard.tsxapp/_components/GlobalComponents/Cards/ChecklistCard.tsxapp/_components/GlobalComponents/Cards/ChecklistGridItem.tsxapp/_components/GlobalComponents/Cards/ChecklistListItem.tsxapp/_components/GlobalComponents/Cards/NoteCard.tsxapp/_components/GlobalComponents/Cards/NoteGridItem.tsxapp/_components/GlobalComponents/Cards/NoteListItem.tsxapp/_components/GlobalComponents/Modals/ChecklistModals/EditChecklistModal.tsxapp/_components/GlobalComponents/Modals/NotesModal/EditNoteModal.tsxapp/_components/GlobalComponents/Modals/SharingModals/ShareModal.tsxapp/_consts/identity.tsapp/_consts/notes.tsapp/_hooks/kanban/useKanban.tsapp/_hooks/kanban/useKanbanItem.tsxapp/_hooks/useAdjacentNotes.tsapp/_hooks/useChecklist.tsxapp/_hooks/useChecklistHome.tsxapp/_hooks/useNoteEditor.tsxapp/_hooks/useNotesHome.tsxapp/_hooks/useSearch.tsapp/_hooks/useSharingTools.tsapp/_hooks/useSidebar.tsxapp/_providers/PermissionsProvider.tsxapp/_providers/ShortcutsProvider.tsxapp/_server/actions/category/move.tsapp/_server/actions/checklist-item/archive.tsapp/_server/actions/checklist-item/bulk-operations.tsapp/_server/actions/checklist-item/crud.tsapp/_server/actions/checklist-item/drop.tsapp/_server/actions/checklist-item/reorder.tsapp/_server/actions/checklist-item/status.tsapp/_server/actions/checklist-item/sub-items.tsapp/_server/actions/checklist/converters.tsapp/_server/actions/checklist/crud.tsapp/_server/actions/checklist/queries.tsapp/_server/actions/checklist/readers.tsapp/_server/actions/config/helpers.tsapp/_server/actions/dashboard/index.tsapp/_server/actions/history/index.tsapp/_server/actions/kanban/calendar.tsapp/_server/actions/kanban/items.tsapp/_server/actions/kanban/search.tsapp/_server/actions/kanban/tempo.tsapp/_server/actions/kanban/time-entries.tsapp/_server/actions/note/crud.tsapp/_server/actions/note/queries.tsapp/_server/actions/note/readers.tsapp/_server/actions/notifications/index.tsapp/_server/actions/sharing/helpers.tsapp/_server/actions/sharing/permissions.tsapp/_server/actions/sharing/queries.tsapp/_server/actions/sharing/share-operations.tsapp/_server/actions/sharing/types.tsapp/_server/actions/sharing/updates.tsapp/_server/actions/users/helpers.tsapp/_server/actions/users/index.tsapp/_server/actions/users/queries.tsapp/_server/lib/legacy-lookup.tsapp/_server/reminders/scanner.tsapp/_types/checklist.tsapp/_types/note.tsapp/_types/sharing.tsapp/_utils/api-utils.tsapp/_utils/global-utils.tsapp/_utils/indexes-utils.tsapp/_utils/kanban/api-transforms.tsapp/_utils/markdown-utils.tsxapp/_utils/sharing-utils.tsapp/api/checklists/[listId]/items/[itemIndex]/check/route.tsapp/api/checklists/[listId]/items/[itemIndex]/route.tsapp/api/checklists/[listId]/items/[itemIndex]/uncheck/route.tsapp/api/checklists/[listId]/items/reorder/route.tsapp/api/checklists/[listId]/items/route.tsapp/api/checklists/[listId]/route.tsapp/api/checklists/route.tsapp/api/kanban/[boardId]/calendar/route.tsapp/api/kanban/[boardId]/items/[itemId]/assign/route.tsapp/api/kanban/[boardId]/items/[itemId]/reminder/route.tsapp/api/kanban/[boardId]/items/[itemId]/route.tsapp/api/kanban/[boardId]/items/[itemId]/status/route.tsapp/api/kanban/[boardId]/items/route.tsapp/api/kanban/[boardId]/route.tsapp/api/kanban/[boardId]/statuses/route.tsapp/api/notes/[noteId]/route.tsapp/api/notes/route.tsapp/api/tasks/[taskId]/items/[itemIndex]/route.tsapp/api/tasks/[taskId]/items/[itemIndex]/status/route.tsapp/api/tasks/[taskId]/items/route.tsapp/api/tasks/[taskId]/route.tsapp/api/tasks/[taskId]/statuses/[statusId]/route.tsapp/api/tasks/[taskId]/statuses/route.tsapp/api/tasks/route.tsapp/public/checklist/[...categoryPath]/page.tsxapp/public/checklist/[uuid]/page.tsxapp/public/note/[...categoryPath]/page.tsxapp/public/note/[uuid]/page.tsxhowto/API.md
💤 Files with no reviewable changes (4)
- app/_components/FeatureComponents/Kanban/KanbanColumn.tsx
- app/_components/FeatureComponents/Notes/Parts/NoteEditor/NoteEditor.tsx
- app/_components/FeatureComponents/Admin/Parts/AdminContent.tsx
- app/_server/actions/sharing/helpers.ts
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/public/checklist/[...categoryPath]/page.tsx (1)
19-25: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftResolve only publicly accessible legacy items.
Without a username,
legacyResolvesearches every owner. A private match produces a permanent UUID redirect while a miss redirects home, creating an unauthenticated existence and UUID oracle even if the canonical page later denies access. Add a public-access-aware resolver before redirecting.🤖 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 `@app/public/checklist/`[...categoryPath]/page.tsx around lines 19 - 25, Update the legacy resolution flow around legacyResolve in the checklist page to resolve only items publicly accessible to unauthenticated users, rather than searching across all owners. Use the existing public-access-aware resolver or pass the appropriate public access context before allowing the permanentRedirect; preserve the current redirect behavior only for a publicly resolvable UUID.app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx (1)
195-204: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
disabledcheck compares against the wrong identifier.
isTogglingPinis now set viaitem.uuid!(line 124), but the dropdown item'sdisabledcheck still compares againstitem.id. The menu item will never visually reflect the "toggling" state (repeated clicks are still safely no-op'd byhandleTogglePin's internal guard, but the UI won't show it as busy).💚 Proposed fix
- disabled: isTogglingPin === item.id, + disabled: isTogglingPin === item.uuid,🤖 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 `@app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx` around lines 195 - 204, Update the pin toggle dropdown item's disabled condition to compare isTogglingPin with item.uuid, matching the identifier assigned by the toggle flow, while preserving the existing disabled behavior for other states.
🧹 Nitpick comments (16)
app/_server/actions/note/migration.ts (1)
7-13: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider memoizing the negative
needsMigration()result. It runsfs.accessplus an order-file scan per mode on every page render that calls this guard; once migration is done the answer is stable for the process lifetime.🤖 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 `@app/_server/actions/note/migration.ts` around lines 7 - 13, Memoize the negative result in CheckForNeedsMigration so that after needsMigration() confirms no migration is required, subsequent authenticated calls return without repeating the filesystem access and order-file scan. Preserve the existing redirect behavior whenever needsMigration() reports that migration is needed.app/_server/actions/checklist/crud.ts (1)
315-327: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the failure before returning the generic error. The
updateListcatch now only logs the content event and swallows the exception (catch { }), so genuine write/rename failures become an opaque"Failed to update list"with no stack.deleteList/cloneChecklistkeep aconsole.error; worth matching.♻️ Suggested change
} catch (error) { + console.error("Error updating list:", error); try { const { title, uuid } = getFormData(formData, ["title", "uuid"]);🤖 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 `@app/_server/actions/checklist/crud.ts` around lines 315 - 327, Update the updateList catch block to log the original caught error with console.error before returning the generic failure response, matching the existing deleteList and cloneChecklist behavior. Preserve the checklist event logging and keep its nested catch from masking the primary update failure.tests/server-actions/note.test.ts (1)
132-142: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the new
bouncerdenial branch.The default mock always returns
{ allowed: true }, while the denial setup only setsmockCanReachtofalse. Add a case wherecanReachsucceeds butbouncerrejects the resolved target, matching the two-stage authorization flow indeleteNote.🤖 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 `@tests/server-actions/note.test.ts` around lines 132 - 142, In the deleteNote tests, add a denial case where mockCanReach resolves true but mockBouncer resolves with allowed false for the target returned by mockTargetDir. Keep the existing successful authorization setup and assert that deleteNote follows the bouncer-denial behavior rather than the canReach failure path.app/_components/FeatureComponents/Sidebar/Parts/CategoryRenderer.tsx (2)
75-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded
"Uncategorized"literal.See consolidated comment for this and the matching occurrence in
SidebarItem.tsx.🤖 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 `@app/_components/FeatureComponents/Sidebar/Parts/CategoryRenderer.tsx` around lines 75 - 78, Replace the hardcoded "Uncategorized" fallback in getItemsInCategory with the shared category-name constant used by the sidebar components, and apply the same constant to the matching fallback in SidebarItem.tsx.
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the shared "uncategorized" constant instead of the hardcoded
"Uncategorized"string. Both sites independently hardcode the fallback category name instead of referencing the new uncategorized constant introduced for this identity migration, risking silent drift if the value ever changes.
app/_components/FeatureComponents/Sidebar/Parts/CategoryRenderer.tsx#L75-78: replace the literal"Uncategorized"fallback ingetItemsInCategorywith the shared constant.app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx#L339-339: replace the literal"Uncategorized"fallback in theMetadataProviderpayload with the same shared constant.🤖 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 `@app/_components/FeatureComponents/Sidebar/Parts/CategoryRenderer.tsx` at line 1, Replace the hardcoded "Uncategorized" fallbacks in getItemsInCategory and the SidebarItem MetadataProvider payload with the shared uncategorized constant introduced for the identity migration, importing it where needed and preserving the existing fallback behavior.app/_components/GlobalComponents/Indicators/ShareBadges.tsx (1)
62-69: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winClickable badge span is not keyboard-operable.
onClickis attached to a plain<span>with norole,tabIndex, or key handler, so keyboard users can't trigger the "shared with" modal thatChecklistHeader/SidebarItemwire through thisonClick.♻️ Suggested fix
<span className={cn( "flex items-center gap-1 shrink-0", onClick && "cursor-pointer hover:text-primary", className, )} onClick={onClick} + role={onClick ? "button" : undefined} + tabIndex={onClick ? 0 : undefined} + onKeyDown={ + onClick + ? (e) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + onClick(); + } + } + : undefined + } >🤖 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 `@app/_components/GlobalComponents/Indicators/ShareBadges.tsx` around lines 62 - 69, Update the clickable badge element in ShareBadges to be keyboard-operable when onClick is provided: add an appropriate interactive role, make it focusable, and handle keyboard activation for Enter and Space while preserving the existing click behavior and non-clickable rendering.app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx (1)
339-339: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded
"Uncategorized"literal.See consolidated comment for this and the matching occurrence in
CategoryRenderer.tsx.🤖 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 `@app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx` at line 339, Replace the hardcoded "Uncategorized" fallback in the SidebarItem category mapping with the shared category-label constant or localization symbol used by CategoryRenderer.tsx, ensuring both occurrences use the same centralized value.app/_hooks/useSharingTools.ts (1)
70-75: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
modeOfis a server action; awaiting it client-side costs a round trip per call.
modeOflives inapp/_server/actions/share/queries.tsunder"use server", so everyawait modeOf(itemType)in this hook (Lines 71, 104, 237, 273, 295, 333, 361) is an RPC that only maps an enum. Consider a pure client-side helper (e.g. inapp/_utils/sharing-utils.ts) and keepmodeOffor server-only callers.🤖 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 `@app/_hooks/useSharingTools.ts` around lines 70 - 75, Replace the client-side modeOf calls in useSharingTools with a pure client-safe helper that maps itemType to the required sharing mode, defined in the appropriate client utility module. Update every listed call site to use the helper without awaiting it, while retaining modeOf for server-only callers.app/_server/actions/share/access.ts (2)
67-91: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a separator-aware containment check in the walk-up guard.
current.startsWith(userDir)treats.../notes/bob2as inside.../notes/bob. Today both call sites deriveuserDirfrom the same path so it can't trigger, but it's a cheap hardening against future callers passing a mismatched pair.♻️ Suggested guard
- while (current.startsWith(userDir)) { + const contained = (dir: string) => + dir === userDir || dir.startsWith(`${userDir}${path.sep}`); + + while (contained(current)) {🤖 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 `@app/_server/actions/share/access.ts` around lines 67 - 91, Update the walk-up guard in _chainGrants to use separator-aware path containment, so a sibling path such as userDir plus an unrelated suffix is not treated as inside userDir. Preserve traversal for userDir itself and descendants, using the platform path utilities rather than a raw startsWith check.
221-241: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueTwo full-file scans per candidate in the loose-mount pass.
resolveAccessalready callsgrepExtractFrontmatter(filePath), so Line 231 re-reads the same frontmatter. On large trees this doubles I/O for every candidate file. Returning the uuid (or the parsed metadata) fromresolveAccesswould halve it.🤖 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 `@app/_server/actions/share/access.ts` around lines 221 - 241, Update resolveAccess and the loose-mount loop to reuse the access-resolution result’s parsed frontmatter or UUID instead of calling grepExtractFrontmatter(filePath) again. Preserve the existing missing-UUID skip behavior and use the reused value when adding entries to loose.app/_server/actions/share/queries.ts (2)
151-153: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInline dynamic import of
./target.Every other dependency here is statically imported. If this is working around a cycle, a short comment saying so helps; otherwise hoist it to the top.
🤖 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 `@app/_server/actions/share/queries.ts` around lines 151 - 153, Update the code around the targetDir call to statically import the ./target dependency with the other imports and remove the inline dynamic import; if the dynamic import is required to avoid a dependency cycle, retain it and add a brief comment documenting that reason.
232-255: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
_factsForrescans the whole tree per exported query.
globalShares,sharedForUserandallSharedeach invoke_factsForfor both modes, and_factsFordoes a grep sweep plus two awaited file reads per candidate. Per the stack contextapp/layout.tsxnow wires all three, so a single page render can repeat the same full scan up to six times. Wrapping_factsForin React's request-scopedcache()(orunstable_cachewith the sharing tags already used inoperations.ts) would collapse them.Also applies to: 282-350
🤖 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 `@app/_server/actions/share/queries.ts` around lines 232 - 255, The _factsFor function repeats the same full-tree scan across exported share queries during one render. Wrap _factsFor with the existing request-scoped caching mechanism, keyed by its Modes argument, so globalShares, sharedForUser, and allShared reuse results for each mode while preserving the current fact-building behavior.app/_server/actions/share/category-info.ts (1)
80-98: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚖️ Poor tradeoffRead-modify-write on
.category-info.jsoncan lose concurrent updates.
catUuid,patchCatInfoandwriteCatOrdereach read then write without coordination. Two concurrent Server Actions on the same folder (e.g. a share plus a reorder) will last-write-wins, and incatUuid's case both callers can mint different uuids while only one persists — the loser then holds a uuid that resolves to nothing. A per-directory in-process mutex around read+write would close this.🤖 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 `@app/_server/actions/share/category-info.ts` around lines 80 - 98, Add a per-directory in-process mutex and use it to serialize each read-modify-write operation in catUuid, patchCatInfo, and writeCatOrder. Hold the directory lock across both the read and write, ensuring concurrent UUID generation cannot return an unpersisted value and updates cannot overwrite one another; preserve existing return behavior.app/_server/actions/category/queries.ts (1)
43-52: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSequential
catAccessper subcategory on a layout-critical path.
getCategoriesruns twice per root layout render (notes + checklists), and each mount subtree resolves access one directory at a time, each walking the parent chain. Batch these withPromise.alloversubTreeto cut the round trips.♻️ Parallelize access checks
- for (const category of subTree) { - const relative = category.path.slice(mount.displayName.length + 1); - const access = await catAccess(mode, path.join(ownerDir, relative)); - - const perms = access?.users[username]; - - if (perms) { - visible.push({ ...category, sharedFrom: mount.owner, permissions: perms }); - } - } + const resolved = await Promise.all( + subTree.map(async (category) => { + const relative = category.path.slice(mount.displayName.length + 1); + const access = await catAccess(mode, path.join(ownerDir, relative)); + return { category, perms: access?.users[username] }; + }), + ); + + for (const { category, perms } of resolved) { + if (perms) { + visible.push({ ...category, sharedFrom: mount.owner, permissions: perms }); + } + }🤖 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 `@app/_server/actions/category/queries.ts` around lines 43 - 52, Update the subcategory access-resolution loop in getCategories to invoke catAccess for all entries in subTree concurrently with Promise.all, then apply the existing permissions filtering and visible.push mapping to the resolved results. Preserve the current relative path, owner, username, and sharedFrom behavior while removing sequential awaits.app/_server/actions/category/crud.ts (1)
57-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMount success path drops the audit log and the
datapayload.The owned-directory branch logs an
INFOcategory_createdentry and returns{ success: true, data: { name, count: 0 } }; this branch returns bare{ success: true }and logs nothing on success. Worth aligning so shared-folder creates are auditable and callers get the same shape.🤖 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 `@app/_server/actions/category/crud.ts` around lines 57 - 71, Update the successful mount-directory branch around ensureDir, revalidateTag, and broadcast to record the same INFO category_created audit entry as the owned-directory branch, then return { success: true, data: { name, count: 0 } } so both create paths share the same response shape.app/_server/actions/category/move.ts (1)
75-93: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winEvery item drag now triggers a recursive disk walk per candidate owner.
_locateItemresolves the UUID by scanninguserDirFor(mode, owner)for the current user plus every mount owner, sequentially, on each drop. With a few hundred notes across several shared owners this is a noticeable regression versus the previous form-suppliedactiveId/category. Consider resolving the UUID from the already-cached metadata (getUserNotes/getUserChecklistswithmetadataOnly, which go throughgetOrCompute) and only falling back to the walk on a miss.🤖 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 `@app/_server/actions/category/move.ts` around lines 75 - 93, The _locateItem function performs a sequential disk walk for every candidate owner on each drag. Update it to first resolve the UUID from cached metadata using getUserNotes/getUserChecklists with metadataOnly through getOrCompute, preserving the corresponding category conversion via shownAs; only invoke the existing _findItemByUuid walk when cached metadata misses.
🤖 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 `@app/_components/FeatureComponents/Migration/Parts/ShareMigrationView.tsx`:
- Around line 46-56: Replace the hardcoded English descriptions and all other
literal migration copy in ShareMigrationView with next-intl translations,
reusing the sharing-related keys added to the locale files. Update the
component’s translation calls consistently across the “Migration Changes” block
and the additional referenced sections, while preserving the existing
conditional content and layout.
In
`@app/_components/GlobalComponents/Modals/ChecklistModals/EditChecklistModal.tsx`:
- Around line 57-59: Update the fetched-checklist refresh logic in
EditChecklistModal to synchronize the form’s category state from
fetchedChecklist.category alongside the existing title update. Preserve the
fallback behavior and existing unarchive flow, ensuring owner submissions use
the refreshed category rather than initialChecklist.category.
In `@app/_server/actions/category/queries.ts`:
- Around line 22-30: Update the root Category construction to derive count for
non-implicit mounts from the ownerDir’s markdown-item count, reusing the
existing category-tree/counting logic where possible instead of hardcoding 0.
Preserve the current implicit-mount count behavior and keep the resulting root
count consistent with its subcategories.
In `@app/_server/actions/checklist/crud.ts`:
- Around line 48-62: Validate the client-provided user identity before deriving
the target directory: in app/_server/actions/checklist/crud.ts lines 48-62,
ensure the parsed userParam username matches the session user unless the
requester is an admin, then continue using the validated acting identity for
targetDir and bouncer. Apply the same validation to formUser in
app/_server/actions/note/crud.ts lines 59-79, and invoke bouncer unconditionally
rather than only when target.isMount.
In `@app/_server/actions/lib/legacy-lookup.ts`:
- Around line 63-67: Serialize the UUID-stamping flow surrounding generateUuid,
fs.readFile, and fs.writeFile so concurrent legacy requests cannot generate
conflicting identifiers. Use a cross-process exclusive lock and reread the file
while holding it, reusing any UUID another process already stamped; otherwise
generate and persist one UUID, then release the lock before returning it.
In `@app/_server/actions/lib/migration-check.ts`:
- Around line 29-42: Update needsMigration to cache a clean migration check in
module state after scanning all CHECKED_MODES and finding neither
LEGACY_SHARING_FILE nor order files; return the cached false result on
subsequent calls while preserving the immediate true result when migration
artifacts are found.
In `@app/_server/actions/migration/share-migration.ts`:
- Around line 95-117: Update the legacy order conversion flow to detect when
resolved category or item entries are missing, especially when _uuidOfFile
cannot resolve a legacy item. Before fs.unlink in the conversion function,
preserve the legacy file or record the required change entry whenever the
resolved lists are shorter than the original lists, and only remove it after all
ordering data has been successfully retained.
- Around line 201-214: Update the migration loop around _applyShares to track
whether any file share writes fail; continue processing entries, but do not
unlink LEGACY_SHARING_FILE when failures occurred. Surface the failure after
processing so callers can detect the incomplete migration, while preserving the
existing success path that records changes and removes the legacy file.
- Around line 216-239: Protect migrateToInlineSharing with a server-side
isAdmin() check before invoking _userDirs, _stampTree, or _migrateShares. Return
the action’s existing failure Result when the caller is not an administrator,
ensuring no filesystem changes occur for unauthorized requests.
In `@app/_server/actions/note/crud.ts`:
- Line 375: Update the broadcast call in the note update flow to use the
normalized actingUsername value instead of currentUser for the username payload.
Preserve the existing note action and entityId fields while ensuring username is
always the normalized string used elsewhere in the function.
In `@app/_server/actions/share/operations.ts`:
- Around line 310-331: Caller-supplied category paths are not constrained to the
owner's directory. In app/_server/actions/share/operations.ts lines 310-331, add
a shared _safeDir(mode, owner, categoryPath) helper that resolves the path and
verifies containment, then have shareFolder use it and return an "Invalid
category" error when it returns null; apply the same guard before
readCatInfo/writeCatInfo in lines 401-419 for setFolderInherit and in lines
441-466 for setFolderPublic's un-publish branch. The publish branch requires no
direct change because it uses shareFolder.
In `@app/_server/actions/share/rename.ts`:
- Around line 30-64: The _renameInFiles function must invalidate the
mode-specific sharing cache after successfully rewriting grants. Track whether
any files were updated, then call the existing _modeTag(mode) revalidation
mechanism after the rename operation when changes occurred, preserving the
current touched count and file-processing behavior.
- Around line 99-111: Add authorization to the exported server action
renameGrants before it calls _renameInFiles or _renameInCats, requiring
isAdmin() as established in operations.ts and rejecting non-admin callers.
Preserve the existing rename behavior for authorized administrators, or move the
implementation into a non-"use server" internal module and expose it only
through the authorized users/crud flow.
In `@app/_server/actions/share/target.ts`:
- Around line 46-81: Contain mount-resolved paths in targetDir by resolving the
owner-side directory against the owner root and returning own when the resolved
path escapes that root. In app/_server/actions/share/target.ts lines 46-81,
apply this to the mount branch while preserving valid mount metadata. In
app/_server/actions/category/crud.ts lines 35-71, validate name and the
resulting joined path against mountParent.dir with isPathSafe before calling
ensureDir; both sites must reject traversal outside the shared mount.
In `@app/_server/actions/ws/broadcast.ts`:
- Around line 1-11: Remove the "use server" directive from the broadcast module
so broadcast is not exposed as a callable server action. Keep the existing
broadcast function and its server-side consumers unchanged, preserving the
__jottyBroadcast guard and event forwarding behavior.
In `@app/_translations/de.json`:
- Around line 1340-1347: Correct the localized sharing strings for the keys
deleteTip through confirmLeaveFolder in app/_translations/de.json lines
1340-1347 by restoring German umlauts; apply the requested Spanish accents and
opening question marks in app/_translations/es.json lines 1341-1347, French
accents and partagé inflections in app/_translations/fr.json lines 1341-1347,
and Italian accents in app/_translations/it.json lines 1339-1347; remove the
space before 를 in app/_translations/ko.json line 1356; and restore Polish
diacritics in app/_translations/pl.json lines 1339-1347.
In `@app/_translations/pt.json`:
- Around line 1363-1372: Update the new translation values around readTip,
writeTip, deleteTip, publicTip, fromUserRead, fromUserWrite, and fromUserDelete
to use consistent European Portuguese wording, replacing Brazilian forms with
the file’s established equivalents. Add the required diacritics in
confirmLeaveItem and confirmLeaveFolder, including próprio, proprietário, and
terá, while preserving the placeholders and message meaning.
In `@app/_translations/tr.json`:
- Around line 1366-1374: Update the newly added Turkish translation values
around writeTip, deleteTip, publicTip, fromUserRead, fromUserWrite,
fromUserDelete, leaveShare, confirmLeaveItem, and confirmLeaveFolder to use
correct Turkish diacritics consistently with the surrounding entries, while
preserving all placeholders and message meaning.
In `@app/_utils/category-utils.ts`:
- Around line 48-59: The category tree construction must not call catUuid
concurrently for folders missing info.uuid. Update the UUID provisioning path
used by the shown infoMap lookup to serialize legacy-folder initialization or
make catUuid re-read the persisted metadata atomically immediately before
writing, reusing an existing UUID when another request has already created one;
ensure each response exposes only the persisted UUID.
In `@app/layout.tsx`:
- Around line 199-208: Update the allSharedItems selection in the layout
data-loading flow to call allShared() for public or unauthenticated routes,
while retaining the existing user-specific shared data behavior for
authenticated non-public routes. Remove the inverted condition so
AppModeProvider receives public shared summaries instead of empty collections.
---
Outside diff comments:
In `@app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx`:
- Around line 195-204: Update the pin toggle dropdown item's disabled condition
to compare isTogglingPin with item.uuid, matching the identifier assigned by the
toggle flow, while preserving the existing disabled behavior for other states.
In `@app/public/checklist/`[...categoryPath]/page.tsx:
- Around line 19-25: Update the legacy resolution flow around legacyResolve in
the checklist page to resolve only items publicly accessible to unauthenticated
users, rather than searching across all owners. Use the existing
public-access-aware resolver or pass the appropriate public access context
before allowing the permanentRedirect; preserve the current redirect behavior
only for a publicly resolvable UUID.
---
Nitpick comments:
In `@app/_components/FeatureComponents/Sidebar/Parts/CategoryRenderer.tsx`:
- Around line 75-78: Replace the hardcoded "Uncategorized" fallback in
getItemsInCategory with the shared category-name constant used by the sidebar
components, and apply the same constant to the matching fallback in
SidebarItem.tsx.
- Line 1: Replace the hardcoded "Uncategorized" fallbacks in getItemsInCategory
and the SidebarItem MetadataProvider payload with the shared uncategorized
constant introduced for the identity migration, importing it where needed and
preserving the existing fallback behavior.
In `@app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx`:
- Line 339: Replace the hardcoded "Uncategorized" fallback in the SidebarItem
category mapping with the shared category-label constant or localization symbol
used by CategoryRenderer.tsx, ensuring both occurrences use the same centralized
value.
In `@app/_components/GlobalComponents/Indicators/ShareBadges.tsx`:
- Around line 62-69: Update the clickable badge element in ShareBadges to be
keyboard-operable when onClick is provided: add an appropriate interactive role,
make it focusable, and handle keyboard activation for Enter and Space while
preserving the existing click behavior and non-clickable rendering.
In `@app/_hooks/useSharingTools.ts`:
- Around line 70-75: Replace the client-side modeOf calls in useSharingTools
with a pure client-safe helper that maps itemType to the required sharing mode,
defined in the appropriate client utility module. Update every listed call site
to use the helper without awaiting it, while retaining modeOf for server-only
callers.
In `@app/_server/actions/category/crud.ts`:
- Around line 57-71: Update the successful mount-directory branch around
ensureDir, revalidateTag, and broadcast to record the same INFO category_created
audit entry as the owned-directory branch, then return { success: true, data: {
name, count: 0 } } so both create paths share the same response shape.
In `@app/_server/actions/category/move.ts`:
- Around line 75-93: The _locateItem function performs a sequential disk walk
for every candidate owner on each drag. Update it to first resolve the UUID from
cached metadata using getUserNotes/getUserChecklists with metadataOnly through
getOrCompute, preserving the corresponding category conversion via shownAs; only
invoke the existing _findItemByUuid walk when cached metadata misses.
In `@app/_server/actions/category/queries.ts`:
- Around line 43-52: Update the subcategory access-resolution loop in
getCategories to invoke catAccess for all entries in subTree concurrently with
Promise.all, then apply the existing permissions filtering and visible.push
mapping to the resolved results. Preserve the current relative path, owner,
username, and sharedFrom behavior while removing sequential awaits.
In `@app/_server/actions/checklist/crud.ts`:
- Around line 315-327: Update the updateList catch block to log the original
caught error with console.error before returning the generic failure response,
matching the existing deleteList and cloneChecklist behavior. Preserve the
checklist event logging and keep its nested catch from masking the primary
update failure.
In `@app/_server/actions/note/migration.ts`:
- Around line 7-13: Memoize the negative result in CheckForNeedsMigration so
that after needsMigration() confirms no migration is required, subsequent
authenticated calls return without repeating the filesystem access and
order-file scan. Preserve the existing redirect behavior whenever
needsMigration() reports that migration is needed.
In `@app/_server/actions/share/access.ts`:
- Around line 67-91: Update the walk-up guard in _chainGrants to use
separator-aware path containment, so a sibling path such as userDir plus an
unrelated suffix is not treated as inside userDir. Preserve traversal for
userDir itself and descendants, using the platform path utilities rather than a
raw startsWith check.
- Around line 221-241: Update resolveAccess and the loose-mount loop to reuse
the access-resolution result’s parsed frontmatter or UUID instead of calling
grepExtractFrontmatter(filePath) again. Preserve the existing missing-UUID skip
behavior and use the reused value when adding entries to loose.
In `@app/_server/actions/share/category-info.ts`:
- Around line 80-98: Add a per-directory in-process mutex and use it to
serialize each read-modify-write operation in catUuid, patchCatInfo, and
writeCatOrder. Hold the directory lock across both the read and write, ensuring
concurrent UUID generation cannot return an unpersisted value and updates cannot
overwrite one another; preserve existing return behavior.
In `@app/_server/actions/share/queries.ts`:
- Around line 151-153: Update the code around the targetDir call to statically
import the ./target dependency with the other imports and remove the inline
dynamic import; if the dynamic import is required to avoid a dependency cycle,
retain it and add a brief comment documenting that reason.
- Around line 232-255: The _factsFor function repeats the same full-tree scan
across exported share queries during one render. Wrap _factsFor with the
existing request-scoped caching mechanism, keyed by its Modes argument, so
globalShares, sharedForUser, and allShared reuse results for each mode while
preserving the current fact-building behavior.
In `@tests/server-actions/note.test.ts`:
- Around line 132-142: In the deleteNote tests, add a denial case where
mockCanReach resolves true but mockBouncer resolves with allowed false for the
target returned by mockTargetDir. Keep the existing successful authorization
setup and assert that deleteNote follows the bouncer-denial behavior rather than
the canReach failure path.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a0018f1b-cdf3-4094-87f6-b5c5965e2e6f
📒 Files selected for processing (131)
app/(loggedInRoutes)/checklist/[...categoryPath]/page.tsxapp/(loggedInRoutes)/checklist/[uuid]/page.tsxapp/(loggedInRoutes)/note/[...categoryPath]/page.tsxapp/(loggedInRoutes)/note/[uuid]/page.tsxapp/_components/FeatureComponents/Admin/Parts/Sharing/AdminSharing.tsxapp/_components/FeatureComponents/Admin/Parts/ThemePreview.tsxapp/_components/FeatureComponents/Checklists/Parts/Common/ChecklistHeader.tsxapp/_components/FeatureComponents/Kanban/KanbanCardDetail.tsxapp/_components/FeatureComponents/Migration/MigrationPage.tsxapp/_components/FeatureComponents/Migration/Parts/ShareMigrationView.tsxapp/_components/FeatureComponents/Migration/Parts/YamlMetadataMigrationView.tsxapp/_components/FeatureComponents/Notes/Parts/NoteEditor/NoteEditorHeader.tsxapp/_components/FeatureComponents/Sidebar/Parts/CategoryList.tsxapp/_components/FeatureComponents/Sidebar/Parts/CategoryRenderer.tsxapp/_components/FeatureComponents/Sidebar/Parts/SharedItemsList.tsxapp/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsxapp/_components/FeatureComponents/Sidebar/Sidebar.tsxapp/_components/GlobalComponents/Indicators/ShareBadges.tsxapp/_components/GlobalComponents/Indicators/SharedFromBadge.tsxapp/_components/GlobalComponents/Modals/ChecklistModals/EditChecklistModal.tsxapp/_components/GlobalComponents/Modals/NotesModal/EditNoteModal.tsxapp/_components/GlobalComponents/Modals/NotesModal/NoteHistoryModal.tsxapp/_components/GlobalComponents/Modals/SharingModals/FolderShareModal.tsxapp/_components/GlobalComponents/Modals/SharingModals/Parts/InheritedNotice.tsxapp/_components/GlobalComponents/Modals/SharingModals/ShareModal.tsxapp/_consts/files.tsapp/_consts/sharing.tsapp/_hooks/useFolderShare.tsapp/_hooks/useSharingTools.tsapp/_hooks/useSidebar.tsxapp/_schemas/sharing-schemas.tsapp/_server/actions/category/crud.tsapp/_server/actions/category/index.tsapp/_server/actions/category/move.tsapp/_server/actions/category/ordering.tsapp/_server/actions/category/queries.tsapp/_server/actions/checklist-item/archive.tsapp/_server/actions/checklist-item/bulk-operations.tsapp/_server/actions/checklist-item/crud.tsapp/_server/actions/checklist-item/drop.tsapp/_server/actions/checklist-item/reorder.tsapp/_server/actions/checklist-item/status.tsapp/_server/actions/checklist-item/sub-items.tsapp/_server/actions/checklist/converters.tsapp/_server/actions/checklist/crud.tsapp/_server/actions/checklist/queries.tsapp/_server/actions/checklist/readers.tsapp/_server/actions/file/index.tsapp/_server/actions/history/index.tsapp/_server/actions/kanban/items.tsapp/_server/actions/kanban/time-entries.tsapp/_server/actions/lib/legacy-lookup.tsapp/_server/actions/lib/metadata-cache.tsapp/_server/actions/lib/migration-check.tsapp/_server/actions/migration/folder-migration.tsapp/_server/actions/migration/helpers.tsapp/_server/actions/migration/index-migration.tsapp/_server/actions/migration/index.tsapp/_server/actions/migration/share-migration.tsapp/_server/actions/migration/sharing-migration.tsapp/_server/actions/migration/yaml-migration.tsapp/_server/actions/note/crud.tsapp/_server/actions/note/migration.tsapp/_server/actions/note/queries.tsapp/_server/actions/note/readers.tsapp/_server/actions/notifications/index.tsapp/_server/actions/reminders/scanner.tsapp/_server/actions/share/access.tsapp/_server/actions/share/category-info.tsapp/_server/actions/share/mounts.tsapp/_server/actions/share/operations.tsapp/_server/actions/share/queries.tsapp/_server/actions/share/rename.tsapp/_server/actions/share/target.tsapp/_server/actions/sharing/helpers.tsapp/_server/actions/sharing/index.tsapp/_server/actions/sharing/io.tsapp/_server/actions/sharing/permissions.tsapp/_server/actions/sharing/queries.tsapp/_server/actions/sharing/share-operations.tsapp/_server/actions/sharing/types.tsapp/_server/actions/sharing/updates.tsapp/_server/actions/users/crud.tsapp/_server/actions/ws/broadcast.tsapp/_translations/de.jsonapp/_translations/en.jsonapp/_translations/es.jsonapp/_translations/fr.jsonapp/_translations/it.jsonapp/_translations/klingon.jsonapp/_translations/ko.jsonapp/_translations/nl.jsonapp/_translations/pirate.jsonapp/_translations/pl.jsonapp/_translations/pt.jsonapp/_translations/ru.jsonapp/_translations/tr.jsonapp/_translations/zh.jsonapp/_types/audit.tsapp/_types/category.tsapp/_types/checklist.tsapp/_types/enums.tsapp/_types/note.tsapp/_types/sharing.tsapp/_utils/api-utils.tsapp/_utils/category-utils.tsapp/_utils/grep-utils.tsapp/_utils/order-utils.tsapp/_utils/sharing-utils.tsapp/_utils/sidebar-store.tsapp/api/checklists/[listId]/items/reorder/route.tsapp/api/file/[username]/[filename]/route.tsapp/api/image/[username]/[filename]/route.tsapp/api/notes/[noteId]/route.tsapp/api/video/[username]/[filename]/route.tsapp/layout.tsxapp/migration/page.tsxapp/public/checklist/[...categoryPath]/page.tsxapp/public/checklist/[uuid]/page.tsxapp/public/note/[...categoryPath]/page.tsxapp/public/note/[uuid]/page.tsxinstrumentation.tstests/api/setup.tstests/server-actions/category.test.tstests/server-actions/checklist-item.test.tstests/server-actions/dashboard.test.tstests/server-actions/drop-item.test.tstests/server-actions/file.test.tstests/server-actions/history.test.tstests/server-actions/note.test.tstests/server-actions/sharing.test.ts
💤 Files with no reviewable changes (21)
- app/_server/actions/sharing/io.ts
- app/_server/actions/migration/index-migration.ts
- app/_server/actions/sharing/helpers.ts
- app/_components/FeatureComponents/Migration/Parts/YamlMetadataMigrationView.tsx
- app/_components/FeatureComponents/Sidebar/Parts/SharedItemsList.tsx
- app/_server/actions/migration/yaml-migration.ts
- app/_server/actions/category/ordering.ts
- app/_server/actions/migration/helpers.ts
- app/_server/actions/migration/sharing-migration.ts
- app/_server/actions/sharing/updates.ts
- tests/server-actions/category.test.ts
- app/_server/actions/sharing/types.ts
- app/_server/actions/sharing/index.ts
- app/_server/actions/migration/folder-migration.ts
- app/_server/actions/category/index.ts
- app/_consts/files.ts
- app/_server/actions/sharing/permissions.ts
- app/_hooks/useSidebar.tsx
- app/_server/actions/sharing/share-operations.ts
- app/_components/FeatureComponents/Admin/Parts/Sharing/AdminSharing.tsx
- app/_server/actions/sharing/queries.ts
🚧 Files skipped from review as they are similar to previous changes (23)
- app/_server/actions/notifications/index.ts
- app/(loggedInRoutes)/note/[uuid]/page.tsx
- app/_components/GlobalComponents/Modals/SharingModals/ShareModal.tsx
- app/_server/actions/checklist-item/sub-items.ts
- app/_types/checklist.ts
- app/(loggedInRoutes)/checklist/[...categoryPath]/page.tsx
- app/(loggedInRoutes)/checklist/[uuid]/page.tsx
- app/(loggedInRoutes)/note/[...categoryPath]/page.tsx
- app/_server/actions/kanban/time-entries.ts
- app/public/note/[...categoryPath]/page.tsx
- app/_components/FeatureComponents/Notes/Parts/NoteEditor/NoteEditorHeader.tsx
- app/api/checklists/[listId]/items/reorder/route.ts
- app/public/checklist/[uuid]/page.tsx
- app/_server/actions/checklist-item/status.ts
- app/_server/actions/checklist/converters.ts
- app/public/note/[uuid]/page.tsx
- app/_server/actions/history/index.ts
- app/_server/actions/kanban/items.ts
- app/_components/GlobalComponents/Modals/NotesModal/EditNoteModal.tsx
- app/_server/actions/checklist-item/bulk-operations.ts
- app/api/notes/[noteId]/route.ts
- app/_server/actions/checklist/queries.ts
- app/_components/FeatureComponents/Kanban/KanbanCardDetail.tsx
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/public/checklist/[...categoryPath]/page.tsx (1)
19-25: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftResolve only publicly accessible legacy items.
Without a username,
legacyResolvesearches every owner. A private match produces a permanent UUID redirect while a miss redirects home, creating an unauthenticated existence and UUID oracle even if the canonical page later denies access. Add a public-access-aware resolver before redirecting.🤖 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 `@app/public/checklist/`[...categoryPath]/page.tsx around lines 19 - 25, Update the legacy resolution flow around legacyResolve in the checklist page to resolve only items publicly accessible to unauthenticated users, rather than searching across all owners. Use the existing public-access-aware resolver or pass the appropriate public access context before allowing the permanentRedirect; preserve the current redirect behavior only for a publicly resolvable UUID.app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx (1)
195-204: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
disabledcheck compares against the wrong identifier.
isTogglingPinis now set viaitem.uuid!(line 124), but the dropdown item'sdisabledcheck still compares againstitem.id. The menu item will never visually reflect the "toggling" state (repeated clicks are still safely no-op'd byhandleTogglePin's internal guard, but the UI won't show it as busy).💚 Proposed fix
- disabled: isTogglingPin === item.id, + disabled: isTogglingPin === item.uuid,🤖 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 `@app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx` around lines 195 - 204, Update the pin toggle dropdown item's disabled condition to compare isTogglingPin with item.uuid, matching the identifier assigned by the toggle flow, while preserving the existing disabled behavior for other states.
🧹 Nitpick comments (16)
app/_server/actions/note/migration.ts (1)
7-13: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider memoizing the negative
needsMigration()result. It runsfs.accessplus an order-file scan per mode on every page render that calls this guard; once migration is done the answer is stable for the process lifetime.🤖 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 `@app/_server/actions/note/migration.ts` around lines 7 - 13, Memoize the negative result in CheckForNeedsMigration so that after needsMigration() confirms no migration is required, subsequent authenticated calls return without repeating the filesystem access and order-file scan. Preserve the existing redirect behavior whenever needsMigration() reports that migration is needed.app/_server/actions/checklist/crud.ts (1)
315-327: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the failure before returning the generic error. The
updateListcatch now only logs the content event and swallows the exception (catch { }), so genuine write/rename failures become an opaque"Failed to update list"with no stack.deleteList/cloneChecklistkeep aconsole.error; worth matching.♻️ Suggested change
} catch (error) { + console.error("Error updating list:", error); try { const { title, uuid } = getFormData(formData, ["title", "uuid"]);🤖 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 `@app/_server/actions/checklist/crud.ts` around lines 315 - 327, Update the updateList catch block to log the original caught error with console.error before returning the generic failure response, matching the existing deleteList and cloneChecklist behavior. Preserve the checklist event logging and keep its nested catch from masking the primary update failure.tests/server-actions/note.test.ts (1)
132-142: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the new
bouncerdenial branch.The default mock always returns
{ allowed: true }, while the denial setup only setsmockCanReachtofalse. Add a case wherecanReachsucceeds butbouncerrejects the resolved target, matching the two-stage authorization flow indeleteNote.🤖 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 `@tests/server-actions/note.test.ts` around lines 132 - 142, In the deleteNote tests, add a denial case where mockCanReach resolves true but mockBouncer resolves with allowed false for the target returned by mockTargetDir. Keep the existing successful authorization setup and assert that deleteNote follows the bouncer-denial behavior rather than the canReach failure path.app/_components/FeatureComponents/Sidebar/Parts/CategoryRenderer.tsx (2)
75-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded
"Uncategorized"literal.See consolidated comment for this and the matching occurrence in
SidebarItem.tsx.🤖 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 `@app/_components/FeatureComponents/Sidebar/Parts/CategoryRenderer.tsx` around lines 75 - 78, Replace the hardcoded "Uncategorized" fallback in getItemsInCategory with the shared category-name constant used by the sidebar components, and apply the same constant to the matching fallback in SidebarItem.tsx.
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the shared "uncategorized" constant instead of the hardcoded
"Uncategorized"string. Both sites independently hardcode the fallback category name instead of referencing the new uncategorized constant introduced for this identity migration, risking silent drift if the value ever changes.
app/_components/FeatureComponents/Sidebar/Parts/CategoryRenderer.tsx#L75-78: replace the literal"Uncategorized"fallback ingetItemsInCategorywith the shared constant.app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx#L339-339: replace the literal"Uncategorized"fallback in theMetadataProviderpayload with the same shared constant.🤖 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 `@app/_components/FeatureComponents/Sidebar/Parts/CategoryRenderer.tsx` at line 1, Replace the hardcoded "Uncategorized" fallbacks in getItemsInCategory and the SidebarItem MetadataProvider payload with the shared uncategorized constant introduced for the identity migration, importing it where needed and preserving the existing fallback behavior.app/_components/GlobalComponents/Indicators/ShareBadges.tsx (1)
62-69: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winClickable badge span is not keyboard-operable.
onClickis attached to a plain<span>with norole,tabIndex, or key handler, so keyboard users can't trigger the "shared with" modal thatChecklistHeader/SidebarItemwire through thisonClick.♻️ Suggested fix
<span className={cn( "flex items-center gap-1 shrink-0", onClick && "cursor-pointer hover:text-primary", className, )} onClick={onClick} + role={onClick ? "button" : undefined} + tabIndex={onClick ? 0 : undefined} + onKeyDown={ + onClick + ? (e) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + onClick(); + } + } + : undefined + } >🤖 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 `@app/_components/GlobalComponents/Indicators/ShareBadges.tsx` around lines 62 - 69, Update the clickable badge element in ShareBadges to be keyboard-operable when onClick is provided: add an appropriate interactive role, make it focusable, and handle keyboard activation for Enter and Space while preserving the existing click behavior and non-clickable rendering.app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx (1)
339-339: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded
"Uncategorized"literal.See consolidated comment for this and the matching occurrence in
CategoryRenderer.tsx.🤖 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 `@app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx` at line 339, Replace the hardcoded "Uncategorized" fallback in the SidebarItem category mapping with the shared category-label constant or localization symbol used by CategoryRenderer.tsx, ensuring both occurrences use the same centralized value.app/_hooks/useSharingTools.ts (1)
70-75: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
modeOfis a server action; awaiting it client-side costs a round trip per call.
modeOflives inapp/_server/actions/share/queries.tsunder"use server", so everyawait modeOf(itemType)in this hook (Lines 71, 104, 237, 273, 295, 333, 361) is an RPC that only maps an enum. Consider a pure client-side helper (e.g. inapp/_utils/sharing-utils.ts) and keepmodeOffor server-only callers.🤖 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 `@app/_hooks/useSharingTools.ts` around lines 70 - 75, Replace the client-side modeOf calls in useSharingTools with a pure client-safe helper that maps itemType to the required sharing mode, defined in the appropriate client utility module. Update every listed call site to use the helper without awaiting it, while retaining modeOf for server-only callers.app/_server/actions/share/access.ts (2)
67-91: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a separator-aware containment check in the walk-up guard.
current.startsWith(userDir)treats.../notes/bob2as inside.../notes/bob. Today both call sites deriveuserDirfrom the same path so it can't trigger, but it's a cheap hardening against future callers passing a mismatched pair.♻️ Suggested guard
- while (current.startsWith(userDir)) { + const contained = (dir: string) => + dir === userDir || dir.startsWith(`${userDir}${path.sep}`); + + while (contained(current)) {🤖 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 `@app/_server/actions/share/access.ts` around lines 67 - 91, Update the walk-up guard in _chainGrants to use separator-aware path containment, so a sibling path such as userDir plus an unrelated suffix is not treated as inside userDir. Preserve traversal for userDir itself and descendants, using the platform path utilities rather than a raw startsWith check.
221-241: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueTwo full-file scans per candidate in the loose-mount pass.
resolveAccessalready callsgrepExtractFrontmatter(filePath), so Line 231 re-reads the same frontmatter. On large trees this doubles I/O for every candidate file. Returning the uuid (or the parsed metadata) fromresolveAccesswould halve it.🤖 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 `@app/_server/actions/share/access.ts` around lines 221 - 241, Update resolveAccess and the loose-mount loop to reuse the access-resolution result’s parsed frontmatter or UUID instead of calling grepExtractFrontmatter(filePath) again. Preserve the existing missing-UUID skip behavior and use the reused value when adding entries to loose.app/_server/actions/share/queries.ts (2)
151-153: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInline dynamic import of
./target.Every other dependency here is statically imported. If this is working around a cycle, a short comment saying so helps; otherwise hoist it to the top.
🤖 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 `@app/_server/actions/share/queries.ts` around lines 151 - 153, Update the code around the targetDir call to statically import the ./target dependency with the other imports and remove the inline dynamic import; if the dynamic import is required to avoid a dependency cycle, retain it and add a brief comment documenting that reason.
232-255: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
_factsForrescans the whole tree per exported query.
globalShares,sharedForUserandallSharedeach invoke_factsForfor both modes, and_factsFordoes a grep sweep plus two awaited file reads per candidate. Per the stack contextapp/layout.tsxnow wires all three, so a single page render can repeat the same full scan up to six times. Wrapping_factsForin React's request-scopedcache()(orunstable_cachewith the sharing tags already used inoperations.ts) would collapse them.Also applies to: 282-350
🤖 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 `@app/_server/actions/share/queries.ts` around lines 232 - 255, The _factsFor function repeats the same full-tree scan across exported share queries during one render. Wrap _factsFor with the existing request-scoped caching mechanism, keyed by its Modes argument, so globalShares, sharedForUser, and allShared reuse results for each mode while preserving the current fact-building behavior.app/_server/actions/share/category-info.ts (1)
80-98: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚖️ Poor tradeoffRead-modify-write on
.category-info.jsoncan lose concurrent updates.
catUuid,patchCatInfoandwriteCatOrdereach read then write without coordination. Two concurrent Server Actions on the same folder (e.g. a share plus a reorder) will last-write-wins, and incatUuid's case both callers can mint different uuids while only one persists — the loser then holds a uuid that resolves to nothing. A per-directory in-process mutex around read+write would close this.🤖 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 `@app/_server/actions/share/category-info.ts` around lines 80 - 98, Add a per-directory in-process mutex and use it to serialize each read-modify-write operation in catUuid, patchCatInfo, and writeCatOrder. Hold the directory lock across both the read and write, ensuring concurrent UUID generation cannot return an unpersisted value and updates cannot overwrite one another; preserve existing return behavior.app/_server/actions/category/queries.ts (1)
43-52: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSequential
catAccessper subcategory on a layout-critical path.
getCategoriesruns twice per root layout render (notes + checklists), and each mount subtree resolves access one directory at a time, each walking the parent chain. Batch these withPromise.alloversubTreeto cut the round trips.♻️ Parallelize access checks
- for (const category of subTree) { - const relative = category.path.slice(mount.displayName.length + 1); - const access = await catAccess(mode, path.join(ownerDir, relative)); - - const perms = access?.users[username]; - - if (perms) { - visible.push({ ...category, sharedFrom: mount.owner, permissions: perms }); - } - } + const resolved = await Promise.all( + subTree.map(async (category) => { + const relative = category.path.slice(mount.displayName.length + 1); + const access = await catAccess(mode, path.join(ownerDir, relative)); + return { category, perms: access?.users[username] }; + }), + ); + + for (const { category, perms } of resolved) { + if (perms) { + visible.push({ ...category, sharedFrom: mount.owner, permissions: perms }); + } + }🤖 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 `@app/_server/actions/category/queries.ts` around lines 43 - 52, Update the subcategory access-resolution loop in getCategories to invoke catAccess for all entries in subTree concurrently with Promise.all, then apply the existing permissions filtering and visible.push mapping to the resolved results. Preserve the current relative path, owner, username, and sharedFrom behavior while removing sequential awaits.app/_server/actions/category/crud.ts (1)
57-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMount success path drops the audit log and the
datapayload.The owned-directory branch logs an
INFOcategory_createdentry and returns{ success: true, data: { name, count: 0 } }; this branch returns bare{ success: true }and logs nothing on success. Worth aligning so shared-folder creates are auditable and callers get the same shape.🤖 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 `@app/_server/actions/category/crud.ts` around lines 57 - 71, Update the successful mount-directory branch around ensureDir, revalidateTag, and broadcast to record the same INFO category_created audit entry as the owned-directory branch, then return { success: true, data: { name, count: 0 } } so both create paths share the same response shape.app/_server/actions/category/move.ts (1)
75-93: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winEvery item drag now triggers a recursive disk walk per candidate owner.
_locateItemresolves the UUID by scanninguserDirFor(mode, owner)for the current user plus every mount owner, sequentially, on each drop. With a few hundred notes across several shared owners this is a noticeable regression versus the previous form-suppliedactiveId/category. Consider resolving the UUID from the already-cached metadata (getUserNotes/getUserChecklistswithmetadataOnly, which go throughgetOrCompute) and only falling back to the walk on a miss.🤖 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 `@app/_server/actions/category/move.ts` around lines 75 - 93, The _locateItem function performs a sequential disk walk for every candidate owner on each drag. Update it to first resolve the UUID from cached metadata using getUserNotes/getUserChecklists with metadataOnly through getOrCompute, preserving the corresponding category conversion via shownAs; only invoke the existing _findItemByUuid walk when cached metadata misses.
🤖 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 `@app/_components/FeatureComponents/Migration/Parts/ShareMigrationView.tsx`:
- Around line 46-56: Replace the hardcoded English descriptions and all other
literal migration copy in ShareMigrationView with next-intl translations,
reusing the sharing-related keys added to the locale files. Update the
component’s translation calls consistently across the “Migration Changes” block
and the additional referenced sections, while preserving the existing
conditional content and layout.
In
`@app/_components/GlobalComponents/Modals/ChecklistModals/EditChecklistModal.tsx`:
- Around line 57-59: Update the fetched-checklist refresh logic in
EditChecklistModal to synchronize the form’s category state from
fetchedChecklist.category alongside the existing title update. Preserve the
fallback behavior and existing unarchive flow, ensuring owner submissions use
the refreshed category rather than initialChecklist.category.
In `@app/_server/actions/category/queries.ts`:
- Around line 22-30: Update the root Category construction to derive count for
non-implicit mounts from the ownerDir’s markdown-item count, reusing the
existing category-tree/counting logic where possible instead of hardcoding 0.
Preserve the current implicit-mount count behavior and keep the resulting root
count consistent with its subcategories.
In `@app/_server/actions/checklist/crud.ts`:
- Around line 48-62: Validate the client-provided user identity before deriving
the target directory: in app/_server/actions/checklist/crud.ts lines 48-62,
ensure the parsed userParam username matches the session user unless the
requester is an admin, then continue using the validated acting identity for
targetDir and bouncer. Apply the same validation to formUser in
app/_server/actions/note/crud.ts lines 59-79, and invoke bouncer unconditionally
rather than only when target.isMount.
In `@app/_server/actions/lib/legacy-lookup.ts`:
- Around line 63-67: Serialize the UUID-stamping flow surrounding generateUuid,
fs.readFile, and fs.writeFile so concurrent legacy requests cannot generate
conflicting identifiers. Use a cross-process exclusive lock and reread the file
while holding it, reusing any UUID another process already stamped; otherwise
generate and persist one UUID, then release the lock before returning it.
In `@app/_server/actions/lib/migration-check.ts`:
- Around line 29-42: Update needsMigration to cache a clean migration check in
module state after scanning all CHECKED_MODES and finding neither
LEGACY_SHARING_FILE nor order files; return the cached false result on
subsequent calls while preserving the immediate true result when migration
artifacts are found.
In `@app/_server/actions/migration/share-migration.ts`:
- Around line 95-117: Update the legacy order conversion flow to detect when
resolved category or item entries are missing, especially when _uuidOfFile
cannot resolve a legacy item. Before fs.unlink in the conversion function,
preserve the legacy file or record the required change entry whenever the
resolved lists are shorter than the original lists, and only remove it after all
ordering data has been successfully retained.
- Around line 201-214: Update the migration loop around _applyShares to track
whether any file share writes fail; continue processing entries, but do not
unlink LEGACY_SHARING_FILE when failures occurred. Surface the failure after
processing so callers can detect the incomplete migration, while preserving the
existing success path that records changes and removes the legacy file.
- Around line 216-239: Protect migrateToInlineSharing with a server-side
isAdmin() check before invoking _userDirs, _stampTree, or _migrateShares. Return
the action’s existing failure Result when the caller is not an administrator,
ensuring no filesystem changes occur for unauthorized requests.
In `@app/_server/actions/note/crud.ts`:
- Line 375: Update the broadcast call in the note update flow to use the
normalized actingUsername value instead of currentUser for the username payload.
Preserve the existing note action and entityId fields while ensuring username is
always the normalized string used elsewhere in the function.
In `@app/_server/actions/share/operations.ts`:
- Around line 310-331: Caller-supplied category paths are not constrained to the
owner's directory. In app/_server/actions/share/operations.ts lines 310-331, add
a shared _safeDir(mode, owner, categoryPath) helper that resolves the path and
verifies containment, then have shareFolder use it and return an "Invalid
category" error when it returns null; apply the same guard before
readCatInfo/writeCatInfo in lines 401-419 for setFolderInherit and in lines
441-466 for setFolderPublic's un-publish branch. The publish branch requires no
direct change because it uses shareFolder.
In `@app/_server/actions/share/rename.ts`:
- Around line 30-64: The _renameInFiles function must invalidate the
mode-specific sharing cache after successfully rewriting grants. Track whether
any files were updated, then call the existing _modeTag(mode) revalidation
mechanism after the rename operation when changes occurred, preserving the
current touched count and file-processing behavior.
- Around line 99-111: Add authorization to the exported server action
renameGrants before it calls _renameInFiles or _renameInCats, requiring
isAdmin() as established in operations.ts and rejecting non-admin callers.
Preserve the existing rename behavior for authorized administrators, or move the
implementation into a non-"use server" internal module and expose it only
through the authorized users/crud flow.
In `@app/_server/actions/share/target.ts`:
- Around line 46-81: Contain mount-resolved paths in targetDir by resolving the
owner-side directory against the owner root and returning own when the resolved
path escapes that root. In app/_server/actions/share/target.ts lines 46-81,
apply this to the mount branch while preserving valid mount metadata. In
app/_server/actions/category/crud.ts lines 35-71, validate name and the
resulting joined path against mountParent.dir with isPathSafe before calling
ensureDir; both sites must reject traversal outside the shared mount.
In `@app/_server/actions/ws/broadcast.ts`:
- Around line 1-11: Remove the "use server" directive from the broadcast module
so broadcast is not exposed as a callable server action. Keep the existing
broadcast function and its server-side consumers unchanged, preserving the
__jottyBroadcast guard and event forwarding behavior.
In `@app/_translations/de.json`:
- Around line 1340-1347: Correct the localized sharing strings for the keys
deleteTip through confirmLeaveFolder in app/_translations/de.json lines
1340-1347 by restoring German umlauts; apply the requested Spanish accents and
opening question marks in app/_translations/es.json lines 1341-1347, French
accents and partagé inflections in app/_translations/fr.json lines 1341-1347,
and Italian accents in app/_translations/it.json lines 1339-1347; remove the
space before 를 in app/_translations/ko.json line 1356; and restore Polish
diacritics in app/_translations/pl.json lines 1339-1347.
In `@app/_translations/pt.json`:
- Around line 1363-1372: Update the new translation values around readTip,
writeTip, deleteTip, publicTip, fromUserRead, fromUserWrite, and fromUserDelete
to use consistent European Portuguese wording, replacing Brazilian forms with
the file’s established equivalents. Add the required diacritics in
confirmLeaveItem and confirmLeaveFolder, including próprio, proprietário, and
terá, while preserving the placeholders and message meaning.
In `@app/_translations/tr.json`:
- Around line 1366-1374: Update the newly added Turkish translation values
around writeTip, deleteTip, publicTip, fromUserRead, fromUserWrite,
fromUserDelete, leaveShare, confirmLeaveItem, and confirmLeaveFolder to use
correct Turkish diacritics consistently with the surrounding entries, while
preserving all placeholders and message meaning.
In `@app/_utils/category-utils.ts`:
- Around line 48-59: The category tree construction must not call catUuid
concurrently for folders missing info.uuid. Update the UUID provisioning path
used by the shown infoMap lookup to serialize legacy-folder initialization or
make catUuid re-read the persisted metadata atomically immediately before
writing, reusing an existing UUID when another request has already created one;
ensure each response exposes only the persisted UUID.
In `@app/layout.tsx`:
- Around line 199-208: Update the allSharedItems selection in the layout
data-loading flow to call allShared() for public or unauthenticated routes,
while retaining the existing user-specific shared data behavior for
authenticated non-public routes. Remove the inverted condition so
AppModeProvider receives public shared summaries instead of empty collections.
---
Outside diff comments:
In `@app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx`:
- Around line 195-204: Update the pin toggle dropdown item's disabled condition
to compare isTogglingPin with item.uuid, matching the identifier assigned by the
toggle flow, while preserving the existing disabled behavior for other states.
In `@app/public/checklist/`[...categoryPath]/page.tsx:
- Around line 19-25: Update the legacy resolution flow around legacyResolve in
the checklist page to resolve only items publicly accessible to unauthenticated
users, rather than searching across all owners. Use the existing
public-access-aware resolver or pass the appropriate public access context
before allowing the permanentRedirect; preserve the current redirect behavior
only for a publicly resolvable UUID.
---
Nitpick comments:
In `@app/_components/FeatureComponents/Sidebar/Parts/CategoryRenderer.tsx`:
- Around line 75-78: Replace the hardcoded "Uncategorized" fallback in
getItemsInCategory with the shared category-name constant used by the sidebar
components, and apply the same constant to the matching fallback in
SidebarItem.tsx.
- Line 1: Replace the hardcoded "Uncategorized" fallbacks in getItemsInCategory
and the SidebarItem MetadataProvider payload with the shared uncategorized
constant introduced for the identity migration, importing it where needed and
preserving the existing fallback behavior.
In `@app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx`:
- Line 339: Replace the hardcoded "Uncategorized" fallback in the SidebarItem
category mapping with the shared category-label constant or localization symbol
used by CategoryRenderer.tsx, ensuring both occurrences use the same centralized
value.
In `@app/_components/GlobalComponents/Indicators/ShareBadges.tsx`:
- Around line 62-69: Update the clickable badge element in ShareBadges to be
keyboard-operable when onClick is provided: add an appropriate interactive role,
make it focusable, and handle keyboard activation for Enter and Space while
preserving the existing click behavior and non-clickable rendering.
In `@app/_hooks/useSharingTools.ts`:
- Around line 70-75: Replace the client-side modeOf calls in useSharingTools
with a pure client-safe helper that maps itemType to the required sharing mode,
defined in the appropriate client utility module. Update every listed call site
to use the helper without awaiting it, while retaining modeOf for server-only
callers.
In `@app/_server/actions/category/crud.ts`:
- Around line 57-71: Update the successful mount-directory branch around
ensureDir, revalidateTag, and broadcast to record the same INFO category_created
audit entry as the owned-directory branch, then return { success: true, data: {
name, count: 0 } } so both create paths share the same response shape.
In `@app/_server/actions/category/move.ts`:
- Around line 75-93: The _locateItem function performs a sequential disk walk
for every candidate owner on each drag. Update it to first resolve the UUID from
cached metadata using getUserNotes/getUserChecklists with metadataOnly through
getOrCompute, preserving the corresponding category conversion via shownAs; only
invoke the existing _findItemByUuid walk when cached metadata misses.
In `@app/_server/actions/category/queries.ts`:
- Around line 43-52: Update the subcategory access-resolution loop in
getCategories to invoke catAccess for all entries in subTree concurrently with
Promise.all, then apply the existing permissions filtering and visible.push
mapping to the resolved results. Preserve the current relative path, owner,
username, and sharedFrom behavior while removing sequential awaits.
In `@app/_server/actions/checklist/crud.ts`:
- Around line 315-327: Update the updateList catch block to log the original
caught error with console.error before returning the generic failure response,
matching the existing deleteList and cloneChecklist behavior. Preserve the
checklist event logging and keep its nested catch from masking the primary
update failure.
In `@app/_server/actions/note/migration.ts`:
- Around line 7-13: Memoize the negative result in CheckForNeedsMigration so
that after needsMigration() confirms no migration is required, subsequent
authenticated calls return without repeating the filesystem access and
order-file scan. Preserve the existing redirect behavior whenever
needsMigration() reports that migration is needed.
In `@app/_server/actions/share/access.ts`:
- Around line 67-91: Update the walk-up guard in _chainGrants to use
separator-aware path containment, so a sibling path such as userDir plus an
unrelated suffix is not treated as inside userDir. Preserve traversal for
userDir itself and descendants, using the platform path utilities rather than a
raw startsWith check.
- Around line 221-241: Update resolveAccess and the loose-mount loop to reuse
the access-resolution result’s parsed frontmatter or UUID instead of calling
grepExtractFrontmatter(filePath) again. Preserve the existing missing-UUID skip
behavior and use the reused value when adding entries to loose.
In `@app/_server/actions/share/category-info.ts`:
- Around line 80-98: Add a per-directory in-process mutex and use it to
serialize each read-modify-write operation in catUuid, patchCatInfo, and
writeCatOrder. Hold the directory lock across both the read and write, ensuring
concurrent UUID generation cannot return an unpersisted value and updates cannot
overwrite one another; preserve existing return behavior.
In `@app/_server/actions/share/queries.ts`:
- Around line 151-153: Update the code around the targetDir call to statically
import the ./target dependency with the other imports and remove the inline
dynamic import; if the dynamic import is required to avoid a dependency cycle,
retain it and add a brief comment documenting that reason.
- Around line 232-255: The _factsFor function repeats the same full-tree scan
across exported share queries during one render. Wrap _factsFor with the
existing request-scoped caching mechanism, keyed by its Modes argument, so
globalShares, sharedForUser, and allShared reuse results for each mode while
preserving the current fact-building behavior.
In `@tests/server-actions/note.test.ts`:
- Around line 132-142: In the deleteNote tests, add a denial case where
mockCanReach resolves true but mockBouncer resolves with allowed false for the
target returned by mockTargetDir. Keep the existing successful authorization
setup and assert that deleteNote follows the bouncer-denial behavior rather than
the canReach failure path.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a0018f1b-cdf3-4094-87f6-b5c5965e2e6f
📒 Files selected for processing (131)
app/(loggedInRoutes)/checklist/[...categoryPath]/page.tsxapp/(loggedInRoutes)/checklist/[uuid]/page.tsxapp/(loggedInRoutes)/note/[...categoryPath]/page.tsxapp/(loggedInRoutes)/note/[uuid]/page.tsxapp/_components/FeatureComponents/Admin/Parts/Sharing/AdminSharing.tsxapp/_components/FeatureComponents/Admin/Parts/ThemePreview.tsxapp/_components/FeatureComponents/Checklists/Parts/Common/ChecklistHeader.tsxapp/_components/FeatureComponents/Kanban/KanbanCardDetail.tsxapp/_components/FeatureComponents/Migration/MigrationPage.tsxapp/_components/FeatureComponents/Migration/Parts/ShareMigrationView.tsxapp/_components/FeatureComponents/Migration/Parts/YamlMetadataMigrationView.tsxapp/_components/FeatureComponents/Notes/Parts/NoteEditor/NoteEditorHeader.tsxapp/_components/FeatureComponents/Sidebar/Parts/CategoryList.tsxapp/_components/FeatureComponents/Sidebar/Parts/CategoryRenderer.tsxapp/_components/FeatureComponents/Sidebar/Parts/SharedItemsList.tsxapp/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsxapp/_components/FeatureComponents/Sidebar/Sidebar.tsxapp/_components/GlobalComponents/Indicators/ShareBadges.tsxapp/_components/GlobalComponents/Indicators/SharedFromBadge.tsxapp/_components/GlobalComponents/Modals/ChecklistModals/EditChecklistModal.tsxapp/_components/GlobalComponents/Modals/NotesModal/EditNoteModal.tsxapp/_components/GlobalComponents/Modals/NotesModal/NoteHistoryModal.tsxapp/_components/GlobalComponents/Modals/SharingModals/FolderShareModal.tsxapp/_components/GlobalComponents/Modals/SharingModals/Parts/InheritedNotice.tsxapp/_components/GlobalComponents/Modals/SharingModals/ShareModal.tsxapp/_consts/files.tsapp/_consts/sharing.tsapp/_hooks/useFolderShare.tsapp/_hooks/useSharingTools.tsapp/_hooks/useSidebar.tsxapp/_schemas/sharing-schemas.tsapp/_server/actions/category/crud.tsapp/_server/actions/category/index.tsapp/_server/actions/category/move.tsapp/_server/actions/category/ordering.tsapp/_server/actions/category/queries.tsapp/_server/actions/checklist-item/archive.tsapp/_server/actions/checklist-item/bulk-operations.tsapp/_server/actions/checklist-item/crud.tsapp/_server/actions/checklist-item/drop.tsapp/_server/actions/checklist-item/reorder.tsapp/_server/actions/checklist-item/status.tsapp/_server/actions/checklist-item/sub-items.tsapp/_server/actions/checklist/converters.tsapp/_server/actions/checklist/crud.tsapp/_server/actions/checklist/queries.tsapp/_server/actions/checklist/readers.tsapp/_server/actions/file/index.tsapp/_server/actions/history/index.tsapp/_server/actions/kanban/items.tsapp/_server/actions/kanban/time-entries.tsapp/_server/actions/lib/legacy-lookup.tsapp/_server/actions/lib/metadata-cache.tsapp/_server/actions/lib/migration-check.tsapp/_server/actions/migration/folder-migration.tsapp/_server/actions/migration/helpers.tsapp/_server/actions/migration/index-migration.tsapp/_server/actions/migration/index.tsapp/_server/actions/migration/share-migration.tsapp/_server/actions/migration/sharing-migration.tsapp/_server/actions/migration/yaml-migration.tsapp/_server/actions/note/crud.tsapp/_server/actions/note/migration.tsapp/_server/actions/note/queries.tsapp/_server/actions/note/readers.tsapp/_server/actions/notifications/index.tsapp/_server/actions/reminders/scanner.tsapp/_server/actions/share/access.tsapp/_server/actions/share/category-info.tsapp/_server/actions/share/mounts.tsapp/_server/actions/share/operations.tsapp/_server/actions/share/queries.tsapp/_server/actions/share/rename.tsapp/_server/actions/share/target.tsapp/_server/actions/sharing/helpers.tsapp/_server/actions/sharing/index.tsapp/_server/actions/sharing/io.tsapp/_server/actions/sharing/permissions.tsapp/_server/actions/sharing/queries.tsapp/_server/actions/sharing/share-operations.tsapp/_server/actions/sharing/types.tsapp/_server/actions/sharing/updates.tsapp/_server/actions/users/crud.tsapp/_server/actions/ws/broadcast.tsapp/_translations/de.jsonapp/_translations/en.jsonapp/_translations/es.jsonapp/_translations/fr.jsonapp/_translations/it.jsonapp/_translations/klingon.jsonapp/_translations/ko.jsonapp/_translations/nl.jsonapp/_translations/pirate.jsonapp/_translations/pl.jsonapp/_translations/pt.jsonapp/_translations/ru.jsonapp/_translations/tr.jsonapp/_translations/zh.jsonapp/_types/audit.tsapp/_types/category.tsapp/_types/checklist.tsapp/_types/enums.tsapp/_types/note.tsapp/_types/sharing.tsapp/_utils/api-utils.tsapp/_utils/category-utils.tsapp/_utils/grep-utils.tsapp/_utils/order-utils.tsapp/_utils/sharing-utils.tsapp/_utils/sidebar-store.tsapp/api/checklists/[listId]/items/reorder/route.tsapp/api/file/[username]/[filename]/route.tsapp/api/image/[username]/[filename]/route.tsapp/api/notes/[noteId]/route.tsapp/api/video/[username]/[filename]/route.tsapp/layout.tsxapp/migration/page.tsxapp/public/checklist/[...categoryPath]/page.tsxapp/public/checklist/[uuid]/page.tsxapp/public/note/[...categoryPath]/page.tsxapp/public/note/[uuid]/page.tsxinstrumentation.tstests/api/setup.tstests/server-actions/category.test.tstests/server-actions/checklist-item.test.tstests/server-actions/dashboard.test.tstests/server-actions/drop-item.test.tstests/server-actions/file.test.tstests/server-actions/history.test.tstests/server-actions/note.test.tstests/server-actions/sharing.test.ts
💤 Files with no reviewable changes (21)
- app/_server/actions/sharing/io.ts
- app/_server/actions/migration/index-migration.ts
- app/_server/actions/sharing/helpers.ts
- app/_components/FeatureComponents/Migration/Parts/YamlMetadataMigrationView.tsx
- app/_components/FeatureComponents/Sidebar/Parts/SharedItemsList.tsx
- app/_server/actions/migration/yaml-migration.ts
- app/_server/actions/category/ordering.ts
- app/_server/actions/migration/helpers.ts
- app/_server/actions/migration/sharing-migration.ts
- app/_server/actions/sharing/updates.ts
- tests/server-actions/category.test.ts
- app/_server/actions/sharing/types.ts
- app/_server/actions/sharing/index.ts
- app/_server/actions/migration/folder-migration.ts
- app/_server/actions/category/index.ts
- app/_consts/files.ts
- app/_server/actions/sharing/permissions.ts
- app/_hooks/useSidebar.tsx
- app/_server/actions/sharing/share-operations.ts
- app/_components/FeatureComponents/Admin/Parts/Sharing/AdminSharing.tsx
- app/_server/actions/sharing/queries.ts
🚧 Files skipped from review as they are similar to previous changes (23)
- app/_server/actions/notifications/index.ts
- app/(loggedInRoutes)/note/[uuid]/page.tsx
- app/_components/GlobalComponents/Modals/SharingModals/ShareModal.tsx
- app/_server/actions/checklist-item/sub-items.ts
- app/_types/checklist.ts
- app/(loggedInRoutes)/checklist/[...categoryPath]/page.tsx
- app/(loggedInRoutes)/checklist/[uuid]/page.tsx
- app/(loggedInRoutes)/note/[...categoryPath]/page.tsx
- app/_server/actions/kanban/time-entries.ts
- app/public/note/[...categoryPath]/page.tsx
- app/_components/FeatureComponents/Notes/Parts/NoteEditor/NoteEditorHeader.tsx
- app/api/checklists/[listId]/items/reorder/route.ts
- app/public/checklist/[uuid]/page.tsx
- app/_server/actions/checklist-item/status.ts
- app/_server/actions/checklist/converters.ts
- app/public/note/[uuid]/page.tsx
- app/_server/actions/history/index.ts
- app/_server/actions/kanban/items.ts
- app/_components/GlobalComponents/Modals/NotesModal/EditNoteModal.tsx
- app/_server/actions/checklist-item/bulk-operations.ts
- app/api/notes/[noteId]/route.ts
- app/_server/actions/checklist/queries.ts
- app/_components/FeatureComponents/Kanban/KanbanCardDetail.tsx
🛑 Comments failed to post (1)
app/_server/actions/ws/broadcast.ts (1)
1-11: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
"use server"turns this internal helper into a callable endpoint.Every export in a
"use server"module gets a public action ID that any client can invoke with arbitrary arguments — here that means spoofingtype/action/entityId/usernamein websocket events to other users, with no auth or validation. This helper is only consumed server-side (category CRUD, kanban, checklist items), so it doesn't need to be an action: drop the directive (or add an auth check + deriveusernameserver-side).🔒 Suggested change
-"use server"; - import type { WsEvent } from "`@/app/_types`"; -export async function broadcast(event: Omit<WsEvent, "connectionId">): Promise<void> { +export async function broadcast( + event: Omit<WsEvent, "connectionId">, +): Promise<void> { if (!globalThis.__jottyBroadcast) { console.warn("[ws] broadcast called but __jottyBroadcast not set"); return; } globalThis.__jottyBroadcast(event); }📝 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.import type { WsEvent } from "`@/app/_types`"; export async function broadcast( event: Omit<WsEvent, "connectionId">, ): Promise<void> { if (!globalThis.__jottyBroadcast) { console.warn("[ws] broadcast called but __jottyBroadcast not set"); return; } globalThis.__jottyBroadcast(event); }🤖 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 `@app/_server/actions/ws/broadcast.ts` around lines 1 - 11, Remove the "use server" directive from the broadcast module so broadcast is not exposed as a callable server action. Keep the existing broadcast function and its server-side consumers unchanged, preserving the __jottyBroadcast guard and event forwarding behavior.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@app/_components/GlobalComponents/FormElements/RadiusSlider.tsx`:
- Around line 61-74: Add an accessible translated name to the range input in
RadiusSlider, using aria-label or an associated aria-labelledby, and provide the
formatted radius through aria-valuetext. Preserve the existing value and change
behavior while ensuring the announced value matches the displayed radius.
In `@app/_hooks/useBorderRadius.ts`:
- Around line 58-66: Update the useBorderRadius hook around handleRadiusChange
and handleResetRadius to restore the previously saved --jotty-radius-admin value
during unmount cleanup. Capture the original property value when the hook
mounts, then remove or restore that exact value on cleanup so unsaved slider
previews do not persist, while preserving saved-radius reset behavior.
- Around line 32-56: Update the useEffect load flow in useBorderRadius to track
whether the effect has been cleaned up or superseded, and return a cleanup
function that marks the load stale. Check this guard after getBorderRadius
resolves and before each post-await state update, including setRadius,
setSavedRadius, and setIsLoading, so stale or unmounted loads cannot update
state.
In `@app/_hooks/useShowEmojis.ts`:
- Line 8: Update the return logic in useShowEmojis to give the user's explicit
preference precedence: return false when user.showChecklistEmojis is "disable",
true when it is "enable", and only then fall back to sessionShowEmojis. Preserve
the existing session-based behavior when no explicit user preference is set.
In `@app/_server/actions/config/settings.ts`:
- Around line 228-231: Update updateAppSettings and saveBorderRadius to prevent
concurrent full-settings snapshots from overwriting one another. Serialize each
read-modify-write operation or route both through a shared atomic
settings-update helper, preserving unrelated fields while applying the requested
changes.
- Around line 62-64: Update the borderRadius normalization in the settings
boundary so numeric persisted values are passed through clampRadius, while
retaining DEFAULT_BORDER_RADIUS for non-number values. Ensure getSettings()
exposes only radius values within the allowed range.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f9b04a68-3e79-4545-8b0b-2842ade5f364
📒 Files selected for processing (50)
app/_components/FeatureComponents/Admin/Parts/Sharing/AdminSharing.tsxapp/_components/FeatureComponents/Admin/Parts/StylingTab.tsxapp/_components/FeatureComponents/Checklists/Parts/Simple/NestedChecklistItem.tsxapp/_components/FeatureComponents/Migration/Parts/ShareMigrationView.tsxapp/_components/FeatureComponents/Notes/Parts/NoteEditor/NoteEditorContent.tsxapp/_components/FeatureComponents/Notes/Parts/TipTap/MinimalModeEditor.tsxapp/_components/FeatureComponents/Notes/Parts/TipTap/TipTapEditor.tsxapp/_components/FeatureComponents/Profile/Parts/ConnectionsGraph/ConnectionsGraph.tsxapp/_components/FeatureComponents/Profile/Parts/LinksTab.tsxapp/_components/FeatureComponents/Profile/Parts/UserPreferencesTab.tsxapp/_components/FeatureComponents/PublicView/PublicNoteView.tsxapp/_components/GlobalComponents/Cards/FileCard.tsxapp/_components/GlobalComponents/Cards/InfoCard.tsxapp/_components/GlobalComponents/Cards/NoteCard.tsxapp/_components/GlobalComponents/FormElements/RadiusSlider.tsxapp/_components/GlobalComponents/Layout/ReadingProgressBar.tsxapp/_components/GlobalComponents/Modals/Modal.tsxapp/_components/GlobalComponents/Modals/SettingsModals/Settings.tsxapp/_consts/styling.tsapp/_consts/themes.tsxapp/_hooks/useBorderRadius.tsapp/_hooks/useRadiusOverride.tsapp/_hooks/useShowEmojis.tsapp/_hooks/useStyling.tsxapp/_providers/EmojiProvider.tsxapp/_providers/ThemeProvider.tsxapp/_schemas/user-schemas.tsapp/_server/actions/config/index.tsapp/_server/actions/config/settings.tsapp/_styles/globals.cssapp/_styles/themes/colors.cssapp/_translations/de.jsonapp/_translations/en.jsonapp/_translations/es.jsonapp/_translations/fr.jsonapp/_translations/it.jsonapp/_translations/klingon.jsonapp/_translations/ko.jsonapp/_translations/nl.jsonapp/_translations/pirate.jsonapp/_translations/pl.jsonapp/_translations/pt.jsonapp/_translations/ru.jsonapp/_translations/tr.jsonapp/_translations/zh.jsonapp/_types/config.tsapp/_types/index.tsapp/_types/user.tsapp/_utils/settings-store.tsapp/layout.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- app/_components/FeatureComponents/Notes/Parts/NoteEditor/NoteEditorContent.tsx
- app/layout.tsx
- app/_components/FeatureComponents/Admin/Parts/Sharing/AdminSharing.tsx
- app/_components/GlobalComponents/Cards/NoteCard.tsx
- app/_components/FeatureComponents/Migration/Parts/ShareMigrationView.tsx
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/_server/actions/migration/share-migration.ts (1)
146-153: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRoute the order write through
patchCatInfoto stay inside the category lane.
_convertOrderreads withreadCatInfoand writes withwriteCatInfooutside_lane(dirPath).catUuidandpatchCatInfoinapp/_server/actions/share/category-info.tsserialize the same.category-info.jsonon that lane. The migration runs while the app serves requests, so a concurrent category UUID assignment can be lost, or this order write can be overwritten.
_convertOrderis not executed inside a queued body, so callingpatchCatInfohere is safe.🛡️ Proposed fix
- const info = await readCatInfo(dirPath); - await writeCatInfo(dirPath, { - ...info, - order: { - categories: categories.length > 0 ? categories : undefined, - items: items.length > 0 ? items : undefined, - }, - }); + await patchCatInfo(dirPath, { + order: { + categories: categories.length > 0 ? categories : undefined, + items: items.length > 0 ? items : undefined, + }, + });Update the import to include
patchCatInfofrom@/app/_server/actions/share/category-info.🤖 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 `@app/_server/actions/migration/share-migration.ts` around lines 146 - 153, Update _convertOrder to import and use patchCatInfo from category-info instead of separately calling readCatInfo and writeCatInfo, applying the categories and items order fields through the lane-aware patch operation.
🧹 Nitpick comments (2)
tests/server-actions/note.test.ts (1)
282-297: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the session username reaches
targetDir, not just the resulting owner.
createNotesetsowner: target.owner, andtargetDiris mocked with a fixed owner. The current assertion passes even ifformUserwon thegetCurrentUser() || formUserprecedence, so the test does not prove impersonation is ignored. Assert the username argument passed to thetargetDirmock.💚 Proposed assertion
const result = await createNote(formData); expect(result.success).toBe(true); expect(result.data?.owner).toBe("testuser"); + expect(mockTargetDir).toHaveBeenCalledWith( + expect.anything(), + "testuser", + "TestCategory", + );🤖 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 `@tests/server-actions/note.test.ts` around lines 282 - 297, Update the “should ignore a formData user that contradicts the session” test to assert that the targetDir mock receives the session username “testuser” as its username argument. Keep the existing result assertions, but verify the targetDir call directly so the test proves formUser cannot override getCurrentUser().app/_server/actions/lib/concurrency.ts (1)
30-43: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
runQueuedis not reentrant, and the per-directory lane owner does not enforce that. A task that callsrunQueuedagain with the same key queues behind its own ancestor and never settles, so the request hangs and every later task on that lane is blocked. The constraint is neither documented at the helper nor guarded at the call sites that share one lane.
app/_server/actions/lib/concurrency.ts#L30-L43: extend the module header to state that a queued task must never callrunQueuedwith the same key.app/_server/actions/share/category-info.ts#L86-L103: confirm that no body ofpatchCatInfo,catUuid, orwriteCatOrderreaches another_lane(dirPath)-queued function, and keep nested work in unqueued internal helpers.🤖 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 `@app/_server/actions/lib/concurrency.ts` around lines 30 - 43, Document in the concurrency module header that tasks run by runQueued must not call runQueued again with the same key. In app/_server/actions/share/category-info.ts lines 86-103, verify that patchCatInfo, catUuid, and writeCatOrder do not invoke other functions queued through _lane(dirPath); move any nested work into unqueued internal helpers while preserving the existing per-directory queue behavior.
🤖 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 `@app/_hooks/useChecklist.tsx`:
- Around line 198-207: Update confirmDeleteList around the deleteList(formData)
call to catch rejected server-action promises. On rejection, show the same
generic error toast used by the existing !result?.success branch, then return so
the modal flow is handled consistently.
In `@app/_server/actions/history/index.ts`:
- Around line 394-397: The _matches predicate in the history lookup must not
accept missing-UUID files before checking all repository content for a strict
UUID match. Run the strict UUID scan first; only when it finds no matching note
should the legacy no-UUID candidate be accepted, while preserving pre-migration
resolution and ensuring repository selection remains constrained to the
validated note owner.
In `@app/`(loggedInRoutes)/checklist/[uuid]/page.tsx:
- Around line 41-48: Replace decodeURIComponent with decodeSegment in the legacy
identifier handling of checklist/[uuid]/page.tsx. Also update the terminal
category-path decoding in checklist/[...categoryPath]/page.tsx and
note/[...categoryPath]/page.tsx to use decodeSegment, preserving the existing
legacy resolution and redirect flows.
In `@app/public/checklist/`[...categoryPath]/page.tsx:
- Line 31: Update legacy path resolution so all matching candidates are returned
before public-access filtering, and redirect only when exactly one candidate is
public. Apply this public-aware resolution at
app/public/checklist/[...categoryPath]/page.tsx:31-31,
app/public/note/[...categoryPath]/page.tsx:29-29, and the invalid-UUID fallback
at app/public/note/[uuid]/page.tsx:44-44, replacing the current first-match UUID
selection followed by isPublicItem checks.
---
Outside diff comments:
In `@app/_server/actions/migration/share-migration.ts`:
- Around line 146-153: Update _convertOrder to import and use patchCatInfo from
category-info instead of separately calling readCatInfo and writeCatInfo,
applying the categories and items order fields through the lane-aware patch
operation.
---
Nitpick comments:
In `@app/_server/actions/lib/concurrency.ts`:
- Around line 30-43: Document in the concurrency module header that tasks run by
runQueued must not call runQueued again with the same key. In
app/_server/actions/share/category-info.ts lines 86-103, verify that
patchCatInfo, catUuid, and writeCatOrder do not invoke other functions queued
through _lane(dirPath); move any nested work into unqueued internal helpers
while preserving the existing per-directory queue behavior.
In `@tests/server-actions/note.test.ts`:
- Around line 282-297: Update the “should ignore a formData user that
contradicts the session” test to assert that the targetDir mock receives the
session username “testuser” as its username argument. Keep the existing result
assertions, but verify the targetDir call directly so the test proves formUser
cannot override getCurrentUser().
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 375ee640-b7f8-429b-8fbd-494fc8207155
📒 Files selected for processing (83)
app/(loggedInRoutes)/checklist/[...categoryPath]/page.tsxapp/(loggedInRoutes)/checklist/[uuid]/page.tsxapp/(loggedInRoutes)/note/[...categoryPath]/page.tsxapp/(loggedInRoutes)/note/[uuid]/page.tsxapp/_components/FeatureComponents/Checklists/ChecklistsPageClient.tsxapp/_components/FeatureComponents/Checklists/Parts/Common/ChecklistHeader.tsxapp/_components/FeatureComponents/Checklists/TasksPageClient.tsxapp/_components/FeatureComponents/Home/Parts/NotesHome.tsxapp/_components/FeatureComponents/Kanban/KanbanPageClient.tsxapp/_components/FeatureComponents/Migration/Parts/ShareMigrationView.tsxapp/_components/FeatureComponents/Notes/NotesPageClient.tsxapp/_components/FeatureComponents/Notes/Parts/NoteEditor/NoteEditorHeader.tsxapp/_components/FeatureComponents/Notes/Parts/TipTap/CustomExtensions/InternalLinkComponent.tsxapp/_components/FeatureComponents/Notes/Parts/UnifiedMarkdownRenderer.tsxapp/_components/FeatureComponents/Sidebar/Parts/CategoryRenderer.tsxapp/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsxapp/_components/FeatureComponents/Tags/TagHoverCard.tsxapp/_components/GlobalComponents/FormElements/RadiusSlider.tsxapp/_components/GlobalComponents/Indicators/ShareBadges.tsxapp/_components/GlobalComponents/Modals/ChecklistModals/EditChecklistModal.tsxapp/_components/GlobalComponents/Modals/NotesModal/EditNoteModal.tsxapp/_components/GlobalComponents/Modals/SettingsModals/Settings.tsxapp/_consts/files.tsapp/_hooks/useBorderRadius.tsapp/_hooks/useChecklist.tsxapp/_hooks/useChecklistHome.tsxapp/_hooks/useFolderShare.tsapp/_hooks/useNoteEditor.tsxapp/_hooks/useNotesHome.tsxapp/_hooks/useSharingTools.tsapp/_hooks/useShowEmojis.tsapp/_hooks/useSidebar.tsxapp/_server/actions/category/crud.tsapp/_server/actions/category/queries.tsapp/_server/actions/checklist-item/crud.tsapp/_server/actions/checklist-item/reorder.tsapp/_server/actions/checklist-item/sub-items.tsapp/_server/actions/checklist/crud.tsapp/_server/actions/checklist/readers.tsapp/_server/actions/config/settings.tsapp/_server/actions/dashboard/index.tsapp/_server/actions/history/index.tsapp/_server/actions/kanban/tempo.tsapp/_server/actions/lib/concurrency.tsapp/_server/actions/lib/legacy-lookup.tsapp/_server/actions/lib/migration-check.tsapp/_server/actions/migration/share-migration.tsapp/_server/actions/note/crud.tsapp/_server/actions/note/readers.tsapp/_server/actions/share/access.tsapp/_server/actions/share/category-info.tsapp/_server/actions/share/operations.tsapp/_server/actions/share/queries.tsapp/_server/actions/share/rename.tsapp/_server/actions/share/target.tsapp/_server/actions/users/helpers.tsapp/_server/actions/ws/broadcast.tsapp/_translations/de.jsonapp/_translations/en.jsonapp/_translations/es.jsonapp/_translations/fr.jsonapp/_translations/it.jsonapp/_translations/pl.jsonapp/_translations/pt.jsonapp/_translations/tr.jsonapp/_types/sharing.tsapp/_utils/global-utils.tsapp/_utils/grep-utils.tsapp/_utils/markdown-utils.tsxapp/_utils/path-utils.tsapp/_utils/settings-store.tsapp/_utils/sharing-utils.tsapp/api/checklists/[listId]/items/route.tsapp/api/notes/[noteId]/route.tsapp/public/checklist/[...categoryPath]/page.tsxapp/public/checklist/[uuid]/page.tsxapp/public/note/[...categoryPath]/page.tsxapp/public/note/[uuid]/page.tsxtests/security/path-containment.test.tstests/server-actions/checklist-item.test.tstests/server-actions/note.test.tstests/server-actions/sharing.test.tstests/utils/concurrency.test.ts
💤 Files with no reviewable changes (2)
- app/_utils/grep-utils.ts
- app/_server/actions/ws/broadcast.ts
🚧 Files skipped from review as they are similar to previous changes (47)
- app/_translations/pl.json
- app/_components/FeatureComponents/Kanban/KanbanPageClient.tsx
- app/_translations/es.json
- app/_translations/fr.json
- app/_server/actions/share/rename.ts
- app/_translations/pt.json
- app/_components/FeatureComponents/Tags/TagHoverCard.tsx
- app/api/checklists/[listId]/items/route.ts
- app/_components/FeatureComponents/Notes/Parts/UnifiedMarkdownRenderer.tsx
- app/_translations/it.json
- app/_components/FeatureComponents/Notes/Parts/TipTap/CustomExtensions/InternalLinkComponent.tsx
- app/_hooks/useChecklistHome.tsx
- app/_hooks/useNotesHome.tsx
- app/_components/GlobalComponents/Modals/ChecklistModals/EditChecklistModal.tsx
- app/_components/FeatureComponents/Home/Parts/NotesHome.tsx
- app/_components/FeatureComponents/Checklists/TasksPageClient.tsx
- app/_server/actions/dashboard/index.ts
- app/_translations/tr.json
- app/_server/actions/users/helpers.ts
- app/api/notes/[noteId]/route.ts
- app/_components/FeatureComponents/Notes/NotesPageClient.tsx
- app/_components/FeatureComponents/Checklists/Parts/Common/ChecklistHeader.tsx
- app/_components/GlobalComponents/Modals/NotesModal/EditNoteModal.tsx
- app/_hooks/useNoteEditor.tsx
- app/_components/GlobalComponents/FormElements/RadiusSlider.tsx
- app/_hooks/useFolderShare.ts
- app/_translations/de.json
- app/(loggedInRoutes)/note/[uuid]/page.tsx
- app/_server/actions/checklist-item/sub-items.ts
- app/_components/FeatureComponents/Migration/Parts/ShareMigrationView.tsx
- app/public/checklist/[uuid]/page.tsx
- app/_hooks/useBorderRadius.ts
- app/_components/FeatureComponents/Notes/Parts/NoteEditor/NoteEditorHeader.tsx
- app/_components/GlobalComponents/Indicators/ShareBadges.tsx
- app/_types/sharing.ts
- app/_server/actions/share/target.ts
- app/_components/FeatureComponents/Sidebar/Parts/CategoryRenderer.tsx
- app/_components/FeatureComponents/Sidebar/Parts/SidebarItem.tsx
- tests/server-actions/sharing.test.ts
- app/_server/actions/checklist/readers.ts
- app/_hooks/useSharingTools.ts
- app/_server/actions/share/operations.ts
- app/_server/actions/checklist/crud.ts
- app/_server/actions/note/crud.ts
- app/_components/FeatureComponents/Checklists/ChecklistsPageClient.tsx
- app/_server/actions/checklist-item/reorder.ts
- app/_server/actions/share/access.ts
Owner
Author
|
@coderabbitai this MR is getting out of hand a little, way too many comments. Feel free to mark as resolved the ones you don't think need to be looked into anymore, be because I fixed it, be because they were a mistake. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo_owner="fccview"
repo_name="jotty"
pr_number="561"
gh api graphql \
-F owner="$repo_owner" \
-F name="$repo_name" \
-F number="$pr_number" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
id
isResolved
isOutdated
path
line
originalLine
comments(first: 50) {
nodes {
author { login }
body
createdAt
url
}
}
}
}
}
}
}' > /tmp/pr561-review-threads.json
jq -r '
.data.repository.pullRequest.reviewThreads.nodes[]
| select(.isResolved == false)
| "THREAD\t\(.id)\nOUTDATED\t\(.isOutdated)\nPATH\t\(.path // "-")\nLINE\t\(.line // .originalLine // 0)\n" +
(.comments[] | "AUTHOR\t\(.author.login)\nURL\t\(.url)\nBODY\t\(.body)\n---")
' /tmp/pr561-review-threads.jsonLength of output: 1438 🏁 Script executed: #!/bin/bash
set -euo pipefail
jq -r '
.data.repository.pullRequest.reviewThreads.nodes[]
| select(.isResolved == false)
| "THREAD\t\(.id)\nOUTDATED\t\(.isOutdated)\nPATH\t\(.path // "-")\nLINE\t\(.line // .originalLine // 0)\n" +
(.comments.nodes[] | "AUTHOR\t\(.author.login)\nURL\t\(.url)\nBODY\t\(.body)\n---")
' /tmp/pr561-review-threads.jsonLength of output: 45097 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== All unresolved review threads =="
jq -r '
.data.repository.pullRequest.reviewThreads.nodes
| to_entries[]
| select(.value.isResolved == false)
| .value as $t
| ($t.comments.nodes[-1]) as $c
| [
(.key + 1),
($t.isOutdated | tostring),
($t.path // "-"),
(($t.line // $t.originalLine // 0) | tostring),
$t.id,
($c.url // "-"),
($c.body | split("\n") | map(select(length > 0)) | .[0:3] | join(" "))
]
| `@tsv`
' /tmp/pr561-review-threads.json
echo
echo "== Current source outlines =="
for file in \
'app/_components/FeatureComponents/Kanban/KanbanPageClient.tsx' \
'app/_server/actions/checklist-item/reorder.ts' \
'app/(loggedInRoutes)/note/[uuid]/page.tsx' \
'app/_server/actions/category/queries.ts' \
'app/_server/actions/checklist/crud.ts' \
'app/_server/actions/note/crud.ts' \
'app/_server/actions/migration/share-migration.ts' \
'app/_server/actions/share/rename.ts' \
'app/_utils/category-utils.ts' \
'app/_hooks/useBorderRadius.ts' \
'app/_hooks/useChecklist.tsx' \
'app/_server/actions/history/index.ts' \
'app/(loggedInRoutes)/checklist/[uuid]/page.tsx' \
'app/(loggedInRoutes)/checklist/[...categoryPath]/page.tsx' \
'app/(loggedInRoutes)/note/[...categoryPath]/page.tsx' \
'app/public/checklist/[...categoryPath]/page.tsx' \
'app/public/note/[...categoryPath]/page.tsx' \
'app/public/note/[uuid]/page.tsx'
do
echo "--- $file ---"
ast-grep outline "$file" --view expanded 2>/dev/null || true
done
echo
echo "== Direct call-site evidence =="
rg -n -C 2 --glob '*.{ts,tsx}' \
'\b(renameGrants|legacyResolve|reorderItems|deleteList|catUuid|decodeSegment|isPinnedEntry)\s*\(' \
app tests || trueLength of output: 41794 🏁 Script executed: #!/bin/bash
set -euo pipefail
show() {
local file="$1" start="$2" end="$3"
echo "--- $file : $start-$end ---"
sed -n "${start},${end}p" "$file" | nl -ba -v "$start"
}
show 'app/_server/actions/checklist-item/reorder.ts' 1 90
show 'app/(loggedInRoutes)/note/[uuid]/page.tsx' 25 105
show 'app/_server/actions/lib/legacy-lookup.ts' 85 155
show 'app/_server/actions/category/queries.ts' 1 105
show 'app/_server/actions/checklist/crud.ts' 1 105
show 'app/_server/actions/note/crud.ts' 35 115
show 'app/_server/actions/migration/share-migration.ts' 90 135
show 'app/_server/actions/share/rename.ts' 1 135
show 'app/_server/actions/share/category-info.ts' 1 125
show 'app/_server/actions/lib/concurrency.ts' 1 100
show 'app/_hooks/useBorderRadius.ts' 1 150
show 'app/_hooks/useChecklist.tsx' 175 230
show 'app/_server/actions/history/index.ts' 340 490
show 'app/(loggedInRoutes)/checklist/[uuid]/page.tsx' 20 105
show 'app/(loggedInRoutes)/checklist/[...categoryPath]/page.tsx' 15 65
show 'app/(loggedInRoutes)/note/[...categoryPath]/page.tsx' 15 65
show 'app/public/checklist/[...categoryPath]/page.tsx' 15 60
show 'app/public/note/[...categoryPath]/page.tsx' 15 60
show 'app/public/note/[uuid]/page.tsx' 25 80
show 'app/_utils/global-utils.ts' 45 115
echo "== Auth helpers and create-action call sites =="
rg -n -C 3 --glob '*.{ts,tsx}' \
'\b(getUsername|isAdmin|createList|createNote)\s*\(' \
app testsLength of output: 1818 🏁 Script executed: #!/bin/bash
set -euo pipefail
show() {
local file="$1" start="$2" end="$3"
echo "--- $file : $start-$end ---"
sed -n "${start},${end}p" "$file" | cat -n | sed "s/^/$(printf '%s' "$start"):+/"
}
show 'app/_server/actions/checklist-item/reorder.ts' 1 90
show 'app/(loggedInRoutes)/note/[uuid]/page.tsx' 25 105
show 'app/_server/actions/lib/legacy-lookup.ts' 85 155
show 'app/_server/actions/category/queries.ts' 1 105
show 'app/_server/actions/checklist/crud.ts' 1 105
show 'app/_server/actions/note/crud.ts' 35 115
show 'app/_server/actions/migration/share-migration.ts' 90 135
show 'app/_server/actions/share/rename.ts' 1 135
show 'app/_server/actions/share/category-info.ts' 1 125
show 'app/_server/actions/lib/concurrency.ts' 1 100
show 'app/_hooks/useBorderRadius.ts' 1 150
show 'app/_hooks/useChecklist.tsx' 175 230
show 'app/_server/actions/history/index.ts' 340 490
show 'app/(loggedInRoutes)/checklist/[uuid]/page.tsx' 20 105
show 'app/(loggedInRoutes)/checklist/[...categoryPath]/page.tsx' 15 65
show 'app/(loggedInRoutes)/note/[...categoryPath]/page.tsx' 15 65
show 'app/public/checklist/[...categoryPath]/page.tsx' 15 60
show 'app/public/note/[...categoryPath]/page.tsx' 15 60
show 'app/public/note/[uuid]/page.tsx' 25 80
show 'app/_utils/global-utils.ts' 45 115
echo "== Auth helpers and create-action call sites =="
rg -n -C 3 --glob '*.{ts,tsx}' \
'\b(getUsername|isAdmin|createList|createNote)\s*\(' \
app testsLength of output: 103250 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Legacy lookup candidate selection =="
sed -n '145,235p' app/_server/actions/lib/legacy-lookup.ts | cat -n
echo "== Order migration completion =="
sed -n '125,170p' app/_server/actions/migration/share-migration.ts | cat -n
echo "== Note UUID read serialization =="
ast-grep outline app/_server/actions/note/readers.ts --view expanded
rg -n -C 4 --glob '*.ts' '\b(_stampUuid|singleFlight|runQueued)\b' \
app/_server/actions/note/readers.ts app/_server/actions/lib/concurrency.ts
echo "== Emoji preference current implementation =="
sed -n '1,80p' app/_hooks/useShowEmojis.ts | cat -n
sed -n '1,110p' app/_utils/settings-store.ts | cat -n
echo "== API create authentication boundaries =="
sed -n '1,135p' app/api/notes/route.ts | cat -n
sed -n '1,145p' app/api/checklists/route.ts | cat -n
sed -n '1,145p' app/_utils/api-utils.ts | cat -n
echo "== History call sites and owner source =="
rg -n -C 3 --glob '*.{ts,tsx}' '\b(getVersion|restoreNoteVersion|getHistory)\s*\(' app tests
echo "== Tests near current migration, history, and legacy routes =="
rg -n -C 2 --glob '*.{test,spec}.{ts,tsx}' \
'(convertOrder|legacyResolve|getVersion|createNote|createList|decodeSegment)' testsLength of output: 42310 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="fccview/jotty"
resolve_thread() {
local thread_id="$1"
gh api graphql \
-F threadId="$thread_id" \
-f query='
mutation($threadId: ID!) {
resolveReviewThread(input: {threadId: $threadId}) {
thread { id isResolved }
}
}' \
--jq '.data.resolveReviewThread.thread | "\(.id)\tresolved=\(.isResolved)"'
}
# Each thread below was inspected against the current PR source.
resolve_thread "PRRT_kwDOPb96Ls6QOWl5" # pinned-entry helper already used
resolve_thread "PRRT_kwDOPb96Ls6QOWmC" # UUID validation already present
resolve_thread "PRRT_kwDOPb96Ls6QOWmM" # username-scoped lookup already present
resolve_thread "PRRT_kwDOPb96Ls6UsDI3" # rename.ts is not a Server Action
resolve_thread "PRRT_kwDOPb96Ls6UsDJl" # catUuid is serialized with runQueued
resolve_thread "PRRT_kwDOPb96Ls6UvhlV" # hook restores saved radius on unmount
resolve_thread "PRRT_kwDOPb96Ls6Uvhla" # explicit disable preference now worksLength of output: 1354 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Current checklist-item create and update implementation =="
sed -n '1,300p' app/_server/actions/checklist-item/crud.ts | cat -n
echo "== Checklist object callers =="
rg -n -C 4 --glob '*.{ts,tsx}' \
'\b(updateItem|createItem)\s*\(' \
app tests
echo "== Resolve the outdated localization thread =="
gh api graphql \
-F threadId="PRRT_kwDOPb96Ls6UsDJG" \
-f query='
mutation($threadId: ID!) {
resolveReviewThread(input: {threadId: $threadId}) {
thread { id isResolved }
}
}' \
--jq '.data.resolveReviewThread.thread | "\(.id)\tresolved=\(.isResolved)"'Length of output: 53320
I resolved eight findings that are obsolete or invalid. Resolved findings
Verified remaining issuesThese nine items remain valid after checking the current code and direct callers.
Full prompt for an AI agentI left the nine verified items unresolved. 🐇 You are interacting with an AI system. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/api/checklists/route.ts (1)
50-62: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep
idas a fallback for checklist reads.
readListsRecursivelycan returnuuid: undefinedwhen stamping fails, andgetUserChecklistsreturns those entries.id: list.uuidwill then omit the responseidkey for unchanged checklists that do not already includeuuidmetadata. Add a fallback such asid: list.uuid ?? list.id.🤖 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 `@app/api/checklists/route.ts` around lines 50 - 62, Update the checklist mapping in getUserChecklists to preserve an identifier when list.uuid is undefined by falling back to the existing list.id value. Change only the id field in the mapped response, keeping the remaining checklist serialization unchanged.app/_server/actions/lib/metadata-cache.ts (1)
58-91: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPreserve pending-key invalidation.
dropByPrefix()bumps the stamp but leaves the in-flightPromiseinpending. A concurrentgetOrCompute()call for the same cache key then awaits and returns that pre-invalidation result, so mount lists refreshed after permission changes can still include access decisions that should have been discarded. Remove the pending entry during prefix drops or discard its result before returning.🤖 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 `@app/_server/actions/lib/metadata-cache.ts` around lines 58 - 91, Update dropByPrefix() to invalidate matching in-flight entries in pending, or otherwise prevent getOrCompute() from returning their pre-invalidation results after the stamp is bumped. Preserve the existing stamp and store invalidation behavior while ensuring concurrent calls receive a recomputed result.
🧹 Nitpick comments (3)
app/_server/actions/note/creator.ts (2)
97-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the swallowed
commitNotefailure.
commitNote(...).catch(() => { })discards any error with no logging. If the history commit fails, there is no trace to debug it later. Log a warning, matching the pattern already used elsewhere in this file for the link-index update failure (Lines 104-110).♻️ Proposed fix
if (!isEncrypted(content)) { - commitNote(target.owner, relativePath, "create", title).catch(() => { }); + commitNote(target.owner, relativePath, "create", title).catch((error) => { + console.warn("Failed to commit note history:", newDoc.id, error); + }); }🤖 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 `@app/_server/actions/note/creator.ts` around lines 97 - 99, Update the commitNote failure handler in the non-encrypted content branch to log a warning with the caught error, matching the existing link-index update failure logging pattern elsewhere in the same action. Preserve the asynchronous fire-and-forget behavior while ensuring commit failures retain useful diagnostic details.
101-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
ItemTypes.NOTEforupdateIndexForItem.
updateIndexForItemdeclaresitemType: ItemType, andItemTypeis"checklist" | "note"fromapp/_types/core.ts. ImportItemTypeshere and passItemTypes.NOTE; update the matching call innote/crud.tsas well.🤖 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 `@app/_server/actions/note/creator.ts` around lines 101 - 103, Import ItemTypes and replace the string "note" passed to updateIndexForItem in the note creator flow with ItemTypes.NOTE. Apply the same change to the matching updateIndexForItem call in note/crud.ts, preserving the existing arguments and behavior.app/_server/actions/lib/legacy-lookup.ts (1)
184-241: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winParallelize per-owner lookups in
_everyMatch.
_everyMatchawaits_findFileand_uuidForsequentially for each owner. This function is reachable from the unauthenticatedpublicResolvepath used by the public checklist/note pages. Response latency grows linearly with the number of users on every legacy public request. UsePromise.allto probe all owners concurrently.♻️ Proposed fix to parallelize owner lookups
- const uuids: string[] = []; - - for (const owner of owners) { - const filePath = await _findFile(mode, category, id, owner); - - if (!filePath) { - continue; - } - - const uuid = await _uuidFor(filePath); - - if (uuid) { - uuids.push(uuid); - } - } - - return uuids; + const resolved = await Promise.all( + owners.map(async (owner) => { + const filePath = await _findFile(mode, category, id, owner); + return filePath ? _uuidFor(filePath) : null; + }), + ); + + return resolved.filter((uuid): uuid is string => Boolean(uuid));🤖 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 `@app/_server/actions/lib/legacy-lookup.ts` around lines 184 - 241, Update _everyMatch to perform each owner’s _findFile and _uuidFor lookup concurrently with Promise.all instead of awaiting owners sequentially. Preserve the existing behavior of skipping missing files, excluding missing UUIDs, and returning the collected UUID strings for publicResolve.
🤖 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 `@app/_server/actions/checklist/creator.ts`:
- Around line 100-111: Update the catch block in the checklist creation flow to
stop reading "uuid" from formData; destructure only the available title field
and pass an empty string as the entity id to logContentEvent, matching the
pattern used by note/creator.ts.
---
Outside diff comments:
In `@app/_server/actions/lib/metadata-cache.ts`:
- Around line 58-91: Update dropByPrefix() to invalidate matching in-flight
entries in pending, or otherwise prevent getOrCompute() from returning their
pre-invalidation results after the stamp is bumped. Preserve the existing stamp
and store invalidation behavior while ensuring concurrent calls receive a
recomputed result.
In `@app/api/checklists/route.ts`:
- Around line 50-62: Update the checklist mapping in getUserChecklists to
preserve an identifier when list.uuid is undefined by falling back to the
existing list.id value. Change only the id field in the mapped response, keeping
the remaining checklist serialization unchanged.
---
Nitpick comments:
In `@app/_server/actions/lib/legacy-lookup.ts`:
- Around line 184-241: Update _everyMatch to perform each owner’s _findFile and
_uuidFor lookup concurrently with Promise.all instead of awaiting owners
sequentially. Preserve the existing behavior of skipping missing files,
excluding missing UUIDs, and returning the collected UUID strings for
publicResolve.
In `@app/_server/actions/note/creator.ts`:
- Around line 97-99: Update the commitNote failure handler in the non-encrypted
content branch to log a warning with the caught error, matching the existing
link-index update failure logging pattern elsewhere in the same action. Preserve
the asynchronous fire-and-forget behavior while ensuring commit failures retain
useful diagnostic details.
- Around line 101-103: Import ItemTypes and replace the string "note" passed to
updateIndexForItem in the note creator flow with ItemTypes.NOTE. Apply the same
change to the matching updateIndexForItem call in note/crud.ts, preserving the
existing arguments and behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 32dfa0e7-902c-449b-bc73-e2c04b40b27c
📒 Files selected for processing (44)
app/(loggedInRoutes)/checklist/[...categoryPath]/page.tsxapp/(loggedInRoutes)/checklist/[uuid]/page.tsxapp/(loggedInRoutes)/note/[...categoryPath]/page.tsxapp/(loggedInRoutes)/note/[uuid]/page.tsxapp/_hooks/lib/delete-list.tsapp/_hooks/useChecklist.tsxapp/_server/actions/category/queries.tsapp/_server/actions/checklist-item/crud.tsapp/_server/actions/checklist/creator.tsapp/_server/actions/checklist/crud.tsapp/_server/actions/history/index.tsapp/_server/actions/lib/actor.tsapp/_server/actions/lib/legacy-lookup.tsapp/_server/actions/lib/metadata-cache.tsapp/_server/actions/migration/share-migration.tsapp/_server/actions/note/creator.tsapp/_server/actions/note/crud.tsapp/_server/actions/note/readers.tsapp/_server/actions/share/mounts.tsapp/_server/actions/share/operations.tsapp/_server/actions/share/rename.tsapp/api/checklists/route.tsapp/api/kanban/route.tsapp/api/notes/route.tsapp/api/tasks/route.tsapp/public/checklist/[...categoryPath]/page.tsxapp/public/checklist/[uuid]/page.tsxapp/public/note/[...categoryPath]/page.tsxapp/public/note/[uuid]/page.tsxtests/api/checklists.test.tstests/api/notes.test.tstests/api/setup.tstests/api/tasks.test.tstests/server-actions/category.test.tstests/server-actions/checklist-item.test.tstests/server-actions/checklist.test.tstests/server-actions/history.test.tstests/server-actions/legacy-lookup.test.tstests/server-actions/note-readers.test.tstests/server-actions/note.test.tstests/server-actions/share-migration.test.tstests/utils/delete-list.test.tstests/utils/metadata-cache.test.tstests/utils/url-segment.test.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- app/_server/actions/category/queries.ts
- app/_hooks/useChecklist.tsx
- app/(loggedInRoutes)/note/[...categoryPath]/page.tsx
- app/public/checklist/[uuid]/page.tsx
- app/(loggedInRoutes)/checklist/[uuid]/page.tsx
- app/_server/actions/migration/share-migration.ts
- app/_server/actions/note/readers.ts
- app/public/note/[uuid]/page.tsx
- app/(loggedInRoutes)/note/[uuid]/page.tsx
- app/_server/actions/share/rename.ts
- app/_server/actions/note/crud.ts
- app/_server/actions/share/operations.ts
- app/api/notes/route.ts
- app/_server/actions/checklist-item/crud.ts
- app/_server/actions/checklist/crud.ts
- app/_server/actions/history/index.ts
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 by CodeRabbit
New Features
Bug Fixes
Documentation