Standardize Checklist and Note Identity on UUIDs#561
Conversation
📝 WalkthroughWalkthroughThe change standardizes checklist and note identity on UUIDs across routes, UI navigation, pinning, sharing, permissions, server actions, storage, and APIs. Canonical 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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
| onClick={handleCopyId} | ||
| className="h-6 w-6 p-0" | ||
| title={`Copy ID: ${checklist?.uuid || checklist?.id}`} | ||
| title={`Copy ID: ${checklist?.uuid}`} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
"Copy ID" tooltip can render literal "undefined".
Dropping the checklist?.id fallback means if checklist.uuid is ever unset, the title renders Copy ID: undefined instead of a sensible placeholder.
💚 Proposed fix
- title={`Copy ID: ${checklist?.uuid}`}
+ title={`Copy ID: ${checklist?.uuid || ""}`}📝 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.
| title={`Copy ID: ${checklist?.uuid}`} | |
| title={`Copy ID: ${checklist?.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/Checklists/Parts/Common/ChecklistHeader.tsx`
at 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.
| const pinnedEntries = user?.pinnedLists || []; | ||
| filtered = filtered.filter((list) => | ||
| pinnedEntries.some( | ||
| (entry) => | ||
| entry === list.uuid || entry.split("/").pop() === list.uuid, | ||
| ), | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Extract duplicated pinned-matching logic into a shared helper.
The entry === list.uuid || entry.split("/").pop() === list.uuid check is repeated 4 times in this file (and again in KanbanPageClient.tsx). ChecklistHome.tsx already exposes a canonical isListPinned from useChecklistHome. Consider extracting this into global-utils.ts (next to itemHref) and reusing it everywhere.
♻️ Proposed helper
+// app/_utils/global-utils.ts
+export const isPinnedEntry = (list: { uuid?: string }, pinnedEntries: string[] = []): boolean =>
+ pinnedEntries.some(
+ (entry) => entry === list.uuid || entry.split("/").pop() === list.uuid,
+ );- filtered = filtered.filter((list) =>
- pinnedEntries.some(
- (entry) =>
- entry === list.uuid || entry.split("/").pop() === list.uuid,
- ),
- );
+ filtered = filtered.filter((list) => isPinnedEntry(list, pinnedEntries));Also applies to: 288-292, 308-312, 328-332
🤖 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/TasksPageClient.tsx` around
lines 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.
| const pinnedEntries = user?.pinnedLists || []; | ||
| filtered = filtered.filter((list) => | ||
| pinnedEntries.some( | ||
| (entry) => | ||
| entry === list.uuid || entry.split("/").pop() === list.uuid, | ||
| ), | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Same duplicated pinned-matching logic as TasksPageClient.tsx.
Consider reusing a single shared helper (e.g. extracted into global-utils.ts) instead of duplicating entry === list.uuid || entry.split("/").pop() === list.uuid across both page clients. Also worth confirming legacy (pre-uuid) pinnedLists entries still resolve correctly, as noted for TasksPageClient.tsx.
Also applies to: 181-183
🤖 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/Kanban/KanbanPageClient.tsx` around lines
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.
| if (note.uuid) { | ||
| formData.append("uuid", note.uuid); | ||
| } | ||
| formData.append("uuid", note.uuid!); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Inconsistent uuid fallback handling.
These lines use note.uuid! (non-null assertion), while handleCopyId (line 162), the tooltip (line 285), and NoteHistoryModal (line 710) all fall back to note?.uuid || "". If note.uuid is ever unset (e.g. a legacy note that hasn't been stamped yet), this appends the literal string "undefined" to the form, and updateNote's getNoteById(uuid) lookup will silently fail to find the note.
Also applies to: 226-226
🤖 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`
at 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.
| .replace("/note/", "") | ||
| .split("/"); | ||
| linkItemId = decodeId(pathParts?.[pathParts.length - 1] || ""); | ||
| linkItemId = decodeURIComponent(pathParts?.[pathParts.length - 1] || ""); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file around the reported line.
FILE="app/_components/FeatureComponents/Notes/Parts/UnifiedMarkdownRenderer.tsx"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
echo "=== $FILE (around line 230) ==="
nl -ba "$FILE" | sed -n '200,260p'
echo
echo "=== Search for decodeId definition and usages ==="
rg -n "function decodeId|const decodeId|decodeId\(" app -S
echo
echo "=== Search for decodeURIComponent around markdown rendering ==="
rg -n "decodeURIComponent" app/_components/FeatureComponents/Notes/Parts -SRepository: fccview/jotty
Length of output: 292
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Probe how decodeURIComponent behaves on the suspected malformed input,
# and whether the current code path can receive such values.
python3 - <<'PY'
from urllib.parse import unquote
samples = [
"",
"abc",
"%20",
"%",
"%2",
"%zz",
"note%2Fid",
"note/%",
]
print("Python urllib.parse.unquote behavior (for comparison only):")
for s in samples:
try:
print(s, "=>", unquote(s))
except Exception as e:
print(s, "=> EXC", type(e).__name__, e)
print("\nJavaScript decodeURIComponent behavior:")
import subprocess, json, textwrap, os, sys
js = r'''
const samples = ["", "abc", "%20", "%", "%2", "%zz", "note%2Fid", "note/%"];
for (const s of samples) {
try {
console.log(JSON.stringify([s, decodeURIComponent(s)]));
} catch (e) {
console.log(JSON.stringify([s, "EXC", e.name, e.message]));
}
}
'''
subprocess.run(["node", "-e", js], check=True)
PYRepository: fccview/jotty
Length of output: 564
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="app/_components/FeatureComponents/Notes/Parts/UnifiedMarkdownRenderer.tsx"
echo "=== Relevant slice from $FILE ==="
sed -n '210,245p' "$FILE" | cat -n
echo
echo "=== decodeId definition/usages ==="
rg -n "decodeId" app -S
echo
echo "=== decodeURIComponent usages in the same area ==="
rg -n "decodeURIComponent" app/_components/FeatureComponents/Notes/Parts -SRepository: fccview/jotty
Length of output: 1738
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the immediate surrounding logic to see whether the input is user-controlled
# and whether any guard exists before decodeURIComponent runs.
FILE="app/_components/FeatureComponents/Notes/Parts/UnifiedMarkdownRenderer.tsx"
python3 - <<'PY'
from pathlib import Path
p = Path("app/_components/FeatureComponents/Notes/Parts/UnifiedMarkdownRenderer.tsx")
text = p.read_text()
for needle in ["decodeURIComponent", "pathParts", "linkItemId", "href"]:
print(f"\n=== {needle} context ===")
idx = text.find(needle)
if idx == -1:
print("not found")
continue
start = max(0, text.rfind("\n", 0, idx - 400))
end = min(len(text), text.find("\n", idx + 400))
print(text[start:end])
PYRepository: fccview/jotty
Length of output: 3640
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the immediate surrounding logic to see whether the input is user-controlled
# and whether any guard exists before decodeURIComponent runs.
python3 - <<'PY'
from pathlib import Path
p = Path("app/_components/FeatureComponents/Notes/Parts/UnifiedMarkdownRenderer.tsx")
text = p.read_text()
for needle in ["decodeURIComponent", "pathParts", "linkItemId", "href"]:
print(f"\n=== {needle} context ===")
idx = text.find(needle)
if idx == -1:
print("not found")
continue
start = max(0, text.rfind("\n", 0, idx - 400))
end = min(len(text), text.find("\n", idx + 400))
print(text[start:end])
PYRepository: fccview/jotty
Length of output: 3640
Guard this decodeURIComponent call Malformed internal links like /note/% will throw here and break markdown rendering. Wrap it in a safe fallback to the raw segment.
🤖 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/UnifiedMarkdownRenderer.tsx` at
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.
| const _candidates = (userDir: string, category: string, id: string): string[] => { | ||
| const paths = [ | ||
| path.join(userDir, category, `${id}.md`), | ||
| path.join(userDir, ARCHIVED_DIR_NAME, category, `${id}.md`), | ||
| ]; | ||
|
|
||
| if (category === UNCATEGORIZED) { | ||
| paths.push(path.join(userDir, `${id}.md`)); | ||
| paths.push(path.join(userDir, ARCHIVED_DIR_NAME, `${id}.md`)); | ||
| } | ||
|
|
||
| return paths; | ||
| }; | ||
|
|
||
| const _findFile = async ( | ||
| mode: Modes, | ||
| category: string, | ||
| id: string, | ||
| username: string, | ||
| ): Promise<string | null> => { | ||
| for (const candidate of _candidates(_modeDir(mode, username), category, id)) { | ||
| try { | ||
| await fs.access(candidate); | ||
| return candidate; | ||
| } catch { | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| }; | ||
|
|
||
| const _uuidFor = async (filePath: string): Promise<string | null> => { | ||
| const { grepExtractField } = await import("@/app/_utils/grep-utils"); | ||
| const existing = await grepExtractField(filePath, "uuid"); | ||
|
|
||
| if (existing) { | ||
| return existing; | ||
| } | ||
|
|
||
| const { generateUuid, updateYamlMetadata } = | ||
| await import("@/app/_utils/yaml-metadata-utils"); | ||
| const { logAudit } = await import("@/app/_server/actions/log"); | ||
|
|
||
| try { | ||
| const content = await fs.readFile(filePath, "utf-8"); | ||
| const stamped = generateUuid(); | ||
| await fs.writeFile(filePath, updateYamlMetadata(content, { uuid: stamped }), "utf-8"); | ||
| return stamped; | ||
| } catch (error) { | ||
| await logAudit({ | ||
| level: "WARNING", | ||
| action: "legacy_lookup", | ||
| category: "system", | ||
| success: false, | ||
| errorMessage: "Failed to stamp uuid during legacy lookup", | ||
| metadata: { filePath, error: String(error) }, | ||
| }); | ||
| return null; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Path traversal via unsanitized category/id in _candidates/_findFile/_uuidFor.
category and id are attacker-controlled (from URL path segments in the legacy checklist/note pages, and from the ?category= query param in app/api/checklists/[listId]/items/reorder/route.ts) and are joined into filesystem paths with no validation against ..// segments. path.join normalizes such sequences, so a crafted value can escape userDir (e.g. reach another user's checklist/notes directory, or any reachable .md file). _uuidFor then reads that file and, if it lacks a uuid: field, overwrites it via fs.writeFile to stamp a new uuid — turning this into an arbitrary-file read+write, not just a read.
This is reachable unauthenticated via app/public/checklist/[uuid]/page.tsx (which also calls legacyResolve without a username, so it iterates every user's directory), and authenticated via the reorder route's category query param.
🐛 Proposed fix
+const _isSafeSegment = (segment: string): boolean =>
+ segment.length > 0 &&
+ segment !== "." &&
+ segment !== ".." &&
+ !segment.includes("/") &&
+ !segment.includes("\\");
+
const _candidates = (userDir: string, category: string, id: string): string[] => {
+ const categorySegments = category.split("/").filter(Boolean);
+ if (!categorySegments.every(_isSafeSegment) || !_isSafeSegment(id)) {
+ return [];
+ }
+
const paths = [
- path.join(userDir, category, `${id}.md`),
- path.join(userDir, ARCHIVED_DIR_NAME, category, `${id}.md`),
+ path.join(userDir, ...categorySegments, `${id}.md`),
+ path.join(userDir, ARCHIVED_DIR_NAME, ...categorySegments, `${id}.md`),
];
if (category === UNCATEGORIZED) {
paths.push(path.join(userDir, `${id}.md`));
paths.push(path.join(userDir, ARCHIVED_DIR_NAME, `${id}.md`));
}
return paths;
};📝 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.
| const _candidates = (userDir: string, category: string, id: string): string[] => { | |
| const paths = [ | |
| path.join(userDir, category, `${id}.md`), | |
| path.join(userDir, ARCHIVED_DIR_NAME, category, `${id}.md`), | |
| ]; | |
| if (category === UNCATEGORIZED) { | |
| paths.push(path.join(userDir, `${id}.md`)); | |
| paths.push(path.join(userDir, ARCHIVED_DIR_NAME, `${id}.md`)); | |
| } | |
| return paths; | |
| }; | |
| const _findFile = async ( | |
| mode: Modes, | |
| category: string, | |
| id: string, | |
| username: string, | |
| ): Promise<string | null> => { | |
| for (const candidate of _candidates(_modeDir(mode, username), category, id)) { | |
| try { | |
| await fs.access(candidate); | |
| return candidate; | |
| } catch { | |
| continue; | |
| } | |
| } | |
| return null; | |
| }; | |
| const _uuidFor = async (filePath: string): Promise<string | null> => { | |
| const { grepExtractField } = await import("@/app/_utils/grep-utils"); | |
| const existing = await grepExtractField(filePath, "uuid"); | |
| if (existing) { | |
| return existing; | |
| } | |
| const { generateUuid, updateYamlMetadata } = | |
| await import("@/app/_utils/yaml-metadata-utils"); | |
| const { logAudit } = await import("@/app/_server/actions/log"); | |
| try { | |
| const content = await fs.readFile(filePath, "utf-8"); | |
| const stamped = generateUuid(); | |
| await fs.writeFile(filePath, updateYamlMetadata(content, { uuid: stamped }), "utf-8"); | |
| return stamped; | |
| } catch (error) { | |
| await logAudit({ | |
| level: "WARNING", | |
| action: "legacy_lookup", | |
| category: "system", | |
| success: false, | |
| errorMessage: "Failed to stamp uuid during legacy lookup", | |
| metadata: { filePath, error: String(error) }, | |
| }); | |
| return null; | |
| } | |
| }; | |
| const _isSafeSegment = (segment: string): boolean => | |
| segment.length > 0 && | |
| segment !== "." && | |
| segment !== ".." && | |
| !segment.includes("/") && | |
| !segment.includes("\\"); | |
| const _candidates = (userDir: string, category: string, id: string): string[] => { | |
| const categorySegments = category.split("/").filter(Boolean); | |
| if (!categorySegments.every(_isSafeSegment) || !_isSafeSegment(id)) { | |
| return []; | |
| } | |
| const paths = [ | |
| path.join(userDir, ...categorySegments, `${id}.md`), | |
| path.join(userDir, ARCHIVED_DIR_NAME, ...categorySegments, `${id}.md`), | |
| ]; | |
| if (category === UNCATEGORIZED) { | |
| paths.push(path.join(userDir, `${id}.md`)); | |
| paths.push(path.join(userDir, ARCHIVED_DIR_NAME, `${id}.md`)); | |
| } | |
| return paths; | |
| }; |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 63-63: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFile(filePath, "utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
[warning] 65-65: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFile(filePath, updateYamlMetadata(content, { uuid: stamped }), "utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🤖 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/lib/legacy-lookup.ts` around lines 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.
Source: Linters/SAST tools
| import { redirect, permanentRedirect } from "next/navigation"; | ||
| import { Modes } from "@/app/_types/enums"; | ||
| import type { Metadata } from "next"; | ||
| import { getMedatadaTitle } from "@/app/_server/actions/config"; | ||
| import { decodeCategoryPath, decodeId } from "@/app/_utils/global-utils"; | ||
| import { PermissionsProvider } from "@/app/_providers/PermissionsProvider"; | ||
| import { MetadataProvider } from "@/app/_providers/MetadataProvider"; | ||
| import { sanitizeUserForClient } from "@/app/_utils/user-sanitize-utils"; | ||
| import { legacyResolve } from "@/app/_server/lib/legacy-lookup"; | ||
| import { decodeCategoryPath } from "@/app/_utils/global-utils"; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file and the legacy lookup implementation, plus related API route.
git ls-files 'app/(loggedInRoutes)/checklist/[...categoryPath]/page.tsx' \
'app/_server/lib/legacy-lookup.ts' \
'app/api/checklists/[listId]/items/reorder/route.ts' \
'app/_server/actions/users.ts' \
'app/_server/lib/*' \
'app/api/checklists/*/items/reorder/route.ts' | sed 's#^`#-` #'
echo '--- page.tsx ---'
cat -n 'app/(loggedInRoutes)/checklist/[...categoryPath]/page.tsx' | sed -n '1,220p'
echo '--- legacy-lookup.ts ---'
cat -n 'app/_server/lib/legacy-lookup.ts' | sed -n '1,260p'
echo '--- reorder route ---'
cat -n 'app/api/checklists/[listId]/items/reorder/route.ts' | sed -n '1,260p'
echo '--- users actions ---'
cat -n 'app/_server/actions/users.ts' | sed -n '1,260p'Repository: fccview/jotty
Length of output: 13203
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Focused inspection of the relevant functions and call sites.
grep -nE 'legacyResolve|resolveApiId|getCurrentUser|username|redirect|permanentRedirect' \
'app/(loggedInRoutes)/checklist/[...categoryPath]/page.tsx' \
'app/_server/lib/legacy-lookup.ts' \
'app/api/checklists/[listId]/items/reorder/route.ts' \
'app/_server/actions/users.ts' || trueRepository: fccview/jotty
Length of output: 2476
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate checklist canonical page and any ownership checks around it.
fd -a 'page.tsx' app | sed -n '1,200p'
echo '--- checklist route matches ---'
rg -n 'checklist/\[|legacyResolve|resolveApiId|getCurrentUser|owner|permission|auth|redirect\(' app -g '!**/node_modules/**' | sed -n '1,260p'Repository: fccview/jotty
Length of output: 24989
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the canonical logged-in checklist page and the surrounding layout/auth behavior.
cat -n 'app/(loggedInRoutes)/checklist/[uuid]/page.tsx' | sed -n '1,220p'
echo '--- loggedIn checklist layout ---'
cat -n 'app/(loggedInRoutes)/checklist/layout.tsx' | sed -n '1,220p'
echo '--- loggedIn root layout ---'
cat -n 'app/(loggedInRoutes)/layout.tsx' | sed -n '1,220p'
echo '--- public legacy redirect page for comparison ---'
cat -n 'app/public/checklist/[...categoryPath]/page.tsx' | sed -n '1,120p'Repository: fccview/jotty
Length of output: 3663
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether the current route is already username-scoped elsewhere in the file tree.
rg -n 'legacyResolve\(Modes\.CHECKLISTS, category, id(,| )|legacyResolve\(Modes\.CHECKLISTS, category, id,|resolveApiId\(Modes\.CHECKLISTS' app/(loggedInRoutes) app/public app/api -g '!**/node_modules/**' | sed -n '1,200p'Repository: fccview/jotty
Length of output: 204
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the route-group layout and any auth gating for the logged-in routes.
cat -n 'app/(loggedInRoutes)/layout.tsx' | sed -n '1,220p'
echo '--- checklist route tree ---'
find 'app/(loggedInRoutes)/checklist' -maxdepth 2 -type f | sortRepository: fccview/jotty
Length of output: 217
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the public legacy redirect page to compare behavior and scope.
cat -n 'app/public/checklist/[...categoryPath]/page.tsx' | sed -n '1,220p'Repository: fccview/jotty
Length of output: 1202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether the current user is loaded in the route-group layout.
rg -n 'getCurrentUser|redirect\("/auth/login"|canAccessAllContent|auth' 'app/(loggedInRoutes)' -g '!**/node_modules/**' | sed -n '1,200p'Repository: fccview/jotty
Length of output: 4216
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n 'app/(loggedInRoutes)/layout.tsx' | sed -n '1,220p'
echo '--- public checklist legacy page ---'
cat -n 'app/public/checklist/[...categoryPath]/page.tsx' | sed -n '1,220p'
echo '--- logged-in checklist route tree ---'
find 'app/(loggedInRoutes)/checklist' -maxdepth 2 -type f | sortRepository: fccview/jotty
Length of output: 217
Scope legacy resolution to the current user.
legacyResolve(...) scans every user's directory here, so duplicate legacy slugs can resolve to the wrong checklist UUID. Pass the signed-in username to keep the redirect within the current account.
🤖 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/`(loggedInRoutes)/checklist/[...categoryPath]/page.tsx around lines 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.
| const userRecord = await getCurrentUser(); | ||
| const username = userRecord?.username || ""; | ||
| const hasContentAccess = await canAccessAllContent(); | ||
|
|
||
| const categoriesResult = await getCategories(Modes.CHECKLISTS); | ||
|
|
||
| let checklist = await getListById(uuid, username); | ||
|
|
||
| if (!checklist && hasContentAccess) { | ||
| checklist = await getListById(uuid); | ||
| } | ||
|
|
||
| if (!checklist) { | ||
| redirect("/"); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether an edge/proxy middleware enforces auth for (loggedInRoutes)
fd -e ts -e tsx 'middleware|proxy' app -x cat {}Repository: fccview/jotty
Length of output: 151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Relevant files =="
git ls-files 'app/(loggedInRoutes)/checklist/[uuid]/page.tsx' 'app/**/note/[uuid]/page.tsx' 'app/**/middleware.ts' 'app/**/proxy.ts' 'app/**/middleware.js' 'app/**/proxy.js' 'lib/**' 'src/**' | sed -n '1,200p'
echo
echo "== Outline checklist page =="
ast-grep outline 'app/(loggedInRoutes)/checklist/[uuid]/page.tsx' --view expanded || true
echo
echo "== Outline admin note page =="
ast-grep outline 'app/**/note/[uuid]/page.tsx' --view expanded || true
echo
echo "== Search for getListById and getCurrentUser =="
rg -n "function getListById|getListById\\(|function getCurrentUser|getCurrentUser\\(" app lib src --glob '!**/node_modules/**' -A 20 -B 5Repository: fccview/jotty
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find auth middleware/proxy files =="
fd -a -e ts -e tsx -e js -e jsx 'middleware|proxy' app . 2>/dev/null | sed -n '1,200p'
echo
echo "== Locate getListById definition =="
rg -n "getListById" . --glob '!**/node_modules/**' -A 20 -B 10
echo
echo "== Locate getCurrentUser definition =="
rg -n "getCurrentUser" . --glob '!**/node_modules/**' -A 20 -B 10Repository: fccview/jotty
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== checklist page =="
cat -n 'app/(loggedInRoutes)/checklist/[uuid]/page.tsx' | sed -n '1,220p'
echo
echo "== getListById implementation =="
rg -n "export const getListById|const getListById" 'app/_server/actions/checklist' -A 120 -B 20
echo
echo "== getCurrentUser implementation =="
rg -n "export const getCurrentUser|const getCurrentUser" 'app/_server/actions/users' -A 120 -B 20
echo
echo "== redirectGuards usage =="
rg -n "redirectGuards\\(" app -A 10 -B 10Repository: fccview/jotty
Length of output: 18132
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Root auth/middleware files =="
git ls-files | rg '(^|/)(middleware|proxy)\.(ts|tsx|js|jsx)$|(^|/)layout\.(ts|tsx|js|jsx)$' | sed -n '1,200p'
echo
echo "== redirectGuards references =="
rg -n "redirectGuards" . --glob '!**/node_modules/**' -A 8 -B 8
echo
echo "== Root middleware/proxy contents =="
for f in $(git ls-files | rg '(^|/)(middleware|proxy)\.(ts|tsx|js|jsx)$'); do
echo "--- $f ---"
cat -n "$f" | sed -n '1,220p'
doneRepository: fccview/jotty
Length of output: 13563
Fail closed before the checklist lookup
getListById(uuid, "") drops into the unscoped UUID path, so this page should redirect as soon as getCurrentUser() is null instead of relying on the later admin fallback check.
🤖 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/`(loggedInRoutes)/checklist/[uuid]/page.tsx around lines 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.
| export default async function NotePage(props: NotePageProps) { | ||
| const params = await props.params; | ||
| const { uuid } = params; | ||
|
|
||
| if (!isUuid(uuid)) { | ||
| const { legacyResolve } = await import("@/app/_server/lib/legacy-lookup"); | ||
| const resolved = await legacyResolve( | ||
| Modes.NOTES, | ||
| UNCATEGORIZED, | ||
| decodeURIComponent(uuid), | ||
| ); | ||
|
|
||
| if (resolved) { | ||
| permanentRedirect(`/note/${resolved}`); | ||
| } | ||
|
|
||
| redirect("/"); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
legacyResolve runs before the session user is known, causing an unscoped global search.
legacyResolve is invoked without a username and before getCurrentUser() is called, so it searches across every user's files (per legacy-lookup.ts's owners fallback) and returns the first owner whose category+id matches. For a legacy note where the current user's own file happens to collide (same category/id) with another user's file, this can resolve — and permanently redirect — to a different user's uuid. The getNoteById calls just below (lines 60-64) correctly use a scoped-first, admin-fallback pattern; the legacy resolution block should follow the same pattern.
🔧 Suggested fix: scope the legacy lookup to the current user first
-export default async function NotePage(props: NotePageProps) {
- const params = await props.params;
- const { uuid } = params;
-
- if (!isUuid(uuid)) {
- const { legacyResolve } = await import("`@/app/_server/lib/legacy-lookup`");
- const resolved = await legacyResolve(
- Modes.NOTES,
- UNCATEGORIZED,
- decodeURIComponent(uuid),
- );
-
- if (resolved) {
- permanentRedirect(`/note/${resolved}`);
- }
-
- redirect("/");
- }
-
- const user = await getCurrentUser();
- const username = user?.username || "";
- const hasContentAccess = await canAccessAllContent();
+export default async function NotePage(props: NotePageProps) {
+ const params = await props.params;
+ const { uuid } = params;
+
+ const user = await getCurrentUser();
+ const username = user?.username || "";
+ const hasContentAccess = await canAccessAllContent();
+
+ if (!isUuid(uuid)) {
+ const { legacyResolve } = await import("`@/app/_server/lib/legacy-lookup`");
+ let resolved = await legacyResolve(
+ Modes.NOTES,
+ UNCATEGORIZED,
+ decodeURIComponent(uuid),
+ username,
+ );
+
+ if (!resolved && hasContentAccess) {
+ resolved = await legacyResolve(
+ Modes.NOTES,
+ UNCATEGORIZED,
+ decodeURIComponent(uuid),
+ );
+ }
+
+ if (resolved) {
+ permanentRedirect(`/note/${resolved}`);
+ }
+
+ redirect("/");
+ }📝 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.
| export default async function NotePage(props: NotePageProps) { | |
| const params = await props.params; | |
| const { uuid } = params; | |
| if (!isUuid(uuid)) { | |
| const { legacyResolve } = await import("@/app/_server/lib/legacy-lookup"); | |
| const resolved = await legacyResolve( | |
| Modes.NOTES, | |
| UNCATEGORIZED, | |
| decodeURIComponent(uuid), | |
| ); | |
| if (resolved) { | |
| permanentRedirect(`/note/${resolved}`); | |
| } | |
| redirect("/"); | |
| } | |
| export default async function NotePage(props: NotePageProps) { | |
| const params = await props.params; | |
| const { uuid } = params; | |
| const user = await getCurrentUser(); | |
| const username = user?.username || ""; | |
| const hasContentAccess = await canAccessAllContent(); | |
| if (!isUuid(uuid)) { | |
| const { legacyResolve } = await import("`@/app/_server/lib/legacy-lookup`"); | |
| let resolved = await legacyResolve( | |
| Modes.NOTES, | |
| UNCATEGORIZED, | |
| decodeURIComponent(uuid), | |
| username, | |
| ); | |
| if (!resolved && hasContentAccess) { | |
| resolved = await legacyResolve( | |
| Modes.NOTES, | |
| UNCATEGORIZED, | |
| decodeURIComponent(uuid), | |
| ); | |
| } | |
| if (resolved) { | |
| permanentRedirect(`/note/${resolved}`); | |
| } | |
| redirect("/"); | |
| } |
🤖 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/`(loggedInRoutes)/note/[uuid]/page.tsx around lines 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.
| const currentUser = await getCurrentUser(); | ||
| const isOwner = currentUser?.username === note.owner; | ||
|
|
||
| if (isPubliclyShared || isOwner || (isOwner && isPrintView)) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Redundant OR term — isPrintView has no effect on this condition.
isOwner || (isOwner && isPrintView) is logically equivalent to just isOwner; the isPrintView check is dead code since isOwner alone already satisfies the OR. This strongly suggests the intended gating was different — e.g., owners should only reach the public view specifically for print mode (otherwise they'd use the private /note/{uuid} editor), which would mean the standalone isOwner term shouldn't be there at all:
🐛 Possible fix (confirm intended behavior first)
- if (isPubliclyShared || isOwner || (isOwner && isPrintView)) {
+ if (isPubliclyShared || (isOwner && isPrintView)) {Or, if owners should always be able to preview regardless of print mode, simplify to:
- if (isPubliclyShared || isOwner || (isOwner && isPrintView)) {
+ if (isPubliclyShared || isOwner) {🤖 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/note/`[uuid]/page.tsx at 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.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Documentation