feat: session pin/bookmark feature in sidebar / 会话置顶功能#925
Conversation
- Handle JSON-stringified input in extractSubagentPrompt (SDK) - Remove debug console.log leaking user content - Add subagent path filter to synchronizeFile (watcher sync) - Add provider check to setSessionTitle history write - Pass bookmark provider in fallback navigation - Add unpinned/edit/delete actions to mobile pinned section - Fix desktop star button accessibility (div → button) - Add bookmark toggle to mobile session list - Wrap save() in try-catch for localStorage edge cases - Support whitespace in task-notification regex Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Move action buttons inline with session row instead of separate row - Make buttons always visible (remove group-hover opacity) - Match button sizes (h-5 w-5), spacing (ml-1), and styles with SidebarSessionItem - Add active:scale-95 touch feedback for mobile
📝 WalkthroughWalkthroughAdds bookmark persistence and pinned-session controls in the sidebar, updates Claude session history helpers to skip subagent artifacts and backfill custom titles, and broadens task-notification parsing in chat message normalization. ChangesBookmark persistence and sidebar controls
Claude session history handling
Task notification parsing
Sequence Diagram(s)sequenceDiagram
participant SidebarSessionItem
participant Sidebar
participant useBookmarks
participant localStorage
participant SidebarContent
participant SidebarBookmarks
SidebarSessionItem->>Sidebar: onToggleBookmark(bookmark)
Sidebar->>useBookmarks: toggleBookmark(bookmark)
useBookmarks->>localStorage: save(bookmarks)
localStorage-->>useBookmarks: storage event
useBookmarks-->>Sidebar: bookmarks state update
Sidebar->>SidebarContent: pass bookmarks and handlers
SidebarContent->>SidebarBookmarks: render pinned sessions
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.0)server/modules/providers/services/sessions.service.tsFile contains syntax errors that prevent linting: Line 147: Expected a statement but instead found '* Application service'.; Line 147: expected ... [truncated 595 characters] ... micolon to end the class property, but found none; Line 150: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '; Line 150: expected a semicolon to end the class property, but found none; Line 150: expected a semicolon to end the class property, but found none; Line 150: expected a semicolon to end the class property, but found none; Line 150: expected a semicolon to end the class property, but found none; Line 150: expected a semicolon to end the class property, but found none; Line 151: Expected a class parameters but instead found 'layout'.; Line 151: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '; Line 382: expected 🔧 ESLint
server/modules/providers/services/sessions.service.tsParsing error: Expression expected. 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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/sidebar/view/subcomponents/SidebarProjectItem.tsx (1)
267-302: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAvoid nesting the star button inside the row button.
The outer
Buttonis the row click target, so the new star<button>creates button-inside-button markup. That is invalid HTML and can break keyboard/focus behavior in some browsers. Render the row container as a non-button when it contains child actions, or move the star control outside the clickable row.🤖 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 `@src/components/sidebar/view/subcomponents/SidebarProjectItem.tsx` around lines 267 - 302, The project row in SidebarProjectItem currently nests the star action button inside the outer Button used for row selection, which creates invalid button-inside-button markup. Update the SidebarProjectItem row so the selectAndToggleProject container is not a button when it includes the inner star control, or move the star toggle control outside the clickable row while preserving toggleStarProject and the existing selection behavior.
🧹 Nitpick comments (2)
src/components/sidebar/view/subcomponents/SidebarContent.tsx (1)
157-157: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
onSelectBookmarkedSessiontype drops theproviderargument.The prop is typed as
(projectId, sessionId) => void, but it's forwarded toSidebarBookmarks.onSelectSession(typed(projectId, sessionId, provider) => void) and the Sidebar implementation actually consumesprovider. It compiles due to parameter bivariance, but the type understates the real contract. Aligning it avoids confusion.♻️ Suggested type
- onSelectBookmarkedSession: (projectId: string, sessionId: string) => void; + onSelectBookmarkedSession: (projectId: string, sessionId: string, provider: string) => void;🤖 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 `@src/components/sidebar/view/subcomponents/SidebarContent.tsx` at line 157, The `onSelectBookmarkedSession` prop type in `SidebarContent` understates the actual callback contract by omitting `provider`, while it is forwarded to `SidebarBookmarks.onSelectSession` and consumed by the sidebar implementation. Update the `SidebarContent` prop declaration and any related typings so `onSelectBookmarkedSession` accepts `(projectId, sessionId, provider) => void`, keeping it aligned with `SidebarBookmarks` and the downstream selection handler.src/components/sidebar/view/subcomponents/SidebarBookmarks.tsx (1)
170-178: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueEditing is unreachable from the mobile pinned row.
The mobile non-editing branch renders only Unpin and Delete — there's no Edit trigger, yet the editing UI (input at Lines 155-162) exists. Editing can only surface on mobile if
editingSessionis set elsewhere. If inline rename is intended on mobile here, add an edit button matching the desktop variant.🤖 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 `@src/components/sidebar/view/subcomponents/SidebarBookmarks.tsx` around lines 170 - 178, The mobile pinned-row branch in SidebarBookmarks is missing the rename trigger, so the inline edit UI cannot be reached from this view. Update the non-editing mobile actions in the SidebarBookmarks component to include an Edit button consistent with the desktop variant, wired to set the editing state for the current session so the existing input UI can open on mobile.
🤖 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 `@server/claude-sdk.js`:
- Around line 52-60: The parsed tool input handling in the JSON parsing path
needs a null guard before accessing prompt, because JSON.parse can return null
and typeof null is still object. Update the logic around the tool input parsing
and the prompt extraction in the same function to return null when parsed is
null (or otherwise non-object) before reading parsed.prompt, so invalid input
does not throw.
In `@server/modules/providers/services/sessions.service.ts`:
- Around line 115-123: The `hasCustomTitle` check in `sessions.service.ts` is
treating any matching `custom-title` event as evidence the history is already
synced, which can leave `session.custom_name` stale. Update the history scan
around the `custom-title` handling so it only marks the session as synced when
the event’s title actually matches the current custom title, and use the same
logic in the later `claude -r`/session append path referenced by the
`customTitle` handling to avoid skipping a newer name.
- Around line 98-105: The startup name sync in syncAllSessionNamesToHistory is
too broad because it only checks jsonl_path and can write custom-title records
into Codex/Gemini transcripts. Restrict the session selection to Claude-only
rows either by adding a provider guard inside syncAllSessionNamesToHistory or by
filtering non-Claude sessions in getSessionsWithCustomName(), and keep the
existing custom_name/jsonl_path handling unchanged for the matched Claude
sessions.
In `@src/components/sidebar/view/Sidebar.tsx`:
- Around line 316-331: `onSelectBookmarkedSession` in `Sidebar` currently no-ops
when `projects.find(...)` չի finds a matching project, so bookmarked sessions
for stale or unloaded projects cannot open. Update this handler to mirror the
fallback in `onConversationResultClick`: if the project is missing, still invoke
`handleSessionClick` with a session stub containing the `sessionId` and
`provider` plus the original `projectId`, instead of returning early.
In `@src/components/sidebar/view/subcomponents/SidebarBookmarks.tsx`:
- Around line 136-140: The mobile bookmark row in SidebarBookmarks is a
clickable div without keyboard support, unlike the desktop button. Update the
mobile branch in SidebarBookmarks to use an accessible interactive element or
add the missing role, tabIndex, and keyboard handler so onSelect can be
triggered with keyboard input as well as click, keeping the behavior consistent
with the desktop variant.
- Around line 203-209: The SidebarBookmarks section only initializes expanded
from bookmarks.length once, so it stays collapsed when the first bookmark is
added after an empty state. Update SidebarBookmarks to detect the transition
from empty to non-empty and set expanded to true at that point, while preserving
the existing collapse() imperative handle behavior. Use useEffect plus a
previous-length ref in SidebarBookmarks to auto-expand only on the first
bookmark addition.
In `@src/components/sidebar/view/subcomponents/SidebarSessionItem.tsx`:
- Around line 198-219: The mobile session actions in SidebarSessionItem are
missing the same processing guard used by the desktop path, which allows
delete/archive during an active run. Update the non-cursor action block to also
check !isProcessing before rendering the delete button, matching the existing
desktop behavior around requestDeleteSession and keeping active-run sidebar
state consistent.
In `@src/stores/useBookmarkStore.ts`:
- Around line 36-39: The bookmark lookup and mutation methods in
useBookmarkStore are using only sessionId, which can collide across
projects/providers. Update isBookmarked, removeBookmark, and toggleBookmark to
use the full session identity consistently with BookmarkedSession and
SidebarBookmarks, including projectId and provider in the key or matching logic
so actions only affect the intended bookmark.
---
Outside diff comments:
In `@src/components/sidebar/view/subcomponents/SidebarProjectItem.tsx`:
- Around line 267-302: The project row in SidebarProjectItem currently nests the
star action button inside the outer Button used for row selection, which creates
invalid button-inside-button markup. Update the SidebarProjectItem row so the
selectAndToggleProject container is not a button when it includes the inner star
control, or move the star toggle control outside the clickable row while
preserving toggleStarProject and the existing selection behavior.
---
Nitpick comments:
In `@src/components/sidebar/view/subcomponents/SidebarBookmarks.tsx`:
- Around line 170-178: The mobile pinned-row branch in SidebarBookmarks is
missing the rename trigger, so the inline edit UI cannot be reached from this
view. Update the non-editing mobile actions in the SidebarBookmarks component to
include an Edit button consistent with the desktop variant, wired to set the
editing state for the current session so the existing input UI can open on
mobile.
In `@src/components/sidebar/view/subcomponents/SidebarContent.tsx`:
- Line 157: The `onSelectBookmarkedSession` prop type in `SidebarContent`
understates the actual callback contract by omitting `provider`, while it is
forwarded to `SidebarBookmarks.onSelectSession` and consumed by the sidebar
implementation. Update the `SidebarContent` prop declaration and any related
typings so `onSelectBookmarkedSession` accepts `(projectId, sessionId, provider)
=> void`, keeping it aligned with `SidebarBookmarks` and the downstream
selection handler.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 34d1c489-2e35-4432-bdf3-7cbad56075ea
📒 Files selected for processing (20)
server/claude-sdk.jsserver/modules/providers/list/claude/claude-session-synchronizer.provider.tsserver/modules/providers/services/sessions.service.tssrc/components/chat/hooks/useChatMessages.tssrc/components/sidebar/view/Sidebar.tsxsrc/components/sidebar/view/subcomponents/SidebarBookmarks.tsxsrc/components/sidebar/view/subcomponents/SidebarContent.tsxsrc/components/sidebar/view/subcomponents/SidebarProjectItem.tsxsrc/components/sidebar/view/subcomponents/SidebarProjectList.tsxsrc/components/sidebar/view/subcomponents/SidebarProjectSessions.tsxsrc/components/sidebar/view/subcomponents/SidebarSessionItem.tsxsrc/i18n/locales/de/sidebar.jsonsrc/i18n/locales/en/sidebar.jsonsrc/i18n/locales/it/sidebar.jsonsrc/i18n/locales/ja/sidebar.jsonsrc/i18n/locales/ko/sidebar.jsonsrc/i18n/locales/ru/sidebar.jsonsrc/i18n/locales/tr/sidebar.jsonsrc/i18n/locales/zh-CN/sidebar.jsonsrc/stores/useBookmarkStore.ts
| if (typeof toolInput === 'string') { | ||
| try { | ||
| parsed = JSON.parse(toolInput); | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
| if (typeof parsed !== 'object') return null; | ||
| const prompt = typeof parsed.prompt === 'string' ? parsed.prompt : null; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard the JSON null case before reading prompt.
JSON.parse('null') leaves parsed === null; Line 59 still passes because typeof null === 'object', and Line 60 then throws instead of returning null for invalid input.
Proposed fix
- if (typeof parsed !== 'object') return null;
+ if (parsed === null || typeof parsed !== 'object') return null;📝 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.
| if (typeof toolInput === 'string') { | |
| try { | |
| parsed = JSON.parse(toolInput); | |
| } catch { | |
| return null; | |
| } | |
| } | |
| if (typeof parsed !== 'object') return null; | |
| const prompt = typeof parsed.prompt === 'string' ? parsed.prompt : null; | |
| if (typeof toolInput === 'string') { | |
| try { | |
| parsed = JSON.parse(toolInput); | |
| } catch { | |
| return null; | |
| } | |
| } | |
| if (parsed === null || typeof parsed !== 'object') return null; | |
| const prompt = typeof parsed.prompt === 'string' ? parsed.prompt : null; |
🤖 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 `@server/claude-sdk.js` around lines 52 - 60, The parsed tool input handling in
the JSON parsing path needs a null guard before accessing prompt, because
JSON.parse can return null and typeof null is still object. Update the logic
around the tool input parsing and the prompt extraction in the same function to
return null when parsed is null (or otherwise non-object) before reading
parsed.prompt, so invalid input does not throw.
| async function syncAllSessionNamesToHistory(): Promise<void> { | ||
| const sessions = sessionsDb.getSessionsWithCustomName(); | ||
| if (sessions.length === 0) return; | ||
|
|
||
| let synced = 0; | ||
| for (const session of sessions) { | ||
| if (!session.jsonl_path) continue; | ||
| try { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether getSessionsWithCustomName() already filters to Claude sessions
# and whether non-Claude sessions can carry jsonl_path.
rg -n -C3 '\bgetSessionsWithCustomName\s*\(' server
rg -n -C3 '\bjsonl_path\b|\bprovider\b|LLMProvider\.CLAUDE' serverRepository: siteboon/claudecodeui
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the sessions service and repository methods involved in the concern.
rg -n -C3 'getSessionsWithCustomName|syncAllSessionNamesToHistory|setSessionTitle|jsonl_path|provider === LLMProvider\.CLAUDE|provider === '"'"'claude'"'"'' server/modules/providers server/modules -g '!**/dist/**' -g '!**/build/**'
# Narrowly inspect the sessions database implementation.
fd -a 'sessions.*' server | sed -n '1,40p'
fd -a '*session*db*' server | sed -n '1,40p'Repository: siteboon/claudecodeui
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the specific service file around the synced startup path.
FILE='server/modules/providers/services/sessions.service.ts'
wc -l "$FILE"
cat -n "$FILE" | sed -n '1,180p'Repository: siteboon/claudecodeui
Length of output: 6712
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='server/modules/database/repositories/sessions.db.ts'
wc -l "$FILE"
cat -n "$FILE" | sed -n '1,240p'
echo '---'
rg -n -C4 'getSessionsWithCustomName|getSessionById|getAllSessions|provider\s*=' "$FILE"Repository: siteboon/claudecodeui
Length of output: 12237
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the startup caller(s) for the sync path.
rg -n -C4 'syncAllSessionNamesToHistory\(' server/modules/providers server/modules -g '!**/dist/**' -g '!**/build/**'
# Show session insertion paths for non-Claude providers that may still set jsonl_path and custom_name.
rg -n -C3 'jsonl_path = NULL|jsonl_path = excluded.jsonl_path|custom_name|provider TEXT NOT NULL DEFAULT '"'"'claude'"'"'' server/modules/database/repositories/sessions.db.ts server/modules/providers/list -g '!**/dist/**' -g '!**/build/**'Repository: siteboon/claudecodeui
Length of output: 12552
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='server/modules/database/repositories/sessions.db.ts'
# Show the custom-name query implementation.
sed -n '297,420p' "$FILE" | cat -n
echo '---'
# Show every caller of the startup sync and the surrounding context.
rg -n -C4 'syncAllSessionNamesToHistory\(' server/modules -g '!**/dist/**' -g '!**/build/**'Repository: siteboon/claudecodeui
Length of output: 5695
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the provider-specific session synchronizers that can populate custom_name/jsonl_path.
rg -n -C3 'createSession\(|updateSessionCustomName\(|jsonl_path =|custom_name =' server/modules/providers/list/{claude,codex,gemini,opencode} -g '!**/dist/**' -g '!**/build/**'Repository: siteboon/claudecodeui
Length of output: 6391
Scope the startup name sync to Claude sessions server/modules/providers/services/sessions.service.ts:98-135 — the loop only checks jsonl_path, while Codex and Gemini rows also store custom_name and jsonl_path, so this can append Claude custom-title records into non-Claude transcripts. Add a Claude provider guard or filter in getSessionsWithCustomName().
🤖 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 `@server/modules/providers/services/sessions.service.ts` around lines 98 - 105,
The startup name sync in syncAllSessionNamesToHistory is too broad because it
only checks jsonl_path and can write custom-title records into Codex/Gemini
transcripts. Restrict the session selection to Claude-only rows either by adding
a provider guard inside syncAllSessionNamesToHistory or by filtering non-Claude
sessions in getSessionsWithCustomName(), and keep the existing
custom_name/jsonl_path handling unchanged for the matched Claude sessions.
| if ( | ||
| typeof parsed === 'object' && | ||
| parsed !== null && | ||
| (parsed as Record<string, unknown>).type === 'custom-title' && | ||
| (parsed as Record<string, unknown>).sessionId === session.session_id | ||
| ) { | ||
| hasCustomTitle = true; | ||
| break; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Match customTitle before treating the history as synced.
This loop stops on any custom-title event for the session. If the JSONL already contains an older title, startup will skip appending the current session.custom_name, and claude -r can keep showing the stale name.
Proposed fix
try {
const parsed = JSON.parse(trimmed);
- if (
- typeof parsed === 'object' &&
- parsed !== null &&
- (parsed as Record<string, unknown>).type === 'custom-title' &&
- (parsed as Record<string, unknown>).sessionId === session.session_id
- ) {
- hasCustomTitle = true;
- break;
+ if (typeof parsed === 'object' && parsed !== null) {
+ const record = parsed as Record<string, unknown>;
+ if (
+ record.type === 'custom-title' &&
+ record.sessionId === session.session_id &&
+ record.customTitle === session.custom_name
+ ) {
+ hasCustomTitle = true;
+ break;
+ }
}
} catch {Also applies to: 129-135
🤖 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 `@server/modules/providers/services/sessions.service.ts` around lines 115 -
123, The `hasCustomTitle` check in `sessions.service.ts` is treating any
matching `custom-title` event as evidence the history is already synced, which
can leave `session.custom_name` stale. Update the history scan around the
`custom-title` handling so it only marks the session as synced when the event’s
title actually matches the current custom title, and use the same logic in the
later `claude -r`/session append path referenced by the `customTitle` handling
to avoid skipping a newer name.
| onSelectBookmarkedSession={(projectId, sessionId, provider) => { | ||
| const project = projects.find(p => p.projectId === projectId); | ||
| if (project) { | ||
| handleProjectSelect(project); | ||
| const sessions = getProjectSessions(project); | ||
| const existing = sessions.find(s => s.id === sessionId); | ||
| if (existing) { | ||
| handleSessionClick(existing, project.projectId); | ||
| } else { | ||
| handleSessionClick( | ||
| { id: sessionId, __provider: provider || 'claude' }, | ||
| project.projectId, | ||
| ); | ||
| } | ||
| } | ||
| }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts -C4 '\bhandleSessionClick\s*[:=]' src/components/sidebar
rg -nP --type=ts -C2 'handleSessionClick\(' src/components/sidebarRepository: siteboon/claudecodeui
Length of output: 2542
🏁 Script executed:
#!/bin/bash
sed -n '443,470p' src/components/sidebar/hooks/useSidebarController.ts
printf '\n---\n'
sed -n '270,335p' src/components/sidebar/view/Sidebar.tsxRepository: siteboon/claudecodeui
Length of output: 4652
🏁 Script executed:
#!/bin/bash
sed -n '443,470p' src/components/sidebar/hooks/useSidebarController.ts
printf '\n---\n'
sed -n '270,335p' src/components/sidebar/view/Sidebar.tsxRepository: siteboon/claudecodeui
Length of output: 4652
Handle missing-project bookmarks instead of no-op.
onSelectBookmarkedSession returns early when projects.find(...) misses, so stale bookmarks or unloaded projects can’t be opened. Add the same fallback used by onConversationResultClick: call handleSessionClick with the session stub and projectId even when the project isn’t present.
🤖 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 `@src/components/sidebar/view/Sidebar.tsx` around lines 316 - 331,
`onSelectBookmarkedSession` in `Sidebar` currently no-ops when
`projects.find(...)` չի finds a matching project, so bookmarked sessions for
stale or unloaded projects cannot open. Update this handler to mirror the
fallback in `onConversationResultClick`: if the project is missing, still invoke
`handleSessionClick` with a session stub containing the `sessionId` and
`provider` plus the original `projectId`, instead of returning early.
| <div className="md:hidden"> | ||
| <div | ||
| className={cn('p-2 mx-3 my-0.5 rounded-md bg-card border active:scale-[0.98] transition-all duration-150 relative', isSelected ? 'bg-primary/5 border-primary/20' : 'border-border/30')} | ||
| onClick={onSelect} | ||
| > |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Mobile row isn't keyboard-accessible.
The mobile container is a <div onClick={onSelect}> with no role/tabIndex/key handler, so the pinned session can't be selected via keyboard (the desktop variant correctly uses a <button>). This mirrors a pattern that blocks keyboard users.
🤖 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 `@src/components/sidebar/view/subcomponents/SidebarBookmarks.tsx` around lines
136 - 140, The mobile bookmark row in SidebarBookmarks is a clickable div
without keyboard support, unlike the desktop button. Update the mobile branch in
SidebarBookmarks to use an accessible interactive element or add the missing
role, tabIndex, and keyboard handler so onSelect can be triggered with keyboard
input as well as click, keeping the behavior consistent with the desktop
variant.
| const [expanded, setExpanded] = useState(bookmarks.length > 0); | ||
|
|
||
| useImperativeHandle(ref, () => ({ | ||
| collapse: () => setExpanded(false), | ||
| })); | ||
|
|
||
| if (!bookmarks.length) return null; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Pinned section stays collapsed when the first bookmark is added.
expanded is initialized from bookmarks.length > 0 only on mount. This component is always rendered (it just returns null while empty at Line 209), so the initializer typically runs when bookmarks.length === 0, leaving expanded === false. When the user pins their first session the section appears collapsed, hiding the very item they just pinned until they expand it manually. Consider expanding when transitioning from empty to non-empty.
🐛 Auto-expand on first bookmark
const [expanded, setExpanded] = useState(bookmarks.length > 0);
+ const prevCount = useRef(bookmarks.length);
+ useEffect(() => {
+ if (prevCount.current === 0 && bookmarks.length > 0) setExpanded(true);
+ prevCount.current = bookmarks.length;
+ }, [bookmarks.length]);(requires adding useEffect, useRef to the React import)
📝 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 [expanded, setExpanded] = useState(bookmarks.length > 0); | |
| useImperativeHandle(ref, () => ({ | |
| collapse: () => setExpanded(false), | |
| })); | |
| if (!bookmarks.length) return null; | |
| const [expanded, setExpanded] = useState(bookmarks.length > 0); | |
| const prevCount = useRef(bookmarks.length); | |
| useEffect(() => { | |
| if (prevCount.current === 0 && bookmarks.length > 0) setExpanded(true); | |
| prevCount.current = bookmarks.length; | |
| }, [bookmarks.length]); | |
| useImperativeHandle(ref, () => ({ | |
| collapse: () => setExpanded(false), | |
| })); | |
| if (!bookmarks.length) return null; |
🤖 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 `@src/components/sidebar/view/subcomponents/SidebarBookmarks.tsx` around lines
203 - 209, The SidebarBookmarks section only initializes expanded from
bookmarks.length once, so it stays collapsed when the first bookmark is added
after an empty state. Update SidebarBookmarks to detect the transition from
empty to non-empty and set expanded to true at that point, while preserving the
existing collapse() imperative handle behavior. Use useEffect plus a
previous-length ref in SidebarBookmarks to auto-expand only on the first
bookmark addition.
| {!sessionView.isCursorSession && ( | ||
| <> | ||
| <button | ||
| className="ml-1 flex h-5 w-5 items-center justify-center rounded-md opacity-70 transition-transform active:scale-95 bg-blue-50 dark:bg-blue-900/20" | ||
| onClick={(event) => { | ||
| event.stopPropagation(); | ||
| requestToggleBookmark(); | ||
| }} | ||
| title={isBookmarked ? t('bookmarks.unpin', 'Unpin session') : t('bookmarks.pin', 'Pin session')} | ||
| > | ||
| <Pin className={cn('h-2.5 w-2.5', isBookmarked && 'fill-red-500 text-red-500 rotate-45', !isBookmarked && 'text-blue-600 dark:text-blue-400')} /> | ||
| </button> | ||
| <button | ||
| className="ml-1 flex h-5 w-5 items-center justify-center rounded-md bg-red-50 opacity-70 transition-transform active:scale-95 dark:bg-red-900/20" | ||
| onClick={(event) => { | ||
| event.stopPropagation(); | ||
| requestDeleteSession(); | ||
| }} | ||
| > | ||
| <Trash2 className="h-2.5 w-2.5 text-red-600 dark:text-red-400" /> | ||
| </button> | ||
| </> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Keep the mobile delete action gated by !isProcessing.
This block now shows delete for every non-cursor session, while the desktop path still hides it for active runs. That re-enables archive/delete on mobile during processing and can leave the active-run/sidebar state inconsistent.
🤖 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 `@src/components/sidebar/view/subcomponents/SidebarSessionItem.tsx` around
lines 198 - 219, The mobile session actions in SidebarSessionItem are missing
the same processing guard used by the desktop path, which allows delete/archive
during an active run. Update the non-cursor action block to also check
!isProcessing before rendering the delete button, matching the existing desktop
behavior around requestDeleteSession and keeping active-run sidebar state
consistent.
| isBookmarked: (sessionId: string) => boolean; | ||
| bookmarkSession: (session: BookmarkedSession) => void; | ||
| removeBookmark: (sessionId: string) => void; | ||
| toggleBookmark: (session: BookmarkedSession) => void; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use the full session identity for bookmark lookups and removal.
isBookmarked, removeBookmark, and toggleBookmark only key on sessionId, but the stored model and SidebarBookmarks actions rely on projectId and provider too. If two projects/providers reuse the same session id, pinning or unpinning one will affect the other. Use a composite key like { projectId, sessionId, provider } consistently here.
Also applies to: 50-83
🤖 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 `@src/stores/useBookmarkStore.ts` around lines 36 - 39, The bookmark lookup and
mutation methods in useBookmarkStore are using only sessionId, which can collide
across projects/providers. Update isBookmarked, removeBookmark, and
toggleBookmark to use the full session identity consistently with
BookmarkedSession and SidebarBookmarks, including projectId and provider in the
key or matching logic so actions only affect the intended bookmark.
2 participants
Session Pin/Bookmark Feature / 会话置顶功能
What this does
Adds session-level pin/bookmark to the sidebar — different from the existing project star feature:
projects.isStarred)localStorage(cross-tab sync)Example: In a coding project with 50 sessions, pin the 3 most important ones (architecture decision, API spec, deployment config) without starring the whole project.
Changes
extractSubagentPromptinclaude-sdk.js— parse Task tool input for subagent promptsClaudeSessionSynchronizersetSessionTitle/syncAllSessionNamesToHistoryinsessions.service.tsuseBookmarkStore— localStorage-backed bookmark hook with cross-tab syncSidebarBookmarks— pinned-sessions panel in sidebarSidebarSessionItem— pin button on mobile and desktop@blackmammoth — The project star feature is complementary to this, not a replacement. Happy to rename or adjust if needed.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation