Open
Conversation
Add shared prompt definitions (23 IDs, filenames, categories) and the prompt-manager that loads prompts from disk at startup with user customization support. Bundle core prompts as extraResources for all three platforms. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add prompts IPC handlers (get, getAll, save, reset) and preload API factory following the existing modular pattern. Register in both handler and preload index files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Call initializePrompts() early in app.whenReady() before any features that use prompts. Fail-safe with dialog and quit on load failure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace compiled prompt constant imports with getPrompt() calls in group-chat, director-notes, tab-naming, and feedback handlers. Add CLI-specific async prompt loader for batch-processor. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace compiled prompt imports with IPC-based cache+loader pattern in settingsStore, agentStore, useInputProcessing, useWizardHandlers, useAgentListeners, useMergeTransferHandlers, and batchUtils. Add prompts namespace to MaestroAPI type in global.d.ts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace compiled prompt imports with IPC-based cache+loader pattern in contextGroomer, contextSummarizer, inlineWizardConversation, inlineWizardDocumentGeneration, wizardPrompts, and phaseGenerator. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Create promptInit.ts that loads all 13 renderer prompt caches in parallel via IPC. Gate MaestroConsole rendering on promptsReady state. Add refreshRendererPrompts() for Settings UI cache invalidation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Delete src/generated/prompts.ts and scripts/generate-prompts.mjs. Remove build:prompts from dev:main, dev:main:prod-data, build, and validate:push scripts. Update src/prompts/index.ts to re-export shared definitions. Remove src/generated/ from .gitignore. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add MaestroPromptsTab component with split-view browser/editor for all 23 core prompts grouped by category. Supports save (immediate effect via cache refresh), reset to default, modified indicators, and unsaved-changes guard. Integrated into SettingsModal sidebar and settings search. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update 11 test files to mock prompt-manager (main process) or window.maestro.prompts (renderer) instead of old compiled imports. Add prompts mock to global test setup. Fix DEFAULT_BATCH_PROMPT to use let with refresh on load. All 25018 tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add prompt customization entries to CLAUDE.md key files table: Customize prompts, Add new prompt, and updated Modify system prompts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…g instructions Add one-time migration that bakes existing moderator standing instructions into a prompt customization. Remove Group Chat settings tab, moderatorStandingInstructions state/setter, and standing instructions injection from group-chat-router. Conductor profile remains in General tab unchanged. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…error recovery Address all issues found during code review of the core-prompts branch: - HIGH: /commit prompt empty for first-run users. DEFAULT_AI_COMMANDS captured the empty cache at module load; loadSettingsStorePrompts() now rebuilds the array AND patches the live Zustand store when the commit command prompt is still blank. - HIGH: Image-only messages default to empty prompt. DEFAULT_IMAGE_ONLY_PROMPT was a const evaluated at module load; converted to let and updated after loadInputProcessingPrompts() completes. - HIGH: CLI prompt loading broken outside dev/Electron. Replaced the Electron-only process.resourcesPath fallback with multi-candidate path resolution (dev, Electron, script-relative, __dirname-relative). - MEDIUM: Renderer prompt init cannot recover after failure. Cleared initPromise on rejection so subsequent calls retry instead of returning the same rejected promise. - HIGH (DEFAULT_BATCH_PROMPT): Same module-load-time const issue; converted to let with refresh-on-load (fixed in phase 10). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace hardcoded .maestro/playbooks/ paths with the {{AUTORUN_FOLDER}}
template variable so generated Auto Run documents are saved to the
agent's configured Auto Run folder.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
registerPromptsHandlers() was only added to registerAllHandlers() in the handlers index, but main/index.ts registers handlers individually and never calls registerAllHandlers(). Import and call it directly alongside the other no-dependency handlers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… 23 seemed excessive and once the user finds the four he finds the others
…izations - HIGH: Zustand store patch guard changed from !commitCmd.prompt to commitCmd.prompt !== cachedCommitCommandPrompt so save/reset in Maestro Prompts tab updates the live /commit command mid-session - MEDIUM: CLI getCliPrompt() now reads core-prompts-customizations.json from userData before falling back to bundled files, matching the same customization precedence as the Electron prompt-manager Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add Maestro Prompts row to Settings tabs table and new section in docs/configuration.md covering editing, resetting, and Quick Actions access. Add Prompt Customization section to docs/cli.md with the 5 CLI-relevant prompts, customizations JSON file paths per platform, and file format for direct editing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMoves prompts from build-time generated constants to a runtime, disk-backed system: adds a main-process prompt manager, IPC + preload prompt API, renderer initialization and per-module async loaders, a Maestro Prompts settings UI (save/reset), packaging of markdown prompts, and updates across main/renderer/CLI consumers and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer as "Renderer"
participant Preload as "Preload (ipcRenderer)"
participant Main as "Main (ipcMain / prompt-manager)"
participant FS as "FileSystem"
Renderer->>Preload: maestro.prompts.getAll() / get(id)
Preload->>Main: ipc.invoke('prompts:getAll' / 'prompts:get', id)
Main->>Main: if !initialized -> initializePrompts()
Main->>FS: read bundled prompt files (resources/src/prompts/*.md)
Main->>FS: read userData/core-prompts-customizations.json (optional)
Main->>Main: merge defaults + customizations -> promptCache
Main-->>Preload: return prompts / content
Preload-->>Renderer: resolve Promise with { success, prompts|content }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR implements full user customization of all 23 core system prompts via a new Maestro Prompts tab in Settings, along with CLI support for reading those customizations. The previous review findings (CLI not loading customizations, missing Sentry reporting, bare Confidence Score: 5/5Safe to merge — all P0/P1 findings from the previous review are resolved; only minor P2 style issues remain. All three prior review concerns (CLI not loading customizations, missing Sentry capture on prompt-init failure, bare console.* calls) have been addressed in follow-up commits. The remaining findings are P2: an unawaited IPC save in the one-time commit-command migration (retries on next startup, no data loss) and a redundant double-call to unregisterCliActivity (idempotent, harmless). src/renderer/stores/settingsStore.ts (unawaited migration save, line 78) Important Files Changed
Sequence DiagramsequenceDiagram
participant Renderer as Renderer (App.tsx)
participant PromptInit as promptInit.ts
participant IPC as IPC (preload)
participant Main as Main Process
participant PM as prompt-manager.ts
participant Disk as Disk
Renderer->>PromptInit: initializeRendererPrompts()
PromptInit->>IPC: prompts.get(id) x13 parallel
IPC->>Main: prompts:get / prompts:getAll
Main->>PM: getPrompt(id) / getAllPrompts()
PM-->>Main: cached content
Main-->>IPC: success, content
IPC-->>PromptInit: resolved
PromptInit-->>Renderer: promptsReady = true
Note over Renderer,Disk: User edits a prompt in Settings
Renderer->>IPC: prompts.save(id, content)
IPC->>Main: prompts:save
Main->>PM: savePrompt(id, content)
PM->>Disk: read customizations.json
PM->>Disk: write customizations.json
PM->>PM: update in-memory cache
Main-->>IPC: success true
IPC-->>Renderer: result
Renderer->>PromptInit: refreshRendererPrompts()
PromptInit->>IPC: reload all 13 loaders force=true
Reviews (2): Last reviewed commit: "fix: address code review findings across..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/group-chat/group-chat-router.ts (1)
769-781:⚠️ Potential issue | 🟡 MinorReplace all
{{CONDUCTOR_PROFILE}}placeholders in editable moderator prompts.Now that these templates are user-customizable,
{{CONDUCTOR_PROFILE}}can reasonably appear more than once. The singlereplace(...)used just above these prompt bodies only resolves the first token, so later copies leak through verbatim.Suggested fix
- const baseSystemPrompt = getModeratorSystemPrompt().replace( - '{{CONDUCTOR_PROFILE}}', - moderatorSettings.conductorProfile || '(No conductor profile set)' - ); + const baseSystemPrompt = getModeratorSystemPrompt().replace( + /\{\{CONDUCTOR_PROFILE\}\}/g, + moderatorSettings.conductorProfile || '(No conductor profile set)' + ); ... - const synthBasePrompt = getModeratorSystemPrompt().replace( - '{{CONDUCTOR_PROFILE}}', - synthModeratorSettings.conductorProfile || '(No conductor profile set)' - ); + const synthBasePrompt = getModeratorSystemPrompt().replace( + /\{\{CONDUCTOR_PROFILE\}\}/g, + synthModeratorSettings.conductorProfile || '(No conductor profile set)' + );Also applies to: 1701-1716
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/group-chat/group-chat-router.ts` around lines 769 - 781, The moderator prompt construction is vulnerable because a single replace call only replaces the first '{{CONDUCTOR_PROFILE}}' token, leaving later occurrences unchanged; update the code that prepares editable moderator prompt templates (e.g., the strings used to build fullPrompt and the other prompt around lines 1701-1716) to perform a global replacement of '{{CONDUCTOR_PROFILE}}' (use a global regex or split/join) on the template(s) before assembling fullPrompt so every placeholder instance is resolved.
🧹 Nitpick comments (5)
src/__tests__/main/group-chat/group-chat-agent.test.ts (1)
53-58: Make unknown prompt IDs fail this mock instead of returning placeholder text.Line 53 and Line 57 currently hide prompt wiring mistakes. If a production ID changes or is misspelled, these tests can still pass.
Suggested fix
const filename = filenameMap[id]; - if (!filename) return `mock prompt for ${id}`; + if (!filename) throw new Error(`Unexpected prompt id in test: ${id}`); try { return fs.readFileSync(path.join(promptsDir, filename), 'utf-8'); - } catch { - return `mock prompt for ${id}`; + } catch (error) { + throw new Error(`Failed to load prompt fixture for ${id}: ${String(error)}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/group-chat/group-chat-agent.test.ts` around lines 53 - 58, The mock currently returns a placeholder string when a prompt filename is missing or read fails, which masks wiring errors; update the mock so that when filename is falsy or fs.readFileSync(path.join(promptsDir, filename), 'utf-8') throws, the mock throws a descriptive Error instead (including the prompt id and filename) rather than returning `mock prompt for ${id}`, so missing/unknown prompt IDs fail the test; locate the logic using variables filename, promptsDir and the fs.readFileSync call and replace the fallback return with throwing an Error that mentions the id and filename.src/renderer/services/inlineWizardConversation.ts (1)
228-238: Consider adding a guard for empty prompts.If
loadInlineWizardConversationPrompts()hasn't completed, the getters return empty strings, andgenerateInlineWizardPrompt()would return a mostly empty prompt after template substitution. While the inline wizard is user-initiated (so prompts should be loaded by then), a defensive check could prevent confusing agent behavior if prompts somehow aren't loaded.💡 Optional defensive check
export function generateInlineWizardPrompt(config: InlineWizardConversationConfig): string { const { mode, projectName, directoryPath, goal, existingDocs, autoRunFolderPath } = config; // Select the base prompt based on mode let basePrompt: string; if (mode === 'iterate') { basePrompt = getWizardInlineIteratePrompt(); } else { // 'new' mode uses the new plan prompt basePrompt = getWizardInlineNewPrompt(); } + if (!basePrompt) { + throw new Error(`Inline wizard prompt not loaded for mode: ${mode}`); + } + // Handle wizard-specific variables...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/services/inlineWizardConversation.ts` around lines 228 - 238, generateInlineWizardPrompt currently assumes getWizardInlineIteratePrompt/getWizardInlineNewPrompt return non-empty strings; add a defensive guard after selecting basePrompt (e.g., after calling getWizardInlineIteratePrompt or getWizardInlineNewPrompt) to detect an empty string (which can happen if loadInlineWizardConversationPrompts() hasn't run) and handle it by returning a clear fallback prompt or throwing/logging a descriptive error. Reference generateInlineWizardPrompt, getWizardInlineIteratePrompt, getWizardInlineNewPrompt and loadInlineWizardConversationPrompts when adding the check so the code fails fast with an informative message instead of producing a mostly empty prompt.src/renderer/services/contextGroomer.ts (1)
197-197: Consider defensive checks for empty prompts.Unlike
agentStore.tswhich checksgetMaestroSystemPrompt()for truthiness before use, these prompt usages don't guard against empty strings. IfloadContextGroomerPrompts()hasn't completed,getContextTransferPrompt()andgetContextGroomingPrompt()return empty strings.Given context grooming is user-initiated after app startup, this is likely fine in practice, but consider adding a guard or logging a warning if the prompt is unexpectedly empty.
Also applies to: 392-392
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/services/contextGroomer.ts` at line 197, The prompt-returning functions getContextTransferPrompt() and getContextGroomingPrompt() can return empty strings if loadContextGroomerPrompts() hasn't finished; add a defensive guard inside the callers (or within these functions) to detect an empty/falsey prompt and either log a warning via the existing logger or return a sensible fallback before proceeding; specifically update the return site that currently just returns getContextTransferPrompt() (and the analogous site calling getContextGroomingPrompt()) to check the prompt value, log a clear warning mentioning getContextTransferPrompt/getContextGroomingPrompt, and avoid using an empty prompt (e.g., abort the grooming flow or use a default message).src/shared/promptDefinitions.ts (1)
192-208: Minor: Duplicate "System" section comments.Lines 192-193 and 203-204 both have
// Systemcomments, making the grouping slightly confusing. Consider renaming one to// System (misc)or similar for clarity.📝 Suggested comment clarification
// System MAESTRO_SYSTEM_PROMPT: 'maestro-system-prompt', // Group Chat GROUP_CHAT_MODERATOR_SYSTEM: 'group-chat-moderator-system', GROUP_CHAT_MODERATOR_SYNTHESIS: 'group-chat-moderator-synthesis', GROUP_CHAT_PARTICIPANT: 'group-chat-participant', GROUP_CHAT_PARTICIPANT_REQUEST: 'group-chat-participant-request', // Context CONTEXT_GROOMING: 'context-grooming', CONTEXT_TRANSFER: 'context-transfer', CONTEXT_SUMMARIZE: 'context-summarize', - // System + // System (utilities) TAB_NAMING: 'tab-naming', DIRECTOR_NOTES: 'director-notes', FEEDBACK: 'feedback', FEEDBACK_CONVERSATION: 'feedback-conversation',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/promptDefinitions.ts` around lines 192 - 208, The comment header "System" is duplicated and causes confusion; change the second occurrence above TAB_NAMING/DIRECTOR_NOTES/FEEDBACK (locate around the constants TAB_NAMING, DIRECTOR_NOTES, FEEDBACK, FEEDBACK_CONVERSATION) to a clearer label such as "// System (misc)" or "// System - UI/meta" so the grouping is distinct from the earlier "System" header near MAESTRO_SYSTEM_PROMPT; update only the comment text—no code logic changes.src/main/prompt-manager.ts (1)
72-87: Consider usingunknowninstead ofanyfor caught error.Minor typing improvement for stricter type safety.
📝 Suggested type refinement
-async function loadUserCustomizations(): Promise<StoredData | null> { - const filePath = getCustomizationsPath(); - try { - const content = await fs.readFile(filePath, 'utf-8'); - return JSON.parse(content) as StoredData; - } catch (error: any) { - // File not existing is expected (no customizations yet) - if (error?.code === 'ENOENT') { - return null; - } +async function loadUserCustomizations(): Promise<StoredData | null> { + const filePath = getCustomizationsPath(); + try { + const content = await fs.readFile(filePath, 'utf-8'); + return JSON.parse(content) as StoredData; + } catch (error: unknown) { + // File not existing is expected (no customizations yet) + if (error instanceof Error && 'code' in error && error.code === 'ENOENT') { + return null; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/prompt-manager.ts` around lines 72 - 87, In loadUserCustomizations(), change the catch parameter type from any to unknown for stricter typing; then narrow it before accessing .code by asserting or guarding (e.g., const err = error as NodeJS.ErrnoException | undefined and check err?.code === 'ENOENT') and use a safe string conversion when logging (e.g., String(error) or util.inspect) so you don't access properties on an unknown value directly; update the logger.error call to use that converted value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/__tests__/renderer/stores/agentStore.test.ts`:
- Around line 110-119: The prompts.get mock currently returns Promise.resolve({
success: true, content: prompts[id] ?? '' }) which treats unknown IDs the same
as intentionally empty prompts; change the mock in the test (the vi.fn for
prompts.get) so it checks whether id exists in the prompts map and returns
Promise.resolve({ success: true, content: prompts[id] }) for known keys and
Promise.resolve({ success: false, content: '' }) (or similar error payload) for
unknown keys — update the vi.fn implementation used in the test to explicit
existence-check the prompts map rather than using the nullish coalescing
fallback.
In `@src/main/ipc/handlers/prompts.ts`:
- Around line 65-73: The prompts:save IPC handler is missing the initialization
guard; add the same arePromptsInitialized() check used in prompts:get,
prompts:getAll, and prompts:getAllIds at the start of the
ipcMain.handle('prompts:save', ...) callback and immediately return a failure
response (e.g., { success: false, error: 'Prompts not initialized' }) if it
returns false, before calling savePrompt(id, content); keep the existing
try/catch and logger.error usage around savePrompt unchanged.
- Around line 76-84: The 'prompts:reset' IPC handler lacks the initialization
guard present in 'prompts:save'; update the ipcMain.handle('prompts:reset', ...)
handler to first perform the same initialization check used in the
'prompts:save' handler (reuse the same condition/utility), return the same
failure shape when not initialized, and log the event via the same logger
pattern before calling resetPrompt(id) so resetPrompt and logger are only
invoked when initialization is confirmed.
In `@src/renderer/App.tsx`:
- Around line 3102-3105: The catch block that currently only console.errors
prompt initialization failures should also report the failure to Sentry: import
and call captureException(err, { extra: { stage: 'prompt-init' } }) (or
captureMessage for a short event) from the Sentry utilities, include contextual
metadata (e.g., userId, environment, or any available state), then continue to
call setPromptsReady(true) so the app degrades gracefully; update the error
handler around the promise rejection to use captureException/captureMessage
before setting prompts ready.
In `@src/renderer/components/Settings/SettingsModal.tsx`:
- Line 635: The prompts tab mounts MaestroPromptsTab when activeTab ===
'prompts' but doesn't render the expected search anchor/data attribute that
PROMPTS_SETTINGS registers (data-setting-id="prompts-editor"), so
handleSearchNavigate can't find or scroll to the target; update the tab render
to include a wrapper element with the matching data-setting-id (e.g., add a
container around MaestroPromptsTab with data-setting-id="prompts-editor") so
that when activeTab switches to 'prompts' the search can locate, scroll to, and
highlight the correct element.
In `@src/renderer/components/Settings/tabs/MaestroPromptsTab.css`:
- Line 115: The CSS font-family declaration is failing stylelint's
font-family-name-quotes rule; update the font-family property (the line
containing font-family: 'Monaco', 'Menlo', 'Consolas', monospace;) to remove the
unnecessary single quotes around Monaco, Menlo and Consolas so it becomes:
font-family: Monaco, Menlo, Consolas, monospace; (keep quotes only for family
names that require them).
In `@src/renderer/components/Settings/tabs/MaestroPromptsTab.tsx`:
- Around line 55-72: The load/save/reset flows in MaestroPromptsTab currently
swallow failures by only calling setError; import and use the Sentry helpers
(captureException and optionally captureMessage) from src/utils/sentry.ts and
report errors alongside setting UI state: in the useEffect load block catch
clause call captureException(err) before setError, and when
window.maestro.prompts.getAll() or save/reset returns result.success === false
call captureMessage or captureException with contextual info (include operation
name and result.error) before setError; apply the same pattern inside the save
handler (savePrompt) and the reset handler (resetPrompt) so all unexpected
exceptions or failed IPC results are sent to Sentry while preserving the
existing setError UI behavior.
- Around line 55-72: The initial load in MaestroPromptsTab's useEffect swallows
getAll() failures by leaving selectedPrompt null so the error UI never renders;
update the loader logic to always surface load errors regardless of
selectedPrompt by setting an explicit load state or forcing selectedPrompt to a
sentinel when getAll() fails: in the async block around
window.maestro.prompts.getAll() ensure you call setError(...) and also set a
"loaded" flag (e.g., setPromptsLoaded(true)) or setSelectedPrompt to a sentinel
value, then update the component render path to check that error/loaded state
(instead of only checking selectedPrompt) so the error message displays on
initial failure; apply the same change to the similar logic in the other range
(lines 220-299) that fetches prompts.
In `@src/renderer/components/Wizard/services/phaseGenerator.ts`:
- Around line 17-33: The code currently uses cachedPhaseGenDocPrompt = '' as a
sentinel which conflates an intentionally blank prompt with "not loaded"; change
cachedPhaseGenDocPrompt to be nullable (string | null) and initialize to null,
update loadPhaseGeneratorPrompts to set it to the fetched string, and update
getWizardDocumentGenerationPrompt to either throw or assert when
phaseGeneratorPromptsLoaded is false (or return null) so callers (e.g.,
generateDocumentGenerationPrompt) cannot silently use an empty prompt; also
update any other usages that read cachedPhaseGenDocPrompt (the similar block
referenced at lines around 370-374) to handle the nullable value or call
loadPhaseGeneratorPrompts before accessing the getter.
In `@src/renderer/components/Wizard/services/wizardPrompts.ts`:
- Around line 15-46: The caches cachedWizardSystemPrompt and
cachedWizardSystemContinuationPrompt are initialized to empty strings so callers
(generateSystemPrompt) can't tell "not loaded" from intentionally blank; change
their types to string | null (initialize to null), and in loadWizardPrompts
assign the loaded content as before and set wizardPromptsLoaded = true; update
getWizardSystemPrompt and getWizardSystemContinuationPrompt to either throw an
Error when wizardPromptsLoaded is false (or return null consistently) so callers
can detect the unloaded state; adjust any callers that assume non-null to handle
null/throw accordingly.
In `@src/renderer/hooks/agent/useAgentListeners.ts`:
- Around line 53-69: The cachedAutorunSynopsisPrompt uses an empty string as the
unloaded sentinel and getAutorunSynopsisPrompt returns it without verifying load
state; change the logic so the unloaded state is explicit and guarded: keep
agentListenersPromptsLoaded boolean and modify getAutorunSynopsisPrompt (and any
consumers like SYNOPSIS_PROMPT usage) to throw or return null if
agentListenersPromptsLoaded is false, or alternatively initialize
cachedAutorunSynopsisPrompt to undefined and make getAutorunSynopsisPrompt
validate agentListenersPromptsLoaded (or undefined) before returning the prompt;
ensure loadAgentListenersPrompts still sets cachedAutorunSynopsisPrompt and
flips agentListenersPromptsLoaded to true so callers cannot treat an
uninitialized cache as a valid empty prompt.
In `@src/renderer/hooks/agent/useMergeTransferHandlers.ts`:
- Around line 28-44: The code uses an empty-string sentinel
(cachedMergeTransferSystemPrompt = '') which conflates "loaded-but-empty" with
"not-loaded" and can cause appendSystemPrompt to be skipped; change
cachedMergeTransferSystemPrompt to a nullable/undefined initial value (e.g., let
cachedMergeTransferSystemPrompt: string | null = null), keep using
mergeTransferPromptsLoaded as the explicit loaded flag in
loadMergeTransferPrompts (set cachedMergeTransferSystemPrompt = result.content!
which may be '' and set mergeTransferPromptsLoaded = true), and update
getMaestroSystemPrompt to return the nullable value (or undefined) instead of ''
so callers (e.g., where appendSystemPrompt is invoked) can distinguish an
unloaded state from a deliberately blank prompt and only skip appendSystemPrompt
when mergeTransferPromptsLoaded is true and the prompt is truly empty.
In `@src/renderer/hooks/wizard/useWizardHandlers.ts`:
- Around line 40-56: The cached prompt should not default to an empty string
because that makes "not loaded" indistinguishable from "intentionally blank";
change cachedAutorunSynopsisPrompt to be nullable (e.g. string | null) and
update loadWizardHandlersPrompts to set it to result.content (which may be an
empty string) and wizardHandlersPromptsLoaded to true, then modify
getAutorunSynopsisPrompt to first check wizardHandlersPromptsLoaded (or that
cachedAutorunSynopsisPrompt !== null) and either throw/return undefined when not
loaded so callers (e.g. the /history composition) can detect and avoid using an
ambiguous value; update any other usages that call getAutorunSynopsisPrompt
(including the other referenced occurrences) to handle the not-loaded case
accordingly.
In `@src/renderer/services/contextSummarizer.ts`:
- Around line 52-54: The function getContextSummarizePrompt currently returns
cachedContextSummarizePrompt even when it's empty, which yields a misleading
prompt; update getContextSummarizePrompt (and the similar code path around where
the cached prompt is read at the other spot) to check that
cachedContextSummarizePrompt is initialized/non-empty and throw an explicit
Error (or assert) if not, so calling code fails fast instead of proceeding with
an invalid prompt; reference the cachedContextSummarizePrompt variable and the
getContextSummarizePrompt function when making the change.
In `@src/renderer/services/inlineWizardDocumentGeneration.ts`:
- Around line 19-50: The cached prompt variables
(cachedWizardDocumentGenerationPrompt,
cachedWizardInlineIterateGenerationPrompt) currently default to '' which makes
"unloaded" indistinguishable from intentionally blank; update the implementation
so caches are nullable (e.g. string | null) or have callers guard on
inlineWizardDocGenPromptsLoaded: change loadInlineWizardDocGenPrompts to set the
caches to non-null values and inlineWizardDocGenPromptsLoaded=true on success,
and modify getWizardDocumentGenerationPrompt and
getWizardInlineIterateGenerationPrompt (and any call sites like
generateDocumentPrompt) to either throw/return a safe fallback when
inlineWizardDocGenPromptsLoaded is false or check for null before using the
prompt so an uninitialized state cannot be treated as an empty template.
---
Outside diff comments:
In `@src/main/group-chat/group-chat-router.ts`:
- Around line 769-781: The moderator prompt construction is vulnerable because a
single replace call only replaces the first '{{CONDUCTOR_PROFILE}}' token,
leaving later occurrences unchanged; update the code that prepares editable
moderator prompt templates (e.g., the strings used to build fullPrompt and the
other prompt around lines 1701-1716) to perform a global replacement of
'{{CONDUCTOR_PROFILE}}' (use a global regex or split/join) on the template(s)
before assembling fullPrompt so every placeholder instance is resolved.
---
Nitpick comments:
In `@src/__tests__/main/group-chat/group-chat-agent.test.ts`:
- Around line 53-58: The mock currently returns a placeholder string when a
prompt filename is missing or read fails, which masks wiring errors; update the
mock so that when filename is falsy or fs.readFileSync(path.join(promptsDir,
filename), 'utf-8') throws, the mock throws a descriptive Error instead
(including the prompt id and filename) rather than returning `mock prompt for
${id}`, so missing/unknown prompt IDs fail the test; locate the logic using
variables filename, promptsDir and the fs.readFileSync call and replace the
fallback return with throwing an Error that mentions the id and filename.
In `@src/main/prompt-manager.ts`:
- Around line 72-87: In loadUserCustomizations(), change the catch parameter
type from any to unknown for stricter typing; then narrow it before accessing
.code by asserting or guarding (e.g., const err = error as NodeJS.ErrnoException
| undefined and check err?.code === 'ENOENT') and use a safe string conversion
when logging (e.g., String(error) or util.inspect) so you don't access
properties on an unknown value directly; update the logger.error call to use
that converted value.
In `@src/renderer/services/contextGroomer.ts`:
- Line 197: The prompt-returning functions getContextTransferPrompt() and
getContextGroomingPrompt() can return empty strings if
loadContextGroomerPrompts() hasn't finished; add a defensive guard inside the
callers (or within these functions) to detect an empty/falsey prompt and either
log a warning via the existing logger or return a sensible fallback before
proceeding; specifically update the return site that currently just returns
getContextTransferPrompt() (and the analogous site calling
getContextGroomingPrompt()) to check the prompt value, log a clear warning
mentioning getContextTransferPrompt/getContextGroomingPrompt, and avoid using an
empty prompt (e.g., abort the grooming flow or use a default message).
In `@src/renderer/services/inlineWizardConversation.ts`:
- Around line 228-238: generateInlineWizardPrompt currently assumes
getWizardInlineIteratePrompt/getWizardInlineNewPrompt return non-empty strings;
add a defensive guard after selecting basePrompt (e.g., after calling
getWizardInlineIteratePrompt or getWizardInlineNewPrompt) to detect an empty
string (which can happen if loadInlineWizardConversationPrompts() hasn't run)
and handle it by returning a clear fallback prompt or throwing/logging a
descriptive error. Reference generateInlineWizardPrompt,
getWizardInlineIteratePrompt, getWizardInlineNewPrompt and
loadInlineWizardConversationPrompts when adding the check so the code fails fast
with an informative message instead of producing a mostly empty prompt.
In `@src/shared/promptDefinitions.ts`:
- Around line 192-208: The comment header "System" is duplicated and causes
confusion; change the second occurrence above TAB_NAMING/DIRECTOR_NOTES/FEEDBACK
(locate around the constants TAB_NAMING, DIRECTOR_NOTES, FEEDBACK,
FEEDBACK_CONVERSATION) to a clearer label such as "// System (misc)" or "//
System - UI/meta" so the grouping is distinct from the earlier "System" header
near MAESTRO_SYSTEM_PROMPT; update only the comment text—no code logic changes.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 58de1a76-60e6-43bb-95e4-bd9ee7729b31
📒 Files selected for processing (54)
.gitignoreCLAUDE.mdpackage.jsonscripts/generate-prompts.mjssrc/__tests__/main/group-chat/group-chat-agent.test.tssrc/__tests__/main/group-chat/group-chat-moderator.test.tssrc/__tests__/main/group-chat/group-chat-router.test.tssrc/__tests__/main/ipc/handlers/director-notes.test.tssrc/__tests__/main/ipc/handlers/tabNaming.test.tssrc/__tests__/renderer/components/BatchRunnerModal.test.tsxsrc/__tests__/renderer/components/Wizard/services/wizardPrompts.test.tssrc/__tests__/renderer/hooks/useWizardHandlers.test.tssrc/__tests__/renderer/services/contextGroomer.test.tssrc/__tests__/renderer/services/inlineWizardDocumentGeneration.test.tssrc/__tests__/renderer/stores/agentStore.test.tssrc/__tests__/setup.tssrc/cli/services/batch-processor.tssrc/main/group-chat/group-chat-agent.tssrc/main/group-chat/group-chat-moderator.tssrc/main/group-chat/group-chat-router.tssrc/main/index.tssrc/main/ipc/handlers/director-notes.tssrc/main/ipc/handlers/feedback.tssrc/main/ipc/handlers/index.tssrc/main/ipc/handlers/prompts.tssrc/main/ipc/handlers/tabNaming.tssrc/main/preload/index.tssrc/main/preload/prompts.tssrc/main/prompt-manager.tssrc/prompts/index.tssrc/prompts/openspec/openspec.implement.mdsrc/prompts/speckit/speckit.implement.mdsrc/renderer/App.tsxsrc/renderer/components/Settings/SettingsModal.tsxsrc/renderer/components/Settings/searchableSettings.tssrc/renderer/components/Settings/tabs/MaestroPromptsTab.csssrc/renderer/components/Settings/tabs/MaestroPromptsTab.tsxsrc/renderer/components/Wizard/services/phaseGenerator.tssrc/renderer/components/Wizard/services/wizardPrompts.tssrc/renderer/global.d.tssrc/renderer/hooks/agent/useAgentListeners.tssrc/renderer/hooks/agent/useMergeTransferHandlers.tssrc/renderer/hooks/batch/batchUtils.tssrc/renderer/hooks/input/useInputProcessing.tssrc/renderer/hooks/settings/useSettings.tssrc/renderer/hooks/wizard/useWizardHandlers.tssrc/renderer/services/contextGroomer.tssrc/renderer/services/contextSummarizer.tssrc/renderer/services/inlineWizardConversation.tssrc/renderer/services/inlineWizardDocumentGeneration.tssrc/renderer/services/promptInit.tssrc/renderer/stores/agentStore.tssrc/renderer/stores/settingsStore.tssrc/shared/promptDefinitions.ts
💤 Files with no reviewable changes (3)
- .gitignore
- src/renderer/hooks/settings/useSettings.ts
- scripts/generate-prompts.mjs
Replace bare console.log/console.error in promptInit.ts and App.tsx with captureException() from renderer/utils/sentry so prompt initialization failures surface in production error tracking. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
src/renderer/components/Settings/SettingsModal.tsx (1)
637-639:⚠️ Potential issue | 🟡 MinorAdd a searchable anchor wrapper for the Prompts tab content.
handleSearchNavigatedepends ondata-setting-idtargets in the rendered tab content; this branch mountsMaestroPromptsTabwithout one, so search-driven scroll/highlight can miss this tab target.Suggested fix
- {activeTab === 'prompts' && ( - <MaestroPromptsTab theme={theme} initialSelectedPromptId={initialSelectedPromptId} /> - )} + {activeTab === 'prompts' && ( + <div data-setting-id="prompts-editor"> + <MaestroPromptsTab + theme={theme} + initialSelectedPromptId={initialSelectedPromptId} + /> + </div> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/Settings/SettingsModal.tsx` around lines 637 - 639, The Prompts tab mounts MaestroPromptsTab without the searchable anchor wrapper that handleSearchNavigate expects; wrap the rendered content for the 'prompts' branch in the same searchable anchor element (the container that includes the data-setting-id attribute used elsewhere) so that handleSearchNavigate can find the tab target; locate the conditional rendering where activeTab === 'prompts' and ensure the outer element provides the correct data-setting-id (matching the pattern other tabs use) and any CSS/role attributes required for scroll/highlight behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/cli.md`:
- Around line 212-221: Update the fenced code block showing OS-specific paths to
include a language identifier (use "text") so markdownlint stops flagging it;
locate the triple-backtick block containing the macOS/Linux/Windows paths (the
block starting with "# macOS" and ending with
"%APPDATA%\\Maestro\\core-prompts-customizations.json") and change the opening
fence from ``` to ```text (ensure the closing fence remains ```).
In `@docs/configuration.md`:
- Line 21: The phrase "auto run" in the table row under "Maestro Prompts" should
use the product styling "Auto Run" consistently; update the text "Browse and
edit the 23 core system prompts (wizard, auto run, group chat, context, etc.)"
to "Browse and edit the 23 core system prompts (wizard, Auto Run, group chat,
context, etc.)" and make the same change for the matching occurrence mentioned
(the other instance at the same section). Locate the string in the docs and
replace the lowercase "auto run" with the styled "Auto Run" wherever it appears
in that table/descriptive list.
In `@src/cli/services/batch-processor.ts`:
- Around line 91-93: The thrown Error in the prompt-load failure path can leave
the CLI activity registered; update the prompt-loading code (the branch that
throws `Failed to load prompt "${id}"...`) so that after `registerCliActivity()`
has been called it always clears/unregisters the activity before propagating the
error—either by wrapping the load/register sequence in try/finally and calling
`unregisterCliActivity()` (or the existing CLI activity-clear function) in the
finally, or by calling that unregister function immediately before rethrowing
the Error; ensure this change covers the `runPlaybook()` flow so the desktop app
won't remain stuck as busy.
In `@src/main/index.ts`:
- Around line 395-405: The migration currently only checks for the header "##
Standing Instructions" and sets store.set(migratedKey, true) even when the
actual standingInstructions text wasn't copied; update the logic around
getPrompt('group-chat-moderator-system') and savePrompt(...) so you detect
whether the currentPrompt already contains the exact standingInstructions
content (not just the header) and only set the migratedKey after you
successfully insert or merge the standingInstructions; if the header exists but
the content differs, merge/replace the section with the legacy
standingInstructions and call await savePrompt('group-chat-moderator-system',
migratedPrompt) before calling store.set(migratedKey, true), using getPrompt,
savePrompt, standingInstructions, migratedKey and logger to locate and modify
the code paths.
- Around line 378-389: The catch block handling initializePrompts currently logs
and shows a dialog then quits without reporting to Sentry; import or require the
Sentry helper (captureException) and call await captureException(error, {
operation: 'startup:initializePrompts' }) inside that catch block before calling
app.quit() so the fatal error is sent to Sentry; update the catch that wraps
await initializePrompts() (the block that calls logger.error,
dialog.showErrorBox and app.quit) to await captureException(error, { operation:
'startup:initializePrompts' }) using the caught error variable.
In `@src/renderer/stores/settingsStore.ts`:
- Around line 71-79: The current logic in useSettingsStore (customAICommands /
commitCmd) overwrites a user's edited /commit prompt; before updating
customAICommands you must detect an existing override and migrate it into the
new prompt store (the "commit-command" prompt key) instead of clobbering it.
Concretely: in the block that reads useSettingsStore.getState().customAICommands
and finds commitCmd, if commitCmd.prompt differs from cachedCommitCommandPrompt
then write that existing commitCmd.prompt into the new prompt store (e.g., the
"commit-command" entry) and only then update customAICommands to ensure no
silent data loss; use the existing symbols commitCmd, cachedCommitCommandPrompt,
and useSettingsStore.setState to implement the migration atomically.
---
Duplicate comments:
In `@src/renderer/components/Settings/SettingsModal.tsx`:
- Around line 637-639: The Prompts tab mounts MaestroPromptsTab without the
searchable anchor wrapper that handleSearchNavigate expects; wrap the rendered
content for the 'prompts' branch in the same searchable anchor element (the
container that includes the data-setting-id attribute used elsewhere) so that
handleSearchNavigate can find the tab target; locate the conditional rendering
where activeTab === 'prompts' and ensure the outer element provides the correct
data-setting-id (matching the pattern other tabs use) and any CSS/role
attributes required for scroll/highlight behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 63469149-3d03-4cae-8350-d0f9ec2b37b9
📒 Files selected for processing (12)
docs/cli.mddocs/configuration.mdsrc/cli/services/batch-processor.tssrc/main/index.tssrc/renderer/components/AppStandaloneModals.tsxsrc/renderer/components/QuickActionsModal.tsxsrc/renderer/components/Settings/SettingsModal.tsxsrc/renderer/components/Settings/tabs/MaestroPromptsTab.tsxsrc/renderer/stores/modalStore.tssrc/renderer/stores/settingsStore.tssrc/renderer/types/index.tssrc/shared/promptDefinitions.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/renderer/components/Settings/tabs/MaestroPromptsTab.tsx
- src/shared/promptDefinitions.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/renderer/App.tsx (1)
3102-3105: Avoid double-reporting the same prompt-init exception.
initializeRendererPrompts()already reports to Sentry insrc/renderer/services/promptInit.ts:56-84before rethrowing. Capturing again here can duplicate alerts/events.Proposed adjustment
.catch((err) => { - captureException(err instanceof Error ? err : new Error(String(err)), { - extra: { context: 'MaestroConsole.initializeRendererPrompts' }, - }); + // Error is already captured in initializeRendererPrompts(). setPromptsReady(true); // Allow app to render; features degrade gracefully });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/App.tsx` around lines 3102 - 3105, The catch in App.tsx that calls captureException around initializeRendererPrompts is causing duplicate Sentry reports because initializeRendererPrompts (in src/renderer/services/promptInit.ts) already captures and rethrows the error; remove the captureException call in the catch block (or guard it so it only captures when the error wasn't already reported) and simply rethrow or handle the error to avoid double-reporting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/renderer/App.tsx`:
- Around line 3102-3105: The catch in App.tsx that calls captureException around
initializeRendererPrompts is causing duplicate Sentry reports because
initializeRendererPrompts (in src/renderer/services/promptInit.ts) already
captures and rethrows the error; remove the captureException call in the catch
block (or guard it so it only captures when the error wasn't already reported)
and simply rethrow or handle the error to avoid double-reporting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f74353da-5fe2-4d34-92c1-263d04fdf102
📒 Files selected for processing (2)
src/renderer/App.tsxsrc/renderer/services/promptInit.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/services/promptInit.ts
- Add arePromptsInitialized() guards to IPC save/reset handlers - Null sentinel pattern (string | null = null) across 9 renderer caches - Sentry reporting in MaestroPromptsTab, promptInit, App.tsx, main index - CONDUCTOR_PROFILE global regex at both router locations - CLI try/finally for unregisterCliActivity on generator throw - Standing instructions migration checks exact content not just header - /commit prompt migration preserves AI Commands user overrides - Test mock improvements: explicit error on unknown IDs - prompt-manager catch type narrowing (any → unknown) - CSS font-family quote removal, docs styling fixes - data-setting-id wrapper for settings search navigation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Author
|
@greptile please review again |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
src/renderer/services/contextSummarizer.ts (1)
52-57:⚠️ Potential issue | 🟠 MajorFail fast when the summarization prompt cache is uninitialized.
At Line 54, returning
''produces a valid-looking prompt with missing system instructions (used at Line 402), causing silent behavior drift instead of a clear failure.Proposed fix
function getContextSummarizePrompt(): string { - if (!contextSummarizerPromptsLoaded || cachedContextSummarizePrompt === null) { - return ''; + if (!contextSummarizerPromptsLoaded || !cachedContextSummarizePrompt?.trim()) { + throw new Error('Context summarizer prompts not loaded'); } return cachedContextSummarizePrompt; }Also applies to: 402-405
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/services/contextSummarizer.ts` around lines 52 - 57, The getContextSummarizePrompt function should fail fast instead of returning an empty string: replace the current conditional return with a thrown Error when contextSummarizerPromptsLoaded is false or cachedContextSummarizePrompt is null, e.g. throw new Error('Context summarization prompts not initialized'); update callers that expect a string (the consumer that uses getContextSummarizePrompt) to let the error propagate so missing system instructions cause an explicit failure rather than producing a silent empty prompt. Ensure you reference getContextSummarizePrompt, contextSummarizerPromptsLoaded and cachedContextSummarizePrompt when making the change.src/renderer/services/inlineWizardDocumentGeneration.ts (1)
44-56:⚠️ Potential issue | 🟠 MajorFail fast when prompts are not initialized; don’t silently return
''.Returning
''here still makes “unloaded” indistinguishable from an intentionally blank customization. That propagates into prompt selection at Line 406 and can run generation with no base template.Proposed fix
function getWizardDocumentGenerationPrompt(): string { - if (!inlineWizardDocGenPromptsLoaded || cachedWizardDocumentGenerationPrompt === null) { - return ''; - } + if (!inlineWizardDocGenPromptsLoaded || cachedWizardDocumentGenerationPrompt === null) { + throw new Error( + 'Inline wizard prompts not initialized. Call loadInlineWizardDocGenPrompts() before generation.' + ); + } return cachedWizardDocumentGenerationPrompt; } function getWizardInlineIterateGenerationPrompt(): string { - if (!inlineWizardDocGenPromptsLoaded || cachedWizardInlineIterateGenerationPrompt === null) { - return ''; - } + if (!inlineWizardDocGenPromptsLoaded || cachedWizardInlineIterateGenerationPrompt === null) { + throw new Error( + 'Inline wizard prompts not initialized. Call loadInlineWizardDocGenPrompts() before generation.' + ); + } return cachedWizardInlineIterateGenerationPrompt; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/services/inlineWizardDocumentGeneration.ts` around lines 44 - 56, The current getters silently return an empty string when prompts aren't loaded; change getWizardDocumentGenerationPrompt and getWizardInlineIterateGenerationPrompt to fail fast by throwing a clear Error (e.g., "Wizard prompts not initialized") when inlineWizardDocGenPromptsLoaded is false or the cached prompt is null, so callers cannot proceed with an unintended blank template; update any callers that expect a string to either catch the Error or ensure initialization runs before calling these getters.src/renderer/components/Wizard/services/wizardPrompts.ts (1)
40-52:⚠️ Potential issue | 🟠 MajorDon't collapse "unloaded" into an empty wizard template.
Both getters still return
''before load completes, sogenerateSystemPrompt()can silently drop the main wizard prompt and continuation block instead of surfacing the initialization problem.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/Wizard/services/wizardPrompts.ts` around lines 40 - 52, The getters getWizardSystemPrompt and getWizardSystemContinuationPrompt currently return an empty string when wizardPromptsLoaded is false, which allows generateSystemPrompt to silently proceed without the real prompts; change these functions to surface the uninitialized state instead of returning '' — for example return null (or throw a clear Error) when !wizardPromptsLoaded or the cached values are null, e.g. check wizardPromptsLoaded and cachedWizardSystemPrompt / cachedWizardSystemContinuationPrompt and return null (or throw) so callers like generateSystemPrompt can detect initialization failure and handle/log it appropriately.src/renderer/hooks/agent/useAgentListeners.ts (1)
67-71:⚠️ Potential issue | 🟠 MajorKeep the unloaded state observable.
Returning
''here still makes “prompt not loaded yet” indistinguishable from an intentional blank customization. TheonExitsynopsis flow later consumes this as a real base prompt, so a missed init silently generates history with no template instead of failing fast.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/agent/useAgentListeners.ts` around lines 67 - 71, getAutorunSynopsisPrompt currently hides the "not loaded" state by returning '' which is indistinguishable from an intentional empty prompt; change its contract to return null when prompts aren't loaded (use the existing agentListenersPromptsLoaded and cachedAutorunSynopsisPrompt to return null if not loaded or cache is null), update the function signature to string | null, and update all consumers (notably the onExit synopsis flow) to explicitly check for null and fail fast or log/throw instead of treating it as a real base prompt; ensure variables referenced (agentListenersPromptsLoaded, cachedAutorunSynopsisPrompt, getAutorunSynopsisPrompt, and the onExit synopsis consumer) are updated accordingly.src/renderer/hooks/wizard/useWizardHandlers.ts (1)
54-59:⚠️ Potential issue | 🟠 MajorDon't build
/historyfrom an ambiguous cache value.This getter still returns
''when the synopsis prompt has not been loaded.handleHistoryCommand()then composes a real synopsis request from that ambiguous value, which turns a missed init into a silent blank prompt.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/wizard/useWizardHandlers.ts` around lines 54 - 59, getAutorunSynopsisPrompt currently returns an ambiguous empty string when the synopsis hasn't been loaded; change it to return null (or undefined) to signal "not loaded" (update the function getAutorunSynopsisPrompt to return null instead of '' when wizardHandlersPromptsLoaded is false or cachedAutorunSynopsisPrompt is null) and then update callers (notably handleHistoryCommand) to check for null and abort/trigger init rather than composing a synopsis from an empty string. Ensure the unique symbol getAutorunSynopsisPrompt is modified and all usages (handleHistoryCommand) explicitly guard against a null return before building the /history prompt.src/renderer/stores/settingsStore.ts (1)
76-79:⚠️ Potential issue | 🟠 MajorAwait the
/commitmigration save.This fire-and-forget IPC can still be in flight, or fail, after
loadSettingsStorePrompts()resolves. That makes the migration non-deterministic and turns a save failure into silent data loss for the old/commitoverride. Await it and validate the result before considering prompt init complete.As per coding guidelines, "Let exceptions bubble up to Sentry for unhandled errors; only explicitly catch and handle expected/recoverable errors".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/stores/settingsStore.ts` around lines 76 - 79, The code currently fire-and-forgets window.maestro.prompts.save('commit-command', commitCmd.prompt) which can fail after loadSettingsStorePrompts() resolves; change it to await the promise from window.maestro.prompts.save inside the branch where commitCmd.prompt && !force, and only set cachedCommitCommandPrompt = commitCmd.prompt after the awaited save returns a successful result (validate the truthy/success value returned). Do not swallow errors—do not add try/catch—so any exception will bubble up (per guidelines); ensure the change is applied where commitCmd.prompt, force, window.maestro.prompts.save and cachedCommitCommandPrompt are referenced and that this update makes prompt initialization deterministic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli/services/batch-processor.ts`:
- Around line 33-58: getCustomizedPrompt and getCliPrompt currently treat an
empty string as “no customization” because they use truthy checks; update them
so that getCustomizedPrompt returns entry.content whenever entry?.isModified is
true (even if entry.content === ''), and change the caller in getCliPrompt to
check for a non-null/undefined value (e.g., customized !== null && customized
!== undefined) instead of if (customized) so an intentional blank override from
core-prompts-customizations.json is honored by the CLI.
In `@src/main/index.ts`:
- Around line 395-423: Wrap the migration disk write in a try/catch so a failing
savePrompt('group-chat-moderator-system', ...) does not block startup: call
getPrompt and build migratedPrompt as-is, then inside try { await
savePrompt(...); logger.info(...) } catch (err) { captureException(err, { extra:
{ migratedKey, standingInstructionsSlice: standingInstructions.slice(0,200) }
}); logger.warn('Failed to persist migrated moderator standing instructions,
continuing startup', 'Startup'); } finally ensure store.set(migratedKey, true)
still runs so we don't retry repeatedly; reference functions/idents: savePrompt,
getPrompt, store.set, migratedKey, standingInstructions, logger, and
captureException from your Sentry utils.
In `@src/main/prompt-manager.ts`:
- Around line 182-195: Concurrent read-modify-write in savePrompt and
resetPrompt can clobber changes to core-prompts-customizations.json; serialize
these mutations by adding a module-level mutex/queue and acquiring it around the
entire loadUserCustomizations -> modify -> saveUserCustomizations sequence.
Specifically, wrap the body of savePrompt (function savePrompt) and the
corresponding resetPrompt function (and any other functions that call
loadUserCustomizations/saveUserCustomizations) with lock acquisition and release
(or push operations to a single async FIFO) so only one IPC handler performs the
read-modify-write at a time, ensuring each modification is persisted without
races.
---
Duplicate comments:
In `@src/renderer/components/Wizard/services/wizardPrompts.ts`:
- Around line 40-52: The getters getWizardSystemPrompt and
getWizardSystemContinuationPrompt currently return an empty string when
wizardPromptsLoaded is false, which allows generateSystemPrompt to silently
proceed without the real prompts; change these functions to surface the
uninitialized state instead of returning '' — for example return null (or throw
a clear Error) when !wizardPromptsLoaded or the cached values are null, e.g.
check wizardPromptsLoaded and cachedWizardSystemPrompt /
cachedWizardSystemContinuationPrompt and return null (or throw) so callers like
generateSystemPrompt can detect initialization failure and handle/log it
appropriately.
In `@src/renderer/hooks/agent/useAgentListeners.ts`:
- Around line 67-71: getAutorunSynopsisPrompt currently hides the "not loaded"
state by returning '' which is indistinguishable from an intentional empty
prompt; change its contract to return null when prompts aren't loaded (use the
existing agentListenersPromptsLoaded and cachedAutorunSynopsisPrompt to return
null if not loaded or cache is null), update the function signature to string |
null, and update all consumers (notably the onExit synopsis flow) to explicitly
check for null and fail fast or log/throw instead of treating it as a real base
prompt; ensure variables referenced (agentListenersPromptsLoaded,
cachedAutorunSynopsisPrompt, getAutorunSynopsisPrompt, and the onExit synopsis
consumer) are updated accordingly.
In `@src/renderer/hooks/wizard/useWizardHandlers.ts`:
- Around line 54-59: getAutorunSynopsisPrompt currently returns an ambiguous
empty string when the synopsis hasn't been loaded; change it to return null (or
undefined) to signal "not loaded" (update the function getAutorunSynopsisPrompt
to return null instead of '' when wizardHandlersPromptsLoaded is false or
cachedAutorunSynopsisPrompt is null) and then update callers (notably
handleHistoryCommand) to check for null and abort/trigger init rather than
composing a synopsis from an empty string. Ensure the unique symbol
getAutorunSynopsisPrompt is modified and all usages (handleHistoryCommand)
explicitly guard against a null return before building the /history prompt.
In `@src/renderer/services/contextSummarizer.ts`:
- Around line 52-57: The getContextSummarizePrompt function should fail fast
instead of returning an empty string: replace the current conditional return
with a thrown Error when contextSummarizerPromptsLoaded is false or
cachedContextSummarizePrompt is null, e.g. throw new Error('Context
summarization prompts not initialized'); update callers that expect a string
(the consumer that uses getContextSummarizePrompt) to let the error propagate so
missing system instructions cause an explicit failure rather than producing a
silent empty prompt. Ensure you reference getContextSummarizePrompt,
contextSummarizerPromptsLoaded and cachedContextSummarizePrompt when making the
change.
In `@src/renderer/services/inlineWizardDocumentGeneration.ts`:
- Around line 44-56: The current getters silently return an empty string when
prompts aren't loaded; change getWizardDocumentGenerationPrompt and
getWizardInlineIterateGenerationPrompt to fail fast by throwing a clear Error
(e.g., "Wizard prompts not initialized") when inlineWizardDocGenPromptsLoaded is
false or the cached prompt is null, so callers cannot proceed with an unintended
blank template; update any callers that expect a string to either catch the
Error or ensure initialization runs before calling these getters.
In `@src/renderer/stores/settingsStore.ts`:
- Around line 76-79: The code currently fire-and-forgets
window.maestro.prompts.save('commit-command', commitCmd.prompt) which can fail
after loadSettingsStorePrompts() resolves; change it to await the promise from
window.maestro.prompts.save inside the branch where commitCmd.prompt && !force,
and only set cachedCommitCommandPrompt = commitCmd.prompt after the awaited save
returns a successful result (validate the truthy/success value returned). Do not
swallow errors—do not add try/catch—so any exception will bubble up (per
guidelines); ensure the change is applied where commitCmd.prompt, force,
window.maestro.prompts.save and cachedCommitCommandPrompt are referenced and
that this update makes prompt initialization deterministic.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 467073e6-f14d-4864-aedb-6380d6d65abf
📒 Files selected for processing (24)
docs/cli.mddocs/configuration.mdsrc/__tests__/main/group-chat/group-chat-agent.test.tssrc/__tests__/main/group-chat/group-chat-router.test.tssrc/__tests__/renderer/stores/agentStore.test.tssrc/cli/services/batch-processor.tssrc/main/group-chat/group-chat-router.tssrc/main/index.tssrc/main/ipc/handlers/prompts.tssrc/main/prompt-manager.tssrc/renderer/components/Settings/SettingsModal.tsxsrc/renderer/components/Settings/tabs/MaestroPromptsTab.csssrc/renderer/components/Settings/tabs/MaestroPromptsTab.tsxsrc/renderer/components/Wizard/services/phaseGenerator.tssrc/renderer/components/Wizard/services/wizardPrompts.tssrc/renderer/hooks/agent/useAgentListeners.tssrc/renderer/hooks/agent/useMergeTransferHandlers.tssrc/renderer/hooks/wizard/useWizardHandlers.tssrc/renderer/services/contextGroomer.tssrc/renderer/services/contextSummarizer.tssrc/renderer/services/inlineWizardConversation.tssrc/renderer/services/inlineWizardDocumentGeneration.tssrc/renderer/stores/settingsStore.tssrc/shared/promptDefinitions.ts
✅ Files skipped from review due to trivial changes (4)
- src/tests/main/group-chat/group-chat-agent.test.ts
- docs/configuration.md
- src/renderer/components/Settings/tabs/MaestroPromptsTab.css
- src/shared/promptDefinitions.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/tests/main/group-chat/group-chat-router.test.ts
- src/renderer/components/Wizard/services/phaseGenerator.ts
- src/renderer/services/contextGroomer.ts
- src/main/ipc/handlers/prompts.ts
- src/renderer/components/Settings/tabs/MaestroPromptsTab.tsx
- src/main/group-chat/group-chat-router.ts
- docs/cli.md
…y override - Serialize prompt-manager disk writes with a write lock to prevent concurrent read-modify-write races on customizations JSON - Wrap standing instructions migration in try/catch with Sentry reporting so a failing savePrompt does not block startup - Honor intentionally blank prompt customizations in CLI (typeof check instead of truthy, null check instead of truthy on caller) - Await prompts.save in settingsStore commit migration and only update cache on success Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
External CI pipelines may still reference build:prompts. Keep it as a no-op echo so those pipelines don't fail after the generate-prompts build step was removed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/renderer/stores/settingsStore.ts (1)
70-88:⚠️ Potential issue | 🟠 MajorThis still drops a legitimately blank legacy
/commitoverride.The migration decision is based on the live Zustand value, but this file also documents that the bootstrap
/commitprompt is''before settings load. That means an intentionally blank saved override is indistinguishable from the bootstrap placeholder here, so Lines 76-88 overwrite it with the bundled prompt instead of preserving it. This migration needs to read the persistedcustomAICommandssource directly, or wait untilsettingsLoadedis true before deciding whether blank means “custom” or “not loaded yet.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/stores/settingsStore.ts` around lines 70 - 88, The migration currently runs against the in-memory Zustand placeholder and can overwrite an intentionally blank legacy commit prompt; change the logic in the block referencing currentCommands, commitCmd and cachedCommitCommandPrompt so it only performs migration after settings have been loaded (check useSettingsStore.getState().settingsLoaded) or if the live commitCmd.prompt is non-empty, thereby treating an empty string as “not ready” until settingsLoaded is true; if you prefer reading persisted state directly, fetch the persisted customAICommands from the persistence API before deciding to call window.maestro.prompts.save and updating useSettingsStore.setState so blank saved overrides are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/index.ts`:
- Around line 399-428: The code currently calls store.set(migratedKey, true)
unconditionally, marking moderatorStandingInstructionsMigrated even on failures;
change this so store.set(migratedKey, true) is only executed after a successful
migration path (i.e., after savePrompt completes or after detecting that no
change is needed because currentPrompt already contains standingInstructions).
Move the store.set(...) inside the try block just after await savePrompt(...) or
right after the branch that skips migration, and do not set it in the catch
handler; reference getPrompt, savePrompt, standingInstructions, migratedKey, and
store.set to locate and update the logic.
In `@src/main/prompt-manager.ts`:
- Around line 80-100: loadUserCustomizations currently throws on malformed JSON
which blocks startup; change it so that when read/parse fails for reasons other
than ENOENT (e.g., JSON.parse error, permission issues) you quarantine the bad
file (rename getCustomizationsPath() to a .broken.TIMESTAMP or similar using
fs.rename) and log the error with logger.error and LOG_CONTEXT, then return null
so initializePrompts can fall back to defaults; for saveUserCustomizations make
writes atomic by writing JSON to a temporary path (e.g., getCustomizationsPath()
+ '.tmp.' + timestamp or PID) and then fs.rename that temp file to the real path
(ensuring the rename replaces the target atomically), and surface real write
failures but avoid in-place truncation that can leave a corrupt file.
In `@src/renderer/stores/settingsStore.ts`:
- Around line 55-81: The code sets settingsStorePromptsLoaded and assigns
DEFAULT_AI_COMMANDS (using cachedCommitCommandPrompt) before performing the
legacy migration that can change cachedCommitCommandPrompt; move the
creation/assignment of DEFAULT_AI_COMMANDS and the settingsStorePromptsLoaded
flag to after the migration completes and succeeds so the exported default
reflects the final prompt, and when calling
window.maestro.prompts.save('commit-command', ...) propagate unexpected errors
(don’t swallow them) while only handling expected/recoverable failures
explicitly; update references to useSettingsStore.getState().customAICommands
and the commitCmd migration logic to run before finalizing DEFAULT_AI_COMMANDS
and flipping settingsStorePromptsLoaded.
---
Duplicate comments:
In `@src/renderer/stores/settingsStore.ts`:
- Around line 70-88: The migration currently runs against the in-memory Zustand
placeholder and can overwrite an intentionally blank legacy commit prompt;
change the logic in the block referencing currentCommands, commitCmd and
cachedCommitCommandPrompt so it only performs migration after settings have been
loaded (check useSettingsStore.getState().settingsLoaded) or if the live
commitCmd.prompt is non-empty, thereby treating an empty string as “not ready”
until settingsLoaded is true; if you prefer reading persisted state directly,
fetch the persisted customAICommands from the persistence API before deciding to
call window.maestro.prompts.save and updating useSettingsStore.setState so blank
saved overrides are preserved.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5647fcef-cb29-4f30-b0a5-2e621fce98c3
📒 Files selected for processing (5)
package.jsonsrc/cli/services/batch-processor.tssrc/main/index.tssrc/main/prompt-manager.tssrc/renderer/stores/settingsStore.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/cli/services/batch-processor.ts
5 tasks
- Move store.set(migratedKey, true) inside try block so standing instructions migration is retried on next launch if savePrompt fails - Move DEFAULT_AI_COMMANDS assignment and settingsStorePromptsLoaded flag after legacy migration completes so the exported default reflects the final prompt value post-migration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Implementation of #197, after merging this look at #804
Maestro Prompts: Full Control Over Your AI System Prompts
All 23 core system prompts are now fully customizable from a new Maestro Prompts tab in Settings. Edit wizard, auto run, group chat, context management, commit command, and other prompts — changes take effect immediately, no restart needed.
What's new
Settings Tab — A split-view browser/editor organized by category (Wizard, Inline Wizard, Auto Run, Group Chat, Context, Commands, System). Modified prompts show an orange indicator dot. Save and Reset to Default with instant effect.
Command Palette — The 4 most-used prompts are accessible directly from Quick Actions (
Cmd+K): Maestro System Prompt, Auto Run Default, Commit Command, and Group Chat Moderator. Configurable viaQUICK_ACTION_PROMPTSinpromptDefinitions.ts.CLI Support —
maestro-cliplaybook runs now respect user prompt customizations. The CLI reads the samecore-prompts-customizations.jsonthat the desktop app writes, so edits made in the UI carry over to batch execution.Architecture — Prompts are loaded from disk at startup instead of being compiled into the app bundle. User customizations are stored separately and survive app updates. Bundled defaults can always be restored with one click.
Migration notes
group-chat-moderator-systemprompt on first launch.build:promptsstep andscripts/generate-prompts.mjsare removed. Thesrc/generated/directory no longer exists.src/prompts/index.tsnow re-exports shared definitions fromsrc/shared/promptDefinitions.tsinstead of compiled constants.Files created
src/shared/promptDefinitions.ts— single source of truth for 23 prompt IDs, filenames, categoriessrc/main/prompt-manager.ts— disk loader with customization overlaysrc/main/ipc/handlers/prompts.ts— IPC handlers (get, getAll, save, reset)src/main/preload/prompts.ts— preload API factorysrc/renderer/services/promptInit.ts— centralized renderer init (13 parallel loaders)src/renderer/components/Settings/tabs/MaestroPromptsTab.tsx+.cssTesting
Summary by CodeRabbit
New Features
Behavior Changes
Documentation