feat: add prompt manager and prompt library integration #555
Draft
itsmepicus wants to merge 21 commits intositeboon:mainfrom
Draft
feat: add prompt manager and prompt library integration #555itsmepicus wants to merge 21 commits intositeboon:mainfrom
itsmepicus wants to merge 21 commits intositeboon:mainfrom
Conversation
Introduce a prompt manager module with role and template support in chat. Add a prompt library modal, selector controls, prompt cards, shared prompt types, and a hook for loading and applying prompts. Add backend prompt APIs for listing and loading markdown prompts. Wire the routes into the server and ship a built-in prompt catalog under shared/prompts for common engineering roles and reusable templates. Refine chat composer integration so active roles are prepended to outgoing messages. Surface prompt loading errors in the UI through the existing chat flow.
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces a comprehensive Prompt Manager feature enabling users to browse, apply, and manage role-based and template prompts within chat interactions. It includes backend API endpoints for prompt discovery and loading, new shared prompt definitions (roles and templates), TypeScript types, React hooks and components, and integration into the chat composer interface. Changes
Sequence DiagramsequenceDiagram
participant User as User<br/>(Chat UI)
participant Hook as usePrompts<br/>Hook
participant API as Express<br/>Server
participant FS as File<br/>System
participant Composer as Chat<br/>Composer
User->>Hook: openPromptLibrary()
Hook->>API: POST /api/prompts/list
API->>FS: scanDirectories(builtin, user, project)
FS-->>API: Prompt metadata[]
API-->>Hook: PromptsListResponse
Hook-->>User: Display PromptLibrary
User->>User: Select & Apply Role
User->>Hook: applyRole(prompt)
Hook->>API: POST /api/prompts/load
API->>FS: readFile(promptPath)
FS-->>API: Content + Metadata
API-->>Hook: PromptLoadResponse
Hook->>Hook: setActiveRole(name, content, icon)
Hook-->>Composer: Update activeRole state
User->>Composer: Type message
Composer->>Composer: Prepend activeRole.content
User->>Composer: Submit message
Composer-->>API: Send message with role prefix
Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (10)
shared/prompts/templates/bug-fix.md (1)
26-28: Consider adding a language identifier to the code block.The static analysis tool flagged this fenced code block as missing a language specification. Since this is a placeholder for arbitrary error messages/logs, you could use
textorplaintextas a generic identifier.🔧 Suggested fix
**Error messages:** -``` +```text [Paste any error messages or logs]</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@shared/prompts/templates/bug-fix.mdaround lines 26 - 28, The fenced code
block containing "[Paste any error messages or logs]" is missing a language
identifier; update that markdown block in the bug-fix template to use a generic
language token such as "text" or "plaintext" (i.e., change the triple-backtick
fence for that snippet to ```text) so static analysis stops flagging it and the
placeholder remains clearly marked as plain text.</details> </blockquote></details> <details> <summary>src/components/prompt-manager/hooks/usePrompts.ts (3)</summary><blockquote> `59-70`: **Clear error state before applying a new role.** If a previous operation failed, the error state persists even after a successful `applyRole` call. Consider clearing the error at the start. <details> <summary>Suggested fix</summary> ```diff const applyRole = useCallback(async (prompt: Prompt) => { + setError(null); try { const content = await loadPromptContent(prompt); setActiveRole({ name: prompt.name, content, icon: prompt.icon }); } catch (err) { setError(err instanceof Error ? err.message : 'Failed to apply role'); } }, [loadPromptContent]); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/components/prompt-manager/hooks/usePrompts.ts` around lines 59 - 70, The applyRole handler (applyRole) doesn't clear previous errors, so a past failure remains in state even after success; at the start of applyRole, call setError(null) (or clear the error state) before awaiting loadPromptContent to ensure errors are reset, then proceed to setActiveRole as before and keep the existing catch that sets setError(err instanceof Error ? err.message : 'Failed to apply role'). ``` </details> --- `16-16`: **Remove or guard console.log statements for production.** Debug logging should be removed or wrapped in a development-only check to avoid polluting production logs. <details> <summary>Suggested approach</summary> ```diff - console.log('[usePrompts] Loading prompts, projectPath:', projectPath); + if (import.meta.env.DEV) { + console.log('[usePrompts] Loading prompts, projectPath:', projectPath); + } ``` Or remove them entirely if they're not needed for ongoing debugging. </details> Also applies to: 22-22, 31-32, 35-35 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/components/prompt-manager/hooks/usePrompts.ts` at line 16, Remove or guard the development console.log calls in the usePrompts hook: locate the console.log statements inside the usePrompts hook (lines around the console logs at 16, 22, 31-32, 35) and either delete them or wrap them in a development-only condition (e.g., check process.env.NODE_ENV !== 'production' or an isDev flag) or replace them with the app's logger (e.g., debug level) so they are not emitted in production; ensure the checks are applied consistently for all console.log occurrences in usePrompts. ``` </details> --- `11-40`: **Consider adding AbortController to prevent state updates on unmounted components.** If the component unmounts while `loadPrompts` is in flight, the state updates will cause a React warning. This is especially relevant since prompts load asynchronously when the project changes. <details> <summary>Suggested pattern</summary> ```diff const loadPrompts = useCallback(async () => { + const controller = new AbortController(); setLoading(true); setError(null); try { const response = await authenticatedFetch('/api/prompts/list', { method: 'POST', - body: JSON.stringify({ projectPath }) + body: JSON.stringify({ projectPath }), + signal: controller.signal }); // ... rest of try block } catch (err) { + if (err instanceof Error && err.name === 'AbortError') { + return; // Component unmounted, ignore + } console.error('[usePrompts] Error loading prompts:', err); setError(err instanceof Error ? err.message : 'Unknown error'); } finally { setLoading(false); } + + return () => controller.abort(); }, [projectPath]); ``` Note: The cleanup function would need to be managed by the calling component via useEffect. </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/components/prompt-manager/hooks/usePrompts.ts` around lines 11 - 40, The loadPrompts function can update state after the component unmounts; add AbortController support to cancel the in-flight authenticatedFetch and avoid state updates when aborted: create an AbortController inside loadPrompts (or accept a signal param), pass controller.signal to authenticatedFetch, check signal.aborted (or catch the fetch abort error) before calling setPrompts, setLoading, or setError, and ensure the calling component's useEffect creates the controller, calls loadPrompts, and calls controller.abort() in its cleanup so no state updates occur after unmount. Use the unique symbols loadPrompts, authenticatedFetch, setPrompts, setLoading, and setError to locate and implement the change. ``` </details> </blockquote></details> <details> <summary>src/components/prompt-manager/view/PromptCard.tsx (2)</summary><blockquote> `42-59`: **Consider adding accessible labels to action buttons.** The buttons have visible text but no `aria-label` for screen readers when the context might not be clear. Adding the prompt name to the label would improve accessibility. <details> <summary>Suggested enhancement</summary> ```diff {onApply && ( <button onClick={() => onApply(prompt)} className="flex-1 rounded bg-blue-600 px-3 py-1.5 text-sm font-medium text-white transition-colors hover:bg-blue-700" + aria-label={`Apply role: ${prompt.name}`} > Apply Role </button> )} {onInsert && ( <button onClick={() => onInsert(prompt)} className="flex-1 rounded bg-green-600 px-3 py-1.5 text-sm font-medium text-white transition-colors hover:bg-green-700" + aria-label={`Insert template: ${prompt.name}`} > Insert Template </button> )} ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/components/prompt-manager/view/PromptCard.tsx` around lines 42 - 59, The Apply Role and Insert Template buttons in PromptCard.tsx lack accessible labels; update the two conditional button elements that call onApply(prompt) and onInsert(prompt) to include aria-label attributes that combine the action with the prompt identifier (e.g., aria-label={`Apply role: ${prompt.name}`} and aria-label={`Insert template: ${prompt.name}`} or fallback to prompt.id), ensuring you reference the existing onApply/onInsert handlers and the prompt object when constructing the labels. ``` </details> --- `29-39`: **Potential duplicate key issue if tags contain duplicates.** Using `tag` as the key could cause React key collisions if the same tag appears multiple times in the array. Consider using the index as part of the key. <details> <summary>Suggested fix</summary> ```diff {prompt.tags.map((tag) => ( - <span - key={tag} + {prompt.tags.map((tag, index) => ( + <span + key={`${tag}-${index}`} className="rounded bg-blue-100 px-2 py-0.5 text-xs text-blue-700 dark:bg-blue-900 dark:text-blue-300" > {tag} </span> ))} ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/components/prompt-manager/view/PromptCard.tsx` around lines 29 - 39, The PromptCard component uses prompt.tags.map with key={tag}, which can cause React key collisions when tags repeat; update the map callback (in PromptCard where prompt.tags.map is used) to include the index or another unique discriminator in the key (e.g., combine tag and index or use a stable tag id) so each <span> has a unique key (e.g., `${tag}-${index}`) to avoid duplicate-key issues. ``` </details> </blockquote></details> <details> <summary>src/components/prompt-manager/view/PromptSelector.tsx (2)</summary><blockquote> `34-41`: **Add aria-labels for accessibility.** The buttons have `title` attributes but lack `aria-label` for screen reader users. The title tooltip isn't reliably announced. <details> <summary>Suggested enhancement</summary> ```diff <button type="button" onClick={onClearRole} className="rounded p-0.5 transition-colors hover:bg-primary/20" title="Clear role" + aria-label="Clear active role" > <X className="h-3 w-3" /> </button> ``` ```diff <button type="button" onClick={onOpenLibrary} className="rounded-lg p-2 transition-colors hover:bg-accent/60" title="Open Prompt Library" + aria-label="Open Prompt Library" > <Sparkles className="h-5 w-5 text-muted-foreground" /> </button> ``` </details> Also applies to: 46-53 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/components/prompt-manager/view/PromptSelector.tsx` around lines 34 - 41, The button in PromptSelector (the clear-role button using onClearRole and the other similar buttons around the same component) currently only has a title tooltip which isn't reliably announced; add descriptive aria-label attributes (e.g., aria-label="Clear role") to the button element(s) alongside the existing title so screen readers can identify their purpose, ensuring each unique button (the one rendering <X className="h-3 w-3" /> and the other button(s) in the same file) gets an appropriate aria-label matching its action. ``` </details> --- `3-3`: **Remove the wildcard import and consolidate icon usage.** The line `import * as Icons from 'lucide-react'` bundles the entire lucide-react library (~180 kB) into your bundle, even though only a handful of icons are used. This significantly impacts bundle size compared to tree-shaken named imports (~5 kB for the same icons). Since `Sparkles` and `X` are already imported by name, consider refactoring the dynamic icon lookup to use a static map of known role icons instead of relying on the namespace import. For example: <details> <summary>Alternative using a static icon map</summary> ```tsx import { Sparkles, X, Star, Zap, type LucideIcon } from 'lucide-react'; const ROLE_ICON_MAP: Record<string, LucideIcon> = { Star, Zap, // ... other role icons }; // In component: if (activeRole?.icon && activeRole.icon in ROLE_ICON_MAP) { RoleIcon = ROLE_ICON_MAP[activeRole.icon]; } ``` This approach is tree-shakeable and only includes the icons your app actually uses. </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/components/prompt-manager/view/PromptSelector.tsx` at line 3, Remove the wildcard import from lucide-react and replace the dynamic namespace lookup with a static, tree-shakeable icon map: import the specific icons you need (e.g., Sparkles, X, Star, Zap, LucideIcon) and create a ROLE_ICON_MAP mapping role key strings to those icon components; then update the logic that sets RoleIcon (currently using activeRole?.icon and the Icons namespace) to instead check ROLE_ICON_MAP[activeRole.icon] and assign that component when present. Ensure all references to Icons.<...> are replaced with direct imports or the ROLE_ICON_MAP and keep Sparkles/X usage as named imports. ``` </details> </blockquote></details> <details> <summary>src/components/chat/hooks/useChatComposerState.ts (1)</summary><blockquote> `1023-1027`: **`handleInsertTemplate` currently overwrites existing draft input.** Line 1025 replaces the composer text entirely. For an “insert” action, preserving existing input (append/insert-at-cursor) is usually safer UX. <details> <summary>✏️ Suggested behavior (append)</summary> ```diff const handleInsertTemplate = useCallback( async (prompt: Prompt) => { const content = await insertTemplate(prompt); - setInput(content); - inputValueRef.current = content; + setInput((previous) => { + const next = previous.trim() ? `${previous}\n\n${content}` : content; + inputValueRef.current = next; + return next; + }); setShowPromptLibrary(false); }, [insertTemplate], ); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/components/chat/hooks/useChatComposerState.ts` around lines 1023 - 1027, handleInsertTemplate currently replaces the entire composer content with the template; change it to insert/append the template into the existing draft instead: call insertTemplate(prompt) to get the snippet, then compute a newInput by inserting that snippet at the current cursor/selection (fallback to appending if cursor info not available) and update state via setInput(prev => newInput) and inputValueRef.current = newInput; keep the existing setShowPromptLibrary(false) behavior. Use existing refs/state for cursor/selection if available (e.g., composerRef.selectionStart/selectionEnd) to perform proper in-place insertion. ``` </details> </blockquote></details> <details> <summary>src/components/prompt-manager/types/types.ts (1)</summary><blockquote> `2-2`: **Consider allowing unknown categories in the `PromptCategory` type definition.** If markdown prompts gain new categories without TypeScript updates, the strict union will cause type mismatches. Using `KnownPromptCategory | string` or similar would allow the frontend to safely handle unexpected values while maintaining IDE autocomplete for known categories. Alternatively, normalize and validate categories at the API boundary. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/components/prompt-manager/types/types.ts` at line 2, The current strict union type PromptCategory prevents unknown/new categories from markdown prompts; change the type to allow unknown strings while preserving autocomplete by introducing a KnownPromptCategory union (e.g., 'engineering'|'content'|...) and then export PromptCategory = KnownPromptCategory | string, or alternately update the parsing/validation at the API boundary to normalize/validate incoming categories into KnownPromptCategory and map unknown values to a fallback like 'custom'; update any usage sites of PromptCategory (e.g., type annotations, switch statements) to handle arbitrary strings or the fallback case accordingly. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@server/routes/prompts.js:
- Around line 91-105: The route currently uses the client-supplied projectPath
directly (variable projectPath) when building projectDir and calling
scanPromptsDirectory, which allows arbitrary filesystem access; replace this by
accepting a server-authorized project identifier (e.g., projectId) or mapping
and resolve the real project path on the server side, or—if you must accept a
path—validate and normalize it using path.resolve and assert it is a subpath of
a configured workspace root before using it. Update the code paths that build
projectDir and call scanPromptsDirectory (the projectDir construction and both
scanPromptsDirectory(projectDir, 'project') calls) to fetch/derive the safe
server-controlled path, and remove any direct use of req.body.projectPath.
Ensure identical validation/lookup is applied to the other occurrence that
currently uses projectPath.- Around line 59-83: The validatePromptPath function currently uses lexical
resolution only; update it to resolve symlinks and enforce Markdown-only reads
by using fs.realpathSync on both the input promptPath and each allowedDirs
entry, then ensure the real-resolved prompt path startsWith one of the
real-resolved allowed dir paths (reject otherwise) and validate the file is a
regular file (fs.statSync().isFile()). Additionally, before any file read (the
readFile call that uses the validated path), ensure the resolved filename has a
.md extension and reject other extensions. Keep references to
validatePromptPath, allowedDirs, and the subsequent file read call when making
these changes.In
@src/components/chat/hooks/useChatComposerState.ts:
- Line 164: The call to usePrompts passes selectedProject?.path which is
inconsistent with other usages; update the argument to use the same fallback
chain used elsewhere by passing selectedProject?.fullPath ||
selectedProject?.path || null so usePrompts receives the canonical project path
pattern (locate the usePrompts(...) invocation in useChatComposerState and
replace the current selectedProject?.path expression accordingly).In
@src/components/chat/view/subcomponents/ChatComposer.tsx:
- Around line 333-338: The attach-images button inside ChatComposer currently
only uses a title for accessibility; add an explicit aria-label to the button
element (the button that calls openImagePicker) using the same i18n string
t('input.attachImages') to ensure screen readers get a reliable label (you may
keep the title but must add aria-label). Update the JSX for the button that
triggers openImagePicker to include aria-label={t('input.attachImages')}.In
@src/components/prompt-manager/view/PromptLibrary.tsx:
- Around line 55-57: The PromptLibrary modal currently lacks keyboard
accessibility and focus management; update the PromptLibrary component to (1)
add role="dialog" and aria-modal="true" to the modal container and give the
title element a stable id (e.g., titleId) then set aria-labelledby to that id,
(2) on mount focus the title element (or first focusable control) and on unmount
restore focus, (3) add a keydown listener that closes the modal on Escape by
invoking the component's close handler (e.g., closeModal or onClose), and (4)
implement a basic focus trap inside the modal by cycling Tab and Shift+Tab among
focusable elements (query for focusable selectors inside the modal container) so
focus cannot escape the dialog. Ensure listeners are cleaned up on unmount.
Nitpick comments:
In@shared/prompts/templates/bug-fix.md:
- Around line 26-28: The fenced code block containing "[Paste any error messages
or logs]" is missing a language identifier; update that markdown block in the
bug-fix template to use a generic language token such as "text" or "plaintext"
(i.e., change the triple-backtick fence for that snippet to ```text) so static
analysis stops flagging it and the placeholder remains clearly marked as plain
text.In
@src/components/chat/hooks/useChatComposerState.ts:
- Around line 1023-1027: handleInsertTemplate currently replaces the entire
composer content with the template; change it to insert/append the template into
the existing draft instead: call insertTemplate(prompt) to get the snippet, then
compute a newInput by inserting that snippet at the current cursor/selection
(fallback to appending if cursor info not available) and update state via
setInput(prev => newInput) and inputValueRef.current = newInput; keep the
existing setShowPromptLibrary(false) behavior. Use existing refs/state for
cursor/selection if available (e.g., composerRef.selectionStart/selectionEnd) to
perform proper in-place insertion.In
@src/components/prompt-manager/hooks/usePrompts.ts:
- Around line 59-70: The applyRole handler (applyRole) doesn't clear previous
errors, so a past failure remains in state even after success; at the start of
applyRole, call setError(null) (or clear the error state) before awaiting
loadPromptContent to ensure errors are reset, then proceed to setActiveRole as
before and keep the existing catch that sets setError(err instanceof Error ?
err.message : 'Failed to apply role').- Line 16: Remove or guard the development console.log calls in the usePrompts
hook: locate the console.log statements inside the usePrompts hook (lines around
the console logs at 16, 22, 31-32, 35) and either delete them or wrap them in a
development-only condition (e.g., check process.env.NODE_ENV !== 'production' or
an isDev flag) or replace them with the app's logger (e.g., debug level) so they
are not emitted in production; ensure the checks are applied consistently for
all console.log occurrences in usePrompts.- Around line 11-40: The loadPrompts function can update state after the
component unmounts; add AbortController support to cancel the in-flight
authenticatedFetch and avoid state updates when aborted: create an
AbortController inside loadPrompts (or accept a signal param), pass
controller.signal to authenticatedFetch, check signal.aborted (or catch the
fetch abort error) before calling setPrompts, setLoading, or setError, and
ensure the calling component's useEffect creates the controller, calls
loadPrompts, and calls controller.abort() in its cleanup so no state updates
occur after unmount. Use the unique symbols loadPrompts, authenticatedFetch,
setPrompts, setLoading, and setError to locate and implement the change.In
@src/components/prompt-manager/types/types.ts:
- Line 2: The current strict union type PromptCategory prevents unknown/new
categories from markdown prompts; change the type to allow unknown strings while
preserving autocomplete by introducing a KnownPromptCategory union (e.g.,
'engineering'|'content'|...) and then export PromptCategory =
KnownPromptCategory | string, or alternately update the parsing/validation at
the API boundary to normalize/validate incoming categories into
KnownPromptCategory and map unknown values to a fallback like 'custom'; update
any usage sites of PromptCategory (e.g., type annotations, switch statements) to
handle arbitrary strings or the fallback case accordingly.In
@src/components/prompt-manager/view/PromptCard.tsx:
- Around line 42-59: The Apply Role and Insert Template buttons in
PromptCard.tsx lack accessible labels; update the two conditional button
elements that call onApply(prompt) and onInsert(prompt) to include aria-label
attributes that combine the action with the prompt identifier (e.g.,
aria-label={Apply role: ${prompt.name}} and aria-label={Insert template: ${prompt.name}} or fallback to prompt.id), ensuring you reference the existing
onApply/onInsert handlers and the prompt object when constructing the labels.- Around line 29-39: The PromptCard component uses prompt.tags.map with
key={tag}, which can cause React key collisions when tags repeat; update the map
callback (in PromptCard where prompt.tags.map is used) to include the index or
another unique discriminator in the key (e.g., combine tag and index or use a
stable tag id) so each has a unique key (e.g.,${tag}-${index}) to
avoid duplicate-key issues.In
@src/components/prompt-manager/view/PromptSelector.tsx:
- Around line 34-41: The button in PromptSelector (the clear-role button using
onClearRole and the other similar buttons around the same component) currently
only has a title tooltip which isn't reliably announced; add descriptive
aria-label attributes (e.g., aria-label="Clear role") to the button element(s)
alongside the existing title so screen readers can identify their purpose,
ensuring each unique button (the one rendering and the
other button(s) in the same file) gets an appropriate aria-label matching its
action.- Line 3: Remove the wildcard import from lucide-react and replace the dynamic
namespace lookup with a static, tree-shakeable icon map: import the specific
icons you need (e.g., Sparkles, X, Star, Zap, LucideIcon) and create a
ROLE_ICON_MAP mapping role key strings to those icon components; then update the
logic that sets RoleIcon (currently using activeRole?.icon and the Icons
namespace) to instead check ROLE_ICON_MAP[activeRole.icon] and assign that
component when present. Ensure all references to Icons.<...> are replaced with
direct imports or the ROLE_ICON_MAP and keep Sparkles/X usage as named imports.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Repository UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `0144cdd9-11a0-44d5-b532-f250fa5cb9a0` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 4de8b78c6db5d8c2c402afce0f0b4cc16d5b6496 and 2a825cbc8d6a245c7235e5540a4fde63eadfc5dc. </details> <details> <summary>📒 Files selected for processing (28)</summary> * `README.de.md` * `README.ja.md` * `README.ko.md` * `README.md` * `README.ru.md` * `README.zh-CN.md` * `server/index.js` * `server/routes/prompts.js` * `shared/prompts/roles/backend-engineer.md` * `shared/prompts/roles/code-reviewer.md` * `shared/prompts/roles/devops-engineer.md` * `shared/prompts/roles/frontend-developer.md` * `shared/prompts/roles/fullstack-developer.md` * `shared/prompts/templates/bug-fix.md` * `shared/prompts/templates/code-review.md` * `shared/prompts/templates/documentation.md` * `shared/prompts/templates/refactor.md` * `shared/prompts/templates/tests.md` * `src/components/chat/hooks/useChatComposerState.ts` * `src/components/chat/view/ChatInterface.tsx` * `src/components/chat/view/subcomponents/ChatComposer.tsx` * `src/components/chat/view/subcomponents/ChatInputControls.tsx` * `src/components/prompt-manager/hooks/usePrompts.ts` * `src/components/prompt-manager/index.ts` * `src/components/prompt-manager/types/types.ts` * `src/components/prompt-manager/view/PromptCard.tsx` * `src/components/prompt-manager/view/PromptLibrary.tsx` * `src/components/prompt-manager/view/PromptSelector.tsx` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+59
to
+83
| function validatePromptPath(promptPath, allowedDirs) { | ||
| // Resolve to absolute path to prevent traversal attacks | ||
| const resolvedPath = path.resolve(promptPath); | ||
|
|
||
| // Check if path is within allowed directories using path.relative | ||
| const isAllowed = allowedDirs.some(dir => { | ||
| const resolvedDir = path.resolve(dir); | ||
| const relativePath = path.relative(resolvedDir, resolvedPath); | ||
|
|
||
| // Path is valid if: | ||
| // 1. relative path doesn't start with '..' (not outside allowed dir) | ||
| // 2. relative path is not empty (not the dir itself, but that's ok) | ||
| // 3. path doesn't contain null bytes | ||
| return relativePath && | ||
| !relativePath.startsWith('..') && | ||
| !relativePath.includes('\0') && | ||
| !path.isAbsolute(relativePath); | ||
| }); | ||
|
|
||
| if (!isAllowed) { | ||
| throw new Error('Invalid prompt path'); | ||
| } | ||
|
|
||
| return resolvedPath; | ||
| } |
Contributor
There was a problem hiding this comment.
Harden file validation: current checks can allow symlink escapes and non-Markdown reads.
Line 61/65 perform lexical checks only; symlink targets can still escape allowed roots. Also, Line 148 reads any file under allowed dirs. Resolve real paths and enforce .md before reading.
🔒 Proposed hardening patch
-function validatePromptPath(promptPath, allowedDirs) {
- // Resolve to absolute path to prevent traversal attacks
- const resolvedPath = path.resolve(promptPath);
+async function validatePromptPath(promptPath, allowedDirs) {
+ if (typeof promptPath !== 'string' || !promptPath || promptPath.includes('\0')) {
+ throw new Error('Invalid prompt path');
+ }
+
+ const resolvedPath = path.resolve(promptPath);
+ const realPromptPath = await fs.realpath(resolvedPath);
+
+ if (path.extname(realPromptPath).toLowerCase() !== '.md') {
+ throw new Error('Only markdown prompt files are allowed');
+ }
- // Check if path is within allowed directories using path.relative
- const isAllowed = allowedDirs.some(dir => {
- const resolvedDir = path.resolve(dir);
- const relativePath = path.relative(resolvedDir, resolvedPath);
-
- // Path is valid if:
- // 1. relative path doesn't start with '..' (not outside allowed dir)
- // 2. relative path is not empty (not the dir itself, but that's ok)
- // 3. path doesn't contain null bytes
- return relativePath &&
- !relativePath.startsWith('..') &&
- !relativePath.includes('\0') &&
- !path.isAbsolute(relativePath);
- });
+ const checks = await Promise.all(
+ allowedDirs.map(async (dir) => {
+ try {
+ const realDir = await fs.realpath(path.resolve(dir));
+ return realPromptPath === realDir || realPromptPath.startsWith(`${realDir}${path.sep}`);
+ } catch {
+ return false;
+ }
+ }),
+ );
+ const isAllowed = checks.some(Boolean);
if (!isAllowed) {
throw new Error('Invalid prompt path');
}
- return resolvedPath;
+ return realPromptPath;
}
@@
- const validPath = validatePromptPath(promptPath, allowedDirs);
+ const validPath = await validatePromptPath(promptPath, allowedDirs);Also applies to: 145-149
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/prompts.js` around lines 59 - 83, The validatePromptPath
function currently uses lexical resolution only; update it to resolve symlinks
and enforce Markdown-only reads by using fs.realpathSync on both the input
promptPath and each allowedDirs entry, then ensure the real-resolved prompt path
startsWith one of the real-resolved allowed dir paths (reject otherwise) and
validate the file is a regular file (fs.statSync().isFile()). Additionally,
before any file read (the readFile call that uses the validated path), ensure
the resolved filename has a .md extension and reject other extensions. Keep
references to validatePromptPath, allowedDirs, and the subsequent file read call
when making these changes.
server/routes/prompts.js
Outdated
Comment on lines
+91
to
+105
| const { projectPath } = req.body; | ||
|
|
||
| // Scan built-in prompts | ||
| const builtInDir = path.join(__dirname, '../../shared/prompts'); | ||
| const builtIn = await scanPromptsDirectory(builtInDir, 'builtin'); | ||
|
|
||
| // Scan user prompts | ||
| const userDir = path.join(os.homedir(), '.claude', 'prompts'); | ||
| const user = await scanPromptsDirectory(userDir, 'user'); | ||
|
|
||
| // Scan project prompts | ||
| let project = []; | ||
| if (projectPath) { | ||
| const projectDir = path.join(projectPath, '.claude', 'prompts'); | ||
| project = await scanPromptsDirectory(projectDir, 'project'); |
Contributor
There was a problem hiding this comment.
Avoid trusting client-provided projectPath for filesystem access.
Line 103-105 and Line 140-142 directly use request-body projectPath to scan/load files. This lets callers probe arbitrary server paths they can guess. Bind to a server-authorized project identifier/path instead.
Also applies to: 140-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/prompts.js` around lines 91 - 105, The route currently uses the
client-supplied projectPath directly (variable projectPath) when building
projectDir and calling scanPromptsDirectory, which allows arbitrary filesystem
access; replace this by accepting a server-authorized project identifier (e.g.,
projectId) or mapping and resolve the real project path on the server side,
or—if you must accept a path—validate and normalize it using path.resolve and
assert it is a subpath of a configured workspace root before using it. Update
the code paths that build projectDir and call scanPromptsDirectory (the
projectDir construction and both scanPromptsDirectory(projectDir, 'project')
calls) to fetch/derive the safe server-controlled path, and remove any direct
use of req.body.projectPath. Ensure identical validation/lookup is applied to
the other occurrence that currently uses projectPath.
| applyRole, | ||
| clearRole, | ||
| insertTemplate, | ||
| } = usePrompts(selectedProject?.path || null); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Project type definition =="
project_type_file="$(fd '^app\.ts$' src types | head -n1 || true)"
if [[ -n "${project_type_file}" ]]; then
rg -n -C3 'interface Project|type Project|fullPath|path' "$project_type_file"
else
echo "Project type file not found via fd."
fi
echo
echo "== usePrompts callsites =="
rg -n -C2 'usePrompts\(' src/components
echo
echo "== Prompt API payload projectPath sources =="
rg -n -C3 'projectPath' src/components/prompt-manager src/components/chatRepository: siteboon/claudecodeui
Length of output: 15912
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Line 646 context in useChatComposerState.ts =="
file="src/components/chat/hooks/useChatComposerState.ts"
wc -l "$file"
echo
sed -n '640,650p' "$file"
echo
echo "== Full pattern matching for selectedProject path usage in useChatComposerState.ts =="
rg -n 'selectedProject\.(fullPath|path)' "$file"
echo
echo "== usePrompts hook implementation =="
cat -n src/components/prompt-manager/hooks/usePrompts.tsRepository: siteboon/claudecodeui
Length of output: 4220
Use consistent project path pattern for prompt loading.
Line 164 passes selectedProject?.path, but elsewhere in this file (lines 323 and 646) the pattern is selectedProject.fullPath || selectedProject.path. For consistency with the rest of the codebase, line 164 should follow: selectedProject?.fullPath || selectedProject?.path || null.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/chat/hooks/useChatComposerState.ts` at line 164, The call to
usePrompts passes selectedProject?.path which is inconsistent with other usages;
update the argument to use the same fallback chain used elsewhere by passing
selectedProject?.fullPath || selectedProject?.path || null so usePrompts
receives the canonical project path pattern (locate the usePrompts(...)
invocation in useChatComposerState and replace the current selectedProject?.path
expression accordingly).
Comment on lines
+55
to
+57
| return ( | ||
| <div className="fixed inset-0 z-50 flex items-center justify-center bg-black bg-opacity-50"> | ||
| <div className="flex max-h-[80vh] w-full max-w-4xl flex-col rounded-lg bg-white shadow-xl dark:bg-gray-900"> |
Contributor
There was a problem hiding this comment.
Modal lacks keyboard accessibility and focus management.
The modal doesn't handle Escape key to close or trap focus within it. Users relying on keyboard navigation may have difficulty interacting with the modal.
Suggested enhancements
+import { useEffect, useRef } from 'react';
export default function PromptLibrary({
// ... props
}: PromptLibraryProps) {
+ const modalRef = useRef<HTMLDivElement>(null);
const [activeTab, setActiveTab] = useState<PromptType>('role');
// ... other state
+ // Handle Escape key
+ useEffect(() => {
+ if (!isOpen) return;
+ const handleKeyDown = (e: KeyboardEvent) => {
+ if (e.key === 'Escape') {
+ onClose();
+ }
+ };
+ document.addEventListener('keydown', handleKeyDown);
+ return () => document.removeEventListener('keydown', handleKeyDown);
+ }, [isOpen, onClose]);
+
+ // Focus modal on open
+ useEffect(() => {
+ if (isOpen && modalRef.current) {
+ modalRef.current.focus();
+ }
+ }, [isOpen]);
if (!isOpen) return null;
return (
- <div className="fixed inset-0 z-50 flex items-center justify-center bg-black bg-opacity-50">
- <div className="flex max-h-[80vh] w-full max-w-4xl flex-col rounded-lg bg-white shadow-xl dark:bg-gray-900">
+ <div
+ className="fixed inset-0 z-50 flex items-center justify-center bg-black bg-opacity-50"
+ onClick={(e) => e.target === e.currentTarget && onClose()}
+ >
+ <div
+ ref={modalRef}
+ role="dialog"
+ aria-modal="true"
+ aria-labelledby="prompt-library-title"
+ tabIndex={-1}
+ className="flex max-h-[80vh] w-full max-w-4xl flex-col rounded-lg bg-white shadow-xl dark:bg-gray-900"
+ >And add the id to the title:
- <h2 className="text-xl font-semibold text-gray-900 dark:text-gray-100">
+ <h2 id="prompt-library-title" className="text-xl font-semibold text-gray-900 dark:text-gray-100">
Prompt Library
</h2>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/prompt-manager/view/PromptLibrary.tsx` around lines 55 - 57,
The PromptLibrary modal currently lacks keyboard accessibility and focus
management; update the PromptLibrary component to (1) add role="dialog" and
aria-modal="true" to the modal container and give the title element a stable id
(e.g., titleId) then set aria-labelledby to that id, (2) on mount focus the
title element (or first focusable control) and on unmount restore focus, (3) add
a keydown listener that closes the modal on Escape by invoking the component's
close handler (e.g., closeModal or onClose), and (4) implement a basic focus
trap inside the modal by cycling Tab and Shift+Tab among focusable elements
(query for focusable selectors inside the modal container) so focus cannot
escape the dialog. Ensure listeners are cleaned up on unmount.
…prompt-library-menu Codex-generated pull request
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add ActiveRoleWithPriority type with priority and path fields - Replace single activeRole with activeRoles array in usePrompts - Add localStorage persistence for active roles - Implement toggleRole, reorderRoles, removeRole, clearAllRoles methods - Add getCombinedRoleContent to merge multiple role contents - Create RoleManagementModal with drag-and-drop reordering - Create SortableRoleItem component using @dnd-kit - Update PromptCard with iOS-style toggle switch for roles - Update PromptSelector to show role counter and open management modal - Update PromptLibrary to use toggleRole instead of applyRole - Integrate with chat components (ChatInterface, ChatComposer, ChatInputControls) - Add i18n translations for all languages (en, de, ja, ko, ru, zh-CN) - Limit to maximum 5 active roles Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… alert dialogs - Replace setError with window.alert for role limit and toggle errors - Add optimistic UI updates to role toggle for instant visual feedback - Eliminate perceived delay when activating/deactivating roles
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduce a prompt manager module with role and template support in chat.
Add a prompt library modal, selector controls, prompt cards, shared prompt types, and a hook for loading and applying prompts.
TODO:
Add all language support
Summary by CodeRabbit
Release Notes
New Features
Documentation