Conversation
Add a Zotero link picker that searches the local Zotero library via Zotero 7's Local API (http://127.0.0.1:23119) and inserts a `[Title](zotero://select/library/items/KEY)` deep link at the caret — the flow the zotero-link Obsidian plugin popularized. - Rust zotero_search command (reqwest, Zotero-Allowed-Request header, 5s timeout) - @reflect/core typed bindings + markdown link formatter with bracket escaping - ⌘K palette command and editor slash-menu row open the picker dialog - Picker targets the routed note's editor handle; Zotero-offline errors surface in-dialog
Widen the picker dialog to 1.5x and render each result's abstract excerpt (whitespace-collapsed, 200-char cap) plus its URL when the item has one. The Rust command now surfaces abstractNote and url from the Zotero item data.
…vigation Render the picker dialog through the same cmdk Command components the ⌘K palette uses, so ↑/↓ move through the candidate list, Enter picks the highlighted item, and Esc dismisses — matching the note-search flow. Client-side filtering stays off (shouldFilter=false): ranking belongs to Zotero's own search.
Reuse the ⌘K palette's overlay, sizing, and hint footer: same centered viewport position (pt-[12vh]), same max-w-4xl width and min(60vh,36rem) list height, and a keyboard hint bar (↑/↓ Navigate, ↩ Insert, esc Cancel). Escape now closes the picker from the command frame like the palette.
WalkthroughAdds local Zotero 7 search support, normalized item commands, and a desktop picker. Users can open the picker from slash menus or application commands, search Zotero items, and insert deep links into the active note. ChangesZotero integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant NotePane
participant ZoteroPicker
participant zotero_search
participant Editor
User->>NotePane: choose Zotero slash item
NotePane->>ZoteroPicker: open for note path
User->>ZoteroPicker: enter query
ZoteroPicker->>zotero_search: search local Zotero library
zotero_search-->>ZoteroPicker: return normalized items
User->>ZoteroPicker: select item
ZoteroPicker->>Editor: insert Zotero deep link
Editor-->>ZoteroPicker: refocus editor
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/desktop/src/components/zotero/zotero-picker-store.ts (1)
12-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake published picker state immutable.
useZoteroPickerreturns the module-held object. A consumer can mutatetargetPathorepochwithoutnotify(). Mark both fieldsreadonlyand keep updates insideopenZoteroPickerandcloseZoteroPicker.As per coding guidelines, “Use readonly fields for immutable data.”
🤖 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 `@apps/desktop/src/components/zotero/zotero-picker-store.ts` around lines 12 - 17, Update the ZoteroPickerState interface to mark targetPath and epoch as readonly, while retaining all state mutations exclusively within openZoteroPicker and closeZoteroPicker.Source: Coding guidelines
apps/desktop/src/components/zotero/zotero-picker.tsx (1)
56-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace anonymous component-prop object types with named interfaces.
apps/desktop/src/components/zotero/zotero-picker.tsx#L56-L62: Define and useZoteroPickerDialogProps.apps/desktop/src/components/zotero/zotero-picker.test.tsx#L44-L46: Define and useWrapperProps.As per coding guidelines, “Name React props interfaces with the component name plus
Props.”🤖 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 `@apps/desktop/src/components/zotero/zotero-picker.tsx` around lines 56 - 62, Replace the inline props object for ZoteroPickerDialog with a named ZoteroPickerDialogProps interface in apps/desktop/src/components/zotero/zotero-picker.tsx (lines 56-62), and use that interface in the component signature. Define and use the named WrapperProps interface for the test wrapper in apps/desktop/src/components/zotero/zotero-picker.test.tsx (lines 44-46).Source: Coding guidelines
🤖 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 `@apps/desktop/src-tauri/src/zotero.rs`:
- Around line 150-152: Validate Zotero item keys against
^[23456789ABCDEFGHIJKLMNPQRSTUVWXYZ]{8}$ across all affected sites: enforce it
in the Rust ZoteroItem model and packages/core/src/zotero/commands.ts
zoteroItemSchema, reject invalid keys in zoteroItemLink before Markdown
interpolation, and add a delimiter-containing malformed-key test in
commands.test.ts. Replace every ABCD1234 fixture in
packages/core/src/zotero/commands.ts and
packages/core/src/zotero/commands.test.ts with a valid key, updating the cited
ranges in commands.ts (17-20, 83-85) and commands.test.ts (65-82); apply the
Rust validation at apps/desktop/src-tauri/src/zotero.rs (150-152).
In `@apps/desktop/src/components/zotero/zotero-picker.tsx`:
- Around line 109-122: Replace the hand-built dialog wrapper around the Zotero
picker content with the existing CommandDialog primitive, preserving the overlay
close behavior and dialog accessibility label. Ensure the command content is
rendered through the primitive’s dialog content so focus is trapped while open
and restored when closed, and remove the manually assigned dialog attributes
that the primitive provides.
In `@apps/desktop/src/lib/commands/app-commands.ts`:
- Around line 316-329: Add an isTouchEditorSurface() guard in the zotero.insert
command’s run handler before context.openZoteroPicker(), returning without
opening the picker when the current surface is touch-based while preserving the
existing notePath() check and desktop behavior.
In `@apps/desktop/src/routing/app-shortcuts.ts`:
- Around line 257-263: Update CommandContext.openZoteroPicker to resolve the
path via focusedNotePathForRoute and return immediately when it is null; only
call openZoteroPicker with a valid note path, preserving the documented no-op
behavior when no note is active.
---
Nitpick comments:
In `@apps/desktop/src/components/zotero/zotero-picker-store.ts`:
- Around line 12-17: Update the ZoteroPickerState interface to mark targetPath
and epoch as readonly, while retaining all state mutations exclusively within
openZoteroPicker and closeZoteroPicker.
In `@apps/desktop/src/components/zotero/zotero-picker.tsx`:
- Around line 56-62: Replace the inline props object for ZoteroPickerDialog with
a named ZoteroPickerDialogProps interface in
apps/desktop/src/components/zotero/zotero-picker.tsx (lines 56-62), and use that
interface in the component signature. Define and use the named WrapperProps
interface for the test wrapper in
apps/desktop/src/components/zotero/zotero-picker.test.tsx (lines 44-46).
🪄 Autofix
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: 3fc33751-804e-4b9f-9e3f-3d9e57210cfb
📒 Files selected for processing (20)
apps/desktop/src-tauri/src/lib.rsapps/desktop/src-tauri/src/zotero.rsapps/desktop/src/components/command-palette/command-palette.test.tsxapps/desktop/src/components/note-pane.tsxapps/desktop/src/components/sidebar/sidebar.test.tsxapps/desktop/src/components/workspace-content.tsxapps/desktop/src/components/zotero/zotero-picker-store.tsapps/desktop/src/components/zotero/zotero-picker.test.tsxapps/desktop/src/components/zotero/zotero-picker.tsxapps/desktop/src/dev/dev-bridge.tsapps/desktop/src/editor/use-zotero-slash-item.tsapps/desktop/src/lib/attach-files.test.tsapps/desktop/src/lib/commands/app-commands.test.tsapps/desktop/src/lib/commands/app-commands.tsapps/desktop/src/lib/commands/registry.test.tsapps/desktop/src/lib/commands/types.tsapps/desktop/src/routing/app-shortcuts.tspackages/core/src/exports/ai-actions.tspackages/core/src/zotero/commands.test.tspackages/core/src/zotero/commands.ts
| ZoteroItem { | ||
| key: data.key, | ||
| title: data.title, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files '*zotero*' | sed -n '1,120p'
printf '%s\n' '--- key validation and link construction references ---'
rg -n --glob '!node_modules' --glob '!dist' --glob '!build' \
'zoteroItemSchema|zoteroItemLink|item\.key|data\.key|ZoteroItem|select/library/items|key: z\.string|item key|item-key' .
printf '%s\n' '--- Rust file outline ---'
ast-grep outline apps/desktop/src-tauri/src/zotero.rs --view compact || true
printf '%s\n' '--- Rust focused section ---'
sed -n '1,230p' apps/desktop/src-tauri/src/zotero.rs
printf '%s\n' '--- TypeScript commands ---'
sed -n '1,150p' packages/core/src/zotero/commands.ts
printf '%s\n' '--- TypeScript tests ---'
sed -n '1,150p' packages/core/src/zotero/commands.test.tsRepository: team-reflect/reflect-open
Length of output: 20150
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- IPC response validation ---'
fd -t f . packages/core/src apps/desktop/src | xargs rg -n -A35 -B10 \
'function call|const call|export .*call|safeParse|parse\(.*schema|z\.array'
printf '%s\n' '--- Markdown link consumers and parsers ---'
rg -n --glob '!node_modules' --glob '!dist' --glob '!build' \
'insertMarkdown|markdown.*parse|parse.*markdown|remark|micromark|link.*destination|zotero://' \
packages apps | sed -n '1,240p'
printf '%s\n' '--- all Zotero key paths ---'
rg -n -A12 -B8 --glob '!node_modules' --glob '!dist' --glob '!build' \
'zoteroSearch\(|zoteroItemLink\(|ZoteroItem|zotero_search' \
apps packages | sed -n '1,300p'
printf '%s\n' '--- available parser/package metadata ---'
fd -t f 'package.json' . -x sh -c 'printf "%s\n" "--- $1"; rg -n "remark|micromark|markdown|mdast|parser" "$1"' sh {} \;Repository: team-reflect/reflect-open
Length of output: 50382
🌐 Web query:
Zotero official documentation item key 8 characters allowed characters
💡 Result:
According to the official Zotero documentation for Web API write requests, Zotero item keys are 8-character strings [1]. These keys must conform to the following regular expression pattern: /[23456789ABCDEFGHIJKLMNPQRSTUVWXYZ]{8}/ [1]. In practice, this means valid item keys consist of the digits 2-9 and uppercase letters A-Z, excluding the letter "O" to maintain human readability [2]. The characters 0, 1, and O are not used in these keys [2][3]. While the Zotero server generates these keys automatically, developers may create them locally for operations such as syncing or creating parent and child items within the same API request [1].
Citations:
- 1: https://www.zotero.org/support/dev/web_api/v3/write_requests
- 2: https://forums.zotero.org/discussion/63599/generating-key-for-web-api
- 3: https://forums.zotero.org/discussion/115476/how-is-a-valid-item-key-generated
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- IPC helper candidates ---'
fd -t f -i 'invoke' packages/core/src apps/desktop/src | sed -n '1,80p'
fd -t f 'package.json' packages/core apps/desktop | sed -n '1,80p'
printf '%s\n' '--- exact IPC helper ---'
for f in $(fd -t f -i 'invoke' packages/core/src apps/desktop/src); do
case "$f" in
*invoke*.ts|*invoke*.tsx|*invoke*.rs)
printf '%s\n' "--- $f"
sed -n '1,180p' "$f"
;;
esac
done
printf '%s\n' '--- Zotero call sites, bounded ---'
rg -l --glob '!node_modules' --glob '!dist' --glob '!build' \
'zoteroSearch\(|zoteroItemLink\(|zotero_search' apps packages |
while IFS= read -r f; do
printf '%s\n' "--- $f"
rg -n -A18 -B8 'zoteroSearch\(|zoteroItemLink\(|zotero_search' "$f"
done
printf '%s\n' '--- Markdown/editor package references ---'
rg -n --glob 'package.json' --glob 'pnpm-lock.yaml' --glob '!node_modules' \
'remark|micromark|markdown-it|mdast|prosemirror|tiptap|milkdown|codemirror' .
rg -n --glob '!node_modules' --glob '!dist' --glob '!build' \
'insertMarkdown' apps packages | sed -n '1,160p'Repository: team-reflect/reflect-open
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
alphabet = set("23456789ABCDEFGHIJKLMNPQRSTUVWXYZ")
keys = [
"ABCD1234",
"ABCD)123",
"ABCD](x",
"ABCD\n[x",
"ABCD\\\\x",
"ABCD?x#",
]
for key in keys:
valid = len(key) == 8 and all(ch in alphabet for ch in key)
link = f"[Zotero item](zotero://select/library/items/{key})"
first_close = link.find(")")
print({
"key": repr(key),
"strict_zotero_key": valid,
"link": repr(link),
"first_destination_close": first_close,
"suffix_after_first_close": repr(link[first_close + 1:]) if first_close >= 0 else None,
})
PYRepository: team-reflect/reflect-open
Length of output: 1267
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- editor insertion definitions ---'
rg -l --glob '!node_modules' --glob '!dist' --glob '!build' \
'insertMarkdown' apps packages |
while IFS= read -r f; do
printf '%s\n' "--- $f"
rg -n -A25 -B15 'insertMarkdown' "$f"
done
printf '%s\n' '--- exact editor dependencies ---'
for f in $(fd -t f 'package.json' apps packages); do
if rg -q 'meowdown|remark|micromark|prosemirror' "$f"; then
printf '%s\n' "--- $f"
rg -n -A4 -B4 'meowdown|remark|micromark|prosemirror' "$f"
fi
done
printf '%s\n' '--- delimiter payload probe ---'
python3 - <<'PY'
alphabet = set("23456789ABCDEFGHIJKLMNPQRSTUVWXYZ")
keys = ["AB)[x](y", "ABCD)123", "ABCD1234", "ABCD2345"]
for key in keys:
link = f"[Zotero item](zotero://select/library/items/{key})"
print(f"key={key!r}, length={len(key)}, strict_valid={len(key) == 8 and set(key) <= alphabet}")
print(f"generated={link!r}")
PYRepository: team-reflect/reflect-open
Length of output: 35565
Reject malformed Zotero item keys before Markdown insertion.
A malformed key such as AB)[x](y can add a second Markdown link to the note.
- Validate keys against
^[23456789ABCDEFGHIJKLMNPQRSTUVWXYZ]{8}$in Rust andzoteroItemSchema. - Reject invalid keys in
zoteroItemLinkbefore interpolation. - Add a delimiter-containing malformed-key test.
- Replace
ABCD1234fixtures because1is not valid in Zotero keys.
📍 Affects 3 files
apps/desktop/src-tauri/src/zotero.rs#L150-L152(this comment)packages/core/src/zotero/commands.ts#L17-L20packages/core/src/zotero/commands.ts#L83-L85packages/core/src/zotero/commands.test.ts#L65-L82
🤖 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 `@apps/desktop/src-tauri/src/zotero.rs` around lines 150 - 152, Validate Zotero
item keys against ^[23456789ABCDEFGHIJKLMNPQRSTUVWXYZ]{8}$ across all affected
sites: enforce it in the Rust ZoteroItem model and
packages/core/src/zotero/commands.ts zoteroItemSchema, reject invalid keys in
zoteroItemLink before Markdown interpolation, and add a delimiter-containing
malformed-key test in commands.test.ts. Replace every ABCD1234 fixture in
packages/core/src/zotero/commands.ts and
packages/core/src/zotero/commands.test.ts with a valid key, updating the cited
ranges in commands.ts (17-20, 83-85) and commands.test.ts (65-82); apply the
Rust validation at apps/desktop/src-tauri/src/zotero.rs (150-152).
Source: Coding guidelines
| <div | ||
| className="fixed inset-0 z-40 flex items-start justify-center bg-black/20 pt-[12vh]" | ||
| onPointerDown={onClose} | ||
| data-testid="zotero-picker-overlay" | ||
| > | ||
| <div | ||
| role="dialog" | ||
| aria-modal="true" | ||
| aria-label="Insert Zotero item" | ||
| className="w-full max-w-4xl" | ||
| onPointerDown={(event) => { | ||
| event.stopPropagation() // clicks inside must not close | ||
| }} | ||
| > |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
file=$(fd -i '^zotero-picker\.tsx$' . | head -n 1)
printf '%s\n' "FILE=$file"
wc -l "$file"
ast-grep outline "$file"
printf '%s\n' '--- relevant source ---'
sed -n '1,180p' "$file"
printf '%s\n' '--- dialog primitives and usages ---'
fd -i . apps/desktop/src/components/ui | rg 'dialog|command'
rg -n 'Dialog(Content|Overlay|Portal|Title|Description|Trigger)?|<Dialog|Command' apps/desktop/src/components apps/desktop/src -g '*.tsx' | head -n 200Repository: team-reflect/reflect-open
Length of output: 26635
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- dialog primitive ---'
cat -n apps/desktop/src/components/ui/dialog.tsx
printf '%s\n' '--- command primitive ---'
cat -n apps/desktop/src/components/ui/command.tsx
printf '%s\n' '--- command palette overlay ---'
sed -n '120,320p' apps/desktop/src/components/command-palette/command-palette.tsx
printf '%s\n' '--- package versions ---'
rg -n '"`@base-ui/react`"|"cmdk"|"radix-ui"' package.json pnpm-lock.yaml apps/desktop/package.json
printf '%s\n' '--- picker remainder ---'
sed -n '100,230p' apps/desktop/src/components/zotero/zotero-picker.tsxRepository: team-reflect/reflect-open
Length of output: 23763
🌐 Web query:
@base-ui/react Dialog focus trap restore focus on close documentation
💡 Result:
In @base-ui/react, focus management upon closing a dialog is controlled by the finalFocus prop on the Dialog.Popup component [1][2][3]. By default, the component automatically attempts to restore focus to the trigger or the previously focused element [1][2][3]. You can customize this behavior using the finalFocus prop, which supports the following options [1][3]: - true (default): Moves focus based on the default behavior, which targets the element that triggered the dialog or the previously focused element [1][2]. - false: Prevents the component from moving focus when the dialog closes [1][2]. - RefObject: Explicitly moves focus to the referenced DOM element [1][2]. - Function: A callback function that receives the interaction type (e.g., 'mouse', 'keyboard') and returns an element to focus, true to use the default behavior, or false/undefined to perform no action [1][2][3]. If you are using the modal or 'trap-focus' modes, the dialog manages focus trapping within the component [1][4][5]. For scenarios where the default restoration logic is insufficient (such as when the original trigger is removed from the DOM or hidden), providing a specific RefObject or a custom function to finalFocus is the recommended approach to ensure predictable accessibility [1][6][7].
Citations:
- 1: https://base-ui.com/react/components/dialog
- 2: https://app.unpkg.com/@base-ui/react@1.6.0/files/dialog/popup/DialogPopup.d.ts
- 3: https://github.com/mui/base-ui/blob/d81ec002/packages/react/src/dialog/popup/DialogPopup.tsx
- 4: https://github.com/mui/base-ui/blob/d81ec002/packages/react/src/dialog/root/DialogRoot.tsx
- 5: https://github.com/mui/base-ui/blob/d81ec002/packages/react/src/dialog/root/useDialogRoot.ts
- 6: Accessibility: Focus moves to the wrong trigger when using "Open programmatically" buttons mui/base-ui#4169
- 7: [dialog][popover] Fix focus restoration when focused element is hidden with CSS mui/base-ui#3313
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- resolved Base UI version ---'
rg -n -A8 -B3 'base-ui/react@|`@base-ui/react`:' pnpm-lock.yaml | head -n 80
printf '%s\n' '--- all local dialog wrapper usage ---'
rg -n -U '<Dialog(\s|>)|<CommandDialog' apps/desktop/src -g '*.tsx'
printf '%s\n' '--- source-level accessibility probe ---'
python3 - <<'PY'
from pathlib import Path
target = Path('apps/desktop/src/components/zotero/zotero-picker.tsx').read_text()
dialog = Path('apps/desktop/src/components/ui/dialog.tsx').read_text()
command = Path('apps/desktop/src/components/ui/command.tsx').read_text()
checks = {
'target_has_dialog_role': 'role="dialog"' in target,
'target_has_modal_state': 'aria-modal="true"' in target,
'target_has_focus_trap_or_restore_api': any(token in target for token in (
'FocusTrap', 'onCloseAutoFocus', 'restoreFocus', 'trapFocus',
)),
'local_dialog_wraps_base_ui_dialog': '`@base-ui/react/dialog`' in dialog,
'local_command_dialog_uses_local_dialog': '<Dialog {...props}>' in command and '<DialogContent' in command,
}
for name, value in checks.items():
print(f'{name}={value}')
PYRepository: team-reflect/reflect-open
Length of output: 3480
Use the existing CommandDialog primitive.
The hand-built role="dialog" provides no focus trap or focus restoration. aria-modal alone does not prevent keyboard focus from reaching background controls. Wrap the cmdk content in CommandDialog or the local Dialog and DialogContent primitives.
🤖 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 `@apps/desktop/src/components/zotero/zotero-picker.tsx` around lines 109 - 122,
Replace the hand-built dialog wrapper around the Zotero picker content with the
existing CommandDialog primitive, preserving the overlay close behavior and
dialog accessibility label. Ensure the command content is rendered through the
primitive’s dialog content so focus is trapped while open and restored when
closed, and remove the manually assigned dialog attributes that the primitive
provides.
Source: Coding guidelines
| { | ||
| id: 'zotero.insert', | ||
| title: 'Insert Zotero item…', | ||
| keywords: ['zotero', 'citation', 'paper', 'reference', 'literature', 'library'], | ||
| // Searches the local Zotero library (Zotero 7's Local API) and inserts a | ||
| // `[Title](zotero://…)` deep link at the caret of the note the current | ||
| // route edits. The picker surfaces the "Zotero is not running" state. | ||
| run: (context) => { | ||
| if (context.notePath() === null) { | ||
| return | ||
| } | ||
| context.openZoteroPicker() | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C 8 'zotero\.insert|isTouchEditorSurface|touchEditor' \
apps/desktop/srcRepository: team-reflect/reflect-open
Length of output: 33892
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- command registry and palette references ---'
rg -n -C 6 'APP_COMMANDS|appCommands|usePalette|command palette|CommandPalette|commands' \
apps/desktop/src/lib/commands apps/desktop/src/components apps/desktop/src \
-g '*.ts' -g '*.tsx' | head -n 500
printf '%s\n' '--- touch-surface guards near command execution and palette rendering ---'
rg -n -C 10 'isTouchEditorSurface|isMobileSurface|executeCommand|runCommand|openPalette|palette' \
apps/desktop/src/lib/commands apps/desktop/src/components apps/desktop/src \
-g '*.ts' -g '*.tsx' | head -n 500Repository: team-reflect/reflect-open
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate files ---'
fd -t f -e ts -e tsx . apps/desktop/src | rg 'command|palette|command-palette|app-commands'
printf '%s\n' '--- app command exports and consumers ---'
rg -n -C 12 'export .*COMMAND|COMMANDS|appCommands|commandDefinitions|CommandDefinition|CommandContext' \
apps/desktop/src/lib/commands apps/desktop/src \
-g '*.ts' -g '*.tsx' | head -n 600Repository: team-reflect/reflect-open
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- palette result implementation ---'
ast-grep outline apps/desktop/src/components/command-palette/use-palette-results.ts
cat -n apps/desktop/src/components/command-palette/use-palette-results.ts
printf '%s\n' '--- command palette command rendering and execution ---'
sed -n '230,275p' apps/desktop/src/components/command-palette/command-palette.tsx
printf '%s\n' '--- palette tests touching commands or touch surfaces ---'
rg -n -C 12 'commands|zotero|touch|isTouchEditorSurface|listCommands|command:' \
apps/desktop/src/components/command-palette/*.test.tsx \
apps/desktop/src/lib/commands/*.test.tsRepository: team-reflect/reflect-open
Length of output: 37945
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- all direct listCommands/runCommand consumers ---'
rg -n -C 8 'listCommands\(|runCommand\(' apps/desktop/src -g '*.ts' -g '*.tsx'
printf '%s\n' '--- all Zotero command and picker references ---'
rg -n -C 12 'zotero\.insert|openZoteroPicker|useZoteroPicker|Zotero' \
apps/desktop/src -g '*.ts' -g '*.tsx' | head -n 500Repository: team-reflect/reflect-open
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re
results_source = Path("apps/desktop/src/components/command-palette/use-palette-results.ts").read_text()
palette_source = Path("apps/desktop/src/components/command-palette/command-palette.tsx").read_text()
entries_source = Path("apps/desktop/src/components/command-palette/entries.ts").read_text()
command_source = Path("apps/desktop/src/lib/commands/app-commands.ts").read_text()
checks = {
"palette passes all registered commands": bool(
re.search(r"commands:\s*listCommands\(\)", results_source)
),
"palette executes selected commands": bool(
re.search(r"runCommand\(command\.id,\s*context\)", palette_source)
),
"palette has no touch-surface guard": "isTouchEditorSurface" not in palette_source,
"entries has no touch-surface guard": "isTouchEditorSurface" not in entries_source,
"zotero command has no touch-surface guard": (
"isTouchEditorSurface" not in command_source[
command_source.index("id: 'zotero.insert'") :
command_source.index("id: 'zotero.insert'") + 700
]
),
}
for name, passed in checks.items():
print(f"{'PASS' if passed else 'FAIL'}: {name}")
if not all(checks.values()):
raise SystemExit(1)
PYRepository: team-reflect/reflect-open
Length of output: 379
Guard zotero.insert on touch editor surfaces.
The palette includes every registered command and runs selected commands without a touch-surface filter. Since zotero.insert only checks notePath(), touch surfaces can expose and execute this desktop-only picker. Add an isTouchEditorSurface() check before context.openZoteroPicker().
🤖 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 `@apps/desktop/src/lib/commands/app-commands.ts` around lines 316 - 329, Add an
isTouchEditorSurface() guard in the zotero.insert command’s run handler before
context.openZoteroPicker(), returning without opening the picker when the
current surface is touch-based while preserving the existing notePath() check
and desktop behavior.
| // Resolve the target note at open time — the same path {@link notePath} | ||
| // yields, so the picker inserts where the command's other note-scoped | ||
| // siblings would. | ||
| openZoteroPicker: () => | ||
| openZoteroPicker( | ||
| focusedNotePathForRoute(routeRef.current, todayIso(), focusedDailyDateRef.current), | ||
| ), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Honor the documented no-op contract for a missing note.
CommandContext.openZoteroPicker is documented as a no-op when no note is active, but this callback passes a nullable path directly to openZoteroPicker. The picker store creates picker state for a null target, so a direct context caller can open a picker without an insertion target. Return before opening when the resolved path is null.
Proposed fix
- openZoteroPicker: () =>
- openZoteroPicker(
- focusedNotePathForRoute(routeRef.current, todayIso(), focusedDailyDateRef.current),
- ),
+ openZoteroPicker: () => {
+ const targetPath = focusedNotePathForRoute(
+ routeRef.current,
+ todayIso(),
+ focusedDailyDateRef.current,
+ )
+ if (targetPath === null) {
+ return
+ }
+ openZoteroPicker(targetPath)
+ },📝 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.
| // Resolve the target note at open time — the same path {@link notePath} | |
| // yields, so the picker inserts where the command's other note-scoped | |
| // siblings would. | |
| openZoteroPicker: () => | |
| openZoteroPicker( | |
| focusedNotePathForRoute(routeRef.current, todayIso(), focusedDailyDateRef.current), | |
| ), | |
| // Resolve the target note at open time — the same path {`@link` notePath} | |
| // yields, so the picker inserts where the command's other note-scoped | |
| // siblings would. | |
| openZoteroPicker: () => { | |
| const targetPath = focusedNotePathForRoute( | |
| routeRef.current, | |
| todayIso(), | |
| focusedDailyDateRef.current, | |
| ) | |
| if (targetPath === null) { | |
| return | |
| } | |
| openZoteroPicker(targetPath) | |
| }, |
🤖 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 `@apps/desktop/src/routing/app-shortcuts.ts` around lines 257 - 263, Update
CommandContext.openZoteroPicker to resolve the path via focusedNotePathForRoute
and return immediately when it is null; only call openZoteroPicker with a valid
note path, preserving the documented no-op behavior when no note is active.
|
Hi, @ocavue . I've done a lot daily reading with reflect app and Zotero. Most researchers use Zotero to manage paper assets. I think it's a good call to integerte Zoetero by app link. And other note taking apps e.g. Obsidian have already supported this by plugin. |
|
Hi there - really appreciate your effort here, but we're not going to merge this PR. It's too niche. |
maybe you guys could offer a plugin mechanism so that small features like this won't touch main code base. |
2 participants
What
Adds a Zotero link picker that searches the local Zotero library via Zotero 7's built-in Local API (
http://127.0.0.1:23119/api/users/0/items) and inserts a[Title](zotero://select/library/items/KEY)deep link at the caret — the flow the zotero-link Obsidian plugin popularized.How it works
zotero_searchRust command (apps/desktop/src-tauri/src/zotero.rs): reqwest GET against Zotero's Local API with theZotero-Allowed-Request: trueguard header, 5s timeout,Networkerror when Zotero is closed or its local API is disabled. No credentials — anonymous localhost access.@reflect/corebindings (packages/core/src/zotero/commands.ts): Zod-validatedzoteroSearch, thezoteroItemLinkmarkdown formatter (escapes[/]/backslash in the title), an abstract excerpt helper, and a display summary. Rust owns the capability; core owns the markdown policy.zotero.insert)/menu row Zotero item (desktop only)max-w-4xl), list height (min(60vh,36rem)), and keyboard hint footer (↑/↓ Navigate, ↩ Insert, esc Cancel). Rows show title, first author + year, a 200-char abstract excerpt, and the item URL when present.zotero://links open natively: the scheme isn't blocked byopen-external-link.ts, and markdown round-trips losslessly (meowdown keeps the href verbatim).Requirements
Testing
pnpm check(typecheck + lint) ✓-D warnings✓,zoteromodule unit tests ✓@reflect/core:zotero/commands.test.ts(link escaping, summary, excerpt, schema) ✓zotero://links parse, round-trip, and are openable via the OS protocol handlerSummary by CodeRabbit
New Features
Bug Fixes
Tests