Skip to content

Autorun settings#776

Closed
scriptease wants to merge 2 commits intoRunMaestro:rcfrom
scriptease:autorun-settings
Closed

Autorun settings#776
scriptease wants to merge 2 commits intoRunMaestro:rcfrom
scriptease:autorun-settings

Conversation

@scriptease
Copy link
Copy Markdown

@scriptease scriptease commented Apr 9, 2026

Bildschirmfoto 2026-04-09 um 21 38 12 Bildschirmfoto 2026-04-09 um 21 38 07

Overwrite the default autorun prompt for all future agents and agents that don't have a customized prompt already.

My use case it removing the default push to github which takes 10 minutes for some projects, I am looking at you Maestro ;-)

Summary by CodeRabbit

  • New Features

    • Added an "Auto Run" Settings tab to enable, edit, and persist a custom default batch-run prompt with validation.
  • Improvements

    • Batch Runner, auto-start runs, and wizard/playbook flows now use the computed default prompt from settings (so customized prompts are respected everywhere).

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 9, 2026

📝 Walkthrough

Walkthrough

Replaces static DEFAULT_BATCH_PROMPT with a runtime getEffectiveAutoRunPrompt() that returns a trimmed settings override or the built-in default; adds persisted autoRunDefaultPromptOverride to settings and a new SettingsModal "autorun" tab to manage the override.

Changes

Cohort / File(s) Summary
Settings Store & Hooks
src/renderer/stores/settingsStore.ts, src/renderer/hooks/settings/useSettings.ts
Added persisted autoRunDefaultPromptOverride state with setter and hydration; exposed autoRunDefaultPromptOverride and setAutoRunDefaultPromptOverride via useSettings.
Settings UI
src/renderer/components/Settings/SettingsModal.tsx
Added 'autorun' tab: toggle to enable/disable override, editable textarea for override, read-only built-in default display, and validation warning when override lacks a task reference.
Batch Utilities
src/renderer/hooks/batch/batchUtils.ts, src/renderer/hooks/batch/index.ts
Added getEffectiveAutoRunPrompt() that returns trimmed override from settings or the built-in default; re-exported from batch barrel.
Component & Hook Updates
src/renderer/components/BatchRunnerModal.tsx, src/renderer/hooks/batch/usePlaybookManagement.ts, src/renderer/hooks/symphony/useSymphonyContribution.ts, src/renderer/hooks/wizard/useWizardHandlers.ts
Switched internal prompt initialization, comparisons, resets, and auto-start batch prompt population to use getEffectiveAutoRunPrompt() instead of DEFAULT_BATCH_PROMPT.
Test Mocks
src/__tests__/renderer/hooks/useSymphonyContribution.test.ts, src/__tests__/renderer/hooks/useWizardHandlers.test.ts
Replaced mocks that stubbed DEFAULT_BATCH_PROMPT with mocks of renderer/hooks/batch/batchUtils that preserve real exports and override getEffectiveAutoRunPrompt() to supply test prompt values.

Sequence Diagram

sequenceDiagram
    participant User
    participant SettingsModal
    participant SettingsStore
    participant getEffectiveAutoRunPrompt
    participant BatchRunnerModal

    User->>SettingsModal: Toggle/set auto-run prompt override
    SettingsModal->>SettingsStore: setAutoRunDefaultPromptOverride(customPrompt)
    SettingsStore->>SettingsStore: Persist to window.maestro.settings

    Note over User,BatchRunnerModal: When BatchRunnerModal initializes
    BatchRunnerModal->>getEffectiveAutoRunPrompt: getEffectiveAutoRunPrompt()
    getEffectiveAutoRunPrompt->>SettingsStore: Read autoRunDefaultPromptOverride
    alt Override set (non-empty)
        getEffectiveAutoRunPrompt-->>BatchRunnerModal: Return trimmed override
    else
        getEffectiveAutoRunPrompt-->>BatchRunnerModal: Return built-in default
    end
    BatchRunnerModal->>BatchRunnerModal: Initialize/reset prompt to effective value
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested labels

RC

Suggested reviewers

  • pedramamini

Poem

🐰 I hopped in code to make prompts bend,
From fixed to flexible—toward each end.
An override nestles where defaults once lay,
Users shape runs in a brighter way. ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 52.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Autorun settings' is generic and vague, failing to convey the specific implementation detail that this PR adds a customizable default auto-run prompt override feature. Consider a more descriptive title such as 'Add customizable default auto-run prompt override' or 'Allow users to override default batch run prompt' to better reflect the changeset's primary purpose.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Add a new Auto Run tab in Settings with a toggle switch to customize
the default prompt used for all Auto Run / Playbook executions. When
off, the built-in default prompt is shown read-only. When on, the
prompt becomes editable and the override applies globally to any
execution that does not have a per-playbook custom prompt.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@scriptease scriptease marked this pull request as draft April 9, 2026 19:54
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 9, 2026

Greptile Summary

This PR adds a global Auto Run default prompt override in Settings, allowing users to replace the built-in autorunDefaultPrompt for all batch runs that don't have a per-playbook prompt. A new "Auto Run" tab is added to SettingsModal, getEffectiveAutoRunPrompt() is introduced in batchUtils.ts to centralize the override/fallback logic, and all relevant execution paths (wizard, symphony, playbook loading) are updated to consume it.

Confidence Score: 5/5

Safe to merge — feature is well-contained, all execution paths use the centralised getEffectiveAutoRunPrompt() helper, and previous reviewer concerns have been addressed.

All remaining findings are P2 (whitespace-only edge case in the toggle UI). The core feature — persisting a global override and applying it across wizard, symphony, and playbook flows — is implemented correctly. Tests are updated and the validation warning from the prior review thread is now present.

No files require special attention.

Vulnerabilities

No security concerns identified. The override is stored as a plain string in the user's local settings file and is only used as an AI prompt, not executed as code. No new IPC surfaces, network calls, or privilege escalations are introduced.

Important Files Changed

Filename Overview
src/renderer/components/Settings/SettingsModal.tsx Adds new "Auto Run" tab with toggle + textarea for editing the global prompt override; includes task-reference validation warning.
src/renderer/hooks/batch/batchUtils.ts Adds getEffectiveAutoRunPrompt() (reads override with trim/fallback) and validateAgentPromptHasTaskReference() — both are clean and correct.
src/renderer/stores/settingsStore.ts Adds autoRunDefaultPromptOverride field and its setter; pattern is consistent with other settings in the store.
src/renderer/hooks/batch/usePlaybookManagement.ts Uses getEffectiveAutoRunPrompt() as fallback when loading a playbook with no saved prompt — correct behavior.
src/renderer/hooks/symphony/useSymphonyContribution.ts Switched from hardcoded default to getEffectiveAutoRunPrompt() for batch run config prompt.
src/renderer/hooks/wizard/useWizardHandlers.ts Switched from hardcoded default to getEffectiveAutoRunPrompt() for wizard-launched batch run config.
src/tests/renderer/hooks/useSymphonyContribution.test.ts Mocks getEffectiveAutoRunPrompt to decouple test from settings store — correct test isolation.
src/tests/renderer/hooks/useWizardHandlers.test.ts Mocks getEffectiveAutoRunPrompt for test isolation — correct approach.
src/renderer/hooks/settings/useSettings.ts Adds autoRunDefaultPromptOverride / setAutoRunDefaultPromptOverride to the UseSettingsReturn interface — clean addition.
src/renderer/components/BatchRunnerModal.tsx Uses getEffectiveAutoRunPrompt() as the initial prompt default when no per-session prompt exists — correct integration.
src/renderer/hooks/batch/index.ts Exports new utilities (getEffectiveAutoRunPrompt, validateAgentPromptHasTaskReference) from the batch module.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([User opens Settings → Auto Run tab]) --> B{Override toggle on?}
    B -- "No (autoRunDefaultPromptOverride = '')" --> C[Read-only textarea\nshows built-in default]
    B -- "Yes (non-empty string)" --> D[Editable textarea\nshows override value]
    D --> E{validateAgentPromptHasTaskReference?}
    E -- false --> F[Warning shown:\n'prompt does not reference\nMarkdown tasks']
    E -- true --> G[No warning]

    H([Batch run triggered]) --> I[getEffectiveAutoRunPrompt]
    I --> J{autoRunDefaultPromptOverride\n.trim non-empty?}
    J -- Yes --> K[Use override prompt]
    J -- No --> L[Use built-in autorunDefaultPrompt]

    K --> M([BatchRunnerModal / Wizard / Symphony / Playbook])
    L --> M

    subgraph "Per-session priority"
        N{activeSession.batchRunnerPrompt\nnon-empty?} -- Yes --> O[Use per-session prompt]
        N -- No --> I
    end
Loading

Reviews (2): Last reviewed commit: "fix: add task reference validation warni..." | Re-trigger Greptile

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (2)
scripts/showcase/seed/data/maestro-groups.json (1)

5-5: Consider using Title Case for group names.

The group names are in ALL CAPS ("BOOKMARKS", "PROJECTS"), which may not reflect typical user usage patterns. For more realistic seed data, consider Title Case (e.g., "Bookmarks", "Projects") unless the ALL CAPS styling is intentional for the showcase.

📝 Suggested naming refinement
 		{
 			"id": "00000000-0000-4000-a000-000000000001",
-			"name": "BOOKMARKS",
+			"name": "Bookmarks",
 			"emoji": "⭐",
 			"collapsed": false
 		},
 		{
 			"id": "00000000-0000-4000-a000-000000000002",
-			"name": "PROJECTS",
+			"name": "Projects",
 			"emoji": "📁",
 			"collapsed": false
 		}

Also applies to: 11-11

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/showcase/seed/data/maestro-groups.json` at line 5, The group names in
the seed JSON are written in ALL CAPS (e.g., "BOOKMARKS", "PROJECTS"); update
these values to Title Case (e.g., "Bookmarks", "Projects") in the JSON array so
seeded data reads more realistic for users—locate the string values for the
group name keys (the "name" fields containing "BOOKMARKS" and "PROJECTS") and
replace them with their Title Case equivalents consistently across the file.
src/renderer/components/BatchRunnerModal.tsx (1)

156-164: Potential stale state after global setting change.

effectiveDefault is computed once when the component renders. If the user modifies the global Auto Run prompt in Settings while this modal is open, then returns and clicks "Reset", the prompt would reset to the new default (line 357 calls getEffectiveAutoRunPrompt() fresh), but isModified and hasUnsavedChanges (lines 409-410) would still compare against the old effectiveDefault captured at mount.

This is a minor edge case (requires both modals open), but could cause confusing UI state where Reset appears to work but badges/buttons don't update correctly.

♻️ Optional: Use a getter function for consistent behavior
-	const effectiveDefault = getEffectiveAutoRunPrompt();
-	const [prompt, setPrompt] = useState(initialPrompt || effectiveDefault);
+	const [prompt, setPrompt] = useState(initialPrompt || getEffectiveAutoRunPrompt());
 	const [variablesExpanded, setVariablesExpanded] = useState(false);
 	const [savedPrompt, setSavedPrompt] = useState(initialPrompt || '');
 	const [promptComposerOpen, setPromptComposerOpen] = useState(false);
 	const textareaRef = useRef<HTMLTextAreaElement>(null);

 	// Track initial prompt for dirty checking
-	const initialPromptRef = useRef(initialPrompt || effectiveDefault);
+	const initialPromptRef = useRef(initialPrompt || getEffectiveAutoRunPrompt());

Then update comparisons to call the function:

-	const isModified = prompt !== effectiveDefault;
-	const hasUnsavedChanges = prompt !== savedPrompt && prompt !== effectiveDefault;
+	const effectiveDefault = getEffectiveAutoRunPrompt();
+	const isModified = prompt !== effectiveDefault;
+	const hasUnsavedChanges = prompt !== savedPrompt && prompt !== effectiveDefault;

Moving effectiveDefault closer to usage ensures fresh values for comparisons while keeping initialization stable.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/components/BatchRunnerModal.tsx` around lines 156 - 164, The
component captures effectiveDefault once at mount which can become stale if the
global Auto Run prompt changes while the modal is open; update checks and
initial state to use a fresh value by replacing the cached constant with a
getter that calls getEffectiveAutoRunPrompt() when needed (or call
getEffectiveAutoRunPrompt() inline in comparisons), and ensure initialPromptRef
is initialized consistently (e.g., set initialPromptRef.current = initialPrompt
|| getEffectiveAutoRunPrompt() at mount) so isModified and hasUnsavedChanges
(and the Reset handling) always compare against the current effective default;
update references to effectiveDefault in functions that compute
isModified/hasUnsavedChanges/reset logic to call the getter instead of using the
stale effectiveDefault variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/showcase/generate-seed.js`:
- Around line 1016-1048: The settingsData object currently hardcodes
defaultShell: 'bash' which breaks Windows showcases; remove the defaultShell
property from settingsData in generate-seed.js (or, alternatively, call the
platform-aware getDefaultShell() from src/main/stores/defaults.ts if you prefer
to derive it at seed-time) so the seeded settings match the checked-in
maestro-settings.json and platform defaults; after changing the settingsData
object, regenerate the seed file so the showcase uses the platform-safe default
shell.

In `@scripts/showcase/launch.js`:
- Around line 25-29: The spawn call hardcodes MAESTRO_DEMO_DIR to
'/tmp/maestro-showcase', which breaks on Windows and duplicates the path in
scripts/showcase/setup.js; change the env value in the spawn invocation to a
platform-neutral directory built with require('os').tmpdir() and path.join
(e.g., path.join(os.tmpdir(), 'maestro-showcase')), and refactor so both
launch.js (the spawn env) and scripts/showcase/setup.js derive that same path
from a shared helper or the same computation (MAESTRO_DEMO_DIR constant) to
avoid drift; update the spawn invocation’s env to use that computed value
instead of the hardcoded string.

In `@scripts/showcase/plan.md`:
- Around line 148-153: Update the fenced code blocks that currently start with
``` to include a language tag (use text or markdown) so markdownlint stops
flagging them; specifically add a language to the fence containing the snippet
with "scripts/showcase/seed/data/" and the following "/tmp/maestro-showcase/"
block, and also apply the same change to the other fenced block between lines
235–259 that shows the showcase file layout. Make sure each opening
triple-backtick becomes ```text or ```markdown for those blocks.

In `@scripts/showcase/setup.js`:
- Around line 55-63: The current replaceInFile function performs raw string
replacements which corrupt JSON on Windows (unescaped backslashes like in a $CWD
path); change the logic to detect JSON files or attempt JSON.parse: if the file
is JSON or parses successfully, load the object, update only the specific fields
that contain the search token (traverse keys or check property values), then
write back using JSON.stringify(obj, null, 2) to preserve valid escaping; if
it's not JSON, fall back to the existing string replaceAll behavior. Apply the
same fix for the similar replacement logic referenced around the other block
(lines 97-110) so JSON values are always written safely.

In `@src/renderer/hooks/batch/batchUtils.ts`:
- Around line 16-19: In getEffectiveAutoRunPrompt, the override value from
useSettingsStore.getState().autoRunDefaultPromptOverride should treat
whitespace-only strings as empty; change the logic to trim the override (e.g.,
const override =
useSettingsStore.getState().autoRunDefaultPromptOverride?.trim()) and then
return override if non-empty, otherwise return autorunDefaultPrompt so
whitespace-only overrides no longer produce a blank prompt.

In `@src/renderer/hooks/groupChat/useGroupChatHandlers.ts`:
- Around line 320-323: The current logic treats 'agent-working' as moderatorFree
and can dispatch a new moderator turn while previous turn participants are still
running, which can overwrite the single pendingParticipantResponses set in the
main process; update the drain/dispatch checks so only groupChatState === 'idle'
is considered moderator-free (remove 'agent-working' from moderatorFree) in the
block using groupChatExecutionQueue and in the analogous block around the other
dispatch (the code handling lines ~630-646), ensuring new turns are not started
until the main process is made turn-aware and the previous
pendingParticipantResponses set is settled.

---

Nitpick comments:
In `@scripts/showcase/seed/data/maestro-groups.json`:
- Line 5: The group names in the seed JSON are written in ALL CAPS (e.g.,
"BOOKMARKS", "PROJECTS"); update these values to Title Case (e.g., "Bookmarks",
"Projects") in the JSON array so seeded data reads more realistic for
users—locate the string values for the group name keys (the "name" fields
containing "BOOKMARKS" and "PROJECTS") and replace them with their Title Case
equivalents consistently across the file.

In `@src/renderer/components/BatchRunnerModal.tsx`:
- Around line 156-164: The component captures effectiveDefault once at mount
which can become stale if the global Auto Run prompt changes while the modal is
open; update checks and initial state to use a fresh value by replacing the
cached constant with a getter that calls getEffectiveAutoRunPrompt() when needed
(or call getEffectiveAutoRunPrompt() inline in comparisons), and ensure
initialPromptRef is initialized consistently (e.g., set initialPromptRef.current
= initialPrompt || getEffectiveAutoRunPrompt() at mount) so isModified and
hasUnsavedChanges (and the Reset handling) always compare against the current
effective default; update references to effectiveDefault in functions that
compute isModified/hasUnsavedChanges/reset logic to call the getter instead of
using the stale effectiveDefault variable.
🪄 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: 0d2c4343-3c4c-4960-8d03-23f9b70bbeca

📥 Commits

Reviewing files that changed from the base of the PR and between b97da37 and 09807ab.

⛔ Files ignored due to path filters (1)
  • scripts/showcase/screenshot.png is excluded by !**/*.png
📒 Files selected for processing (28)
  • CONTRIBUTING.md
  • THEMES.md
  • package.json
  • scripts/showcase/generate-seed.js
  • scripts/showcase/launch.js
  • scripts/showcase/plan.md
  • scripts/showcase/seed/data/group-chats/00000000-0000-4000-a000-00000000005b/metadata.json
  • scripts/showcase/seed/data/maestro-agent-session-origins.json
  • scripts/showcase/seed/data/maestro-claude-session-origins.json
  • scripts/showcase/seed/data/maestro-groups.json
  • scripts/showcase/seed/data/maestro-sessions.json
  • scripts/showcase/seed/data/maestro-settings.json
  • scripts/showcase/seed/data/maestro-window-state.json
  • scripts/showcase/setup.js
  • src/__tests__/renderer/hooks/useSymphonyContribution.test.ts
  • src/__tests__/renderer/hooks/useWizardHandlers.test.ts
  • src/main/group-chat/group-chat-router.ts
  • src/prompts/group-chat-participant-request.md
  • src/renderer/components/BatchRunnerModal.tsx
  • src/renderer/components/Settings/SettingsModal.tsx
  • src/renderer/hooks/batch/batchUtils.ts
  • src/renderer/hooks/batch/index.ts
  • src/renderer/hooks/batch/usePlaybookManagement.ts
  • src/renderer/hooks/groupChat/useGroupChatHandlers.ts
  • src/renderer/hooks/settings/useSettings.ts
  • src/renderer/hooks/symphony/useSymphonyContribution.ts
  • src/renderer/hooks/wizard/useWizardHandlers.ts
  • src/renderer/stores/settingsStore.ts

@scriptease scriptease marked this pull request as ready for review April 9, 2026 20:09
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 9, 2026

Tip:

Greploop — Automatically fix all review issues by running /greploops in Claude Code. It iterates: fix, push, re-review, repeat until 5/5 confidence.

Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/renderer/components/Settings/SettingsModal.tsx`:
- Around line 695-699: The toggle logic uses raw string truthiness for
autoRunDefaultPromptOverride (and the other similar override states) but runtime
uses trimmed values, so replace truthy checks with trimmed checks (e.g., use
autoRunDefaultPromptOverride?.trim() to decide enabled/disabled) and when
toggling set the state to either '' or autorunDefaultPrompt.trim() (or the
corresponding default trimmed value) via setAutoRunDefaultPromptOverride so
whitespace-only strings are treated as empty; apply the same change to the other
override state setters and checks in this component to ensure consistent
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: b2998f84-1013-42bf-8f18-4bca25c61608

📥 Commits

Reviewing files that changed from the base of the PR and between 09807ab and 6d7bc41.

📒 Files selected for processing (11)
  • src/__tests__/renderer/hooks/useSymphonyContribution.test.ts
  • src/__tests__/renderer/hooks/useWizardHandlers.test.ts
  • src/renderer/components/BatchRunnerModal.tsx
  • src/renderer/components/Settings/SettingsModal.tsx
  • src/renderer/hooks/batch/batchUtils.ts
  • src/renderer/hooks/batch/index.ts
  • src/renderer/hooks/batch/usePlaybookManagement.ts
  • src/renderer/hooks/settings/useSettings.ts
  • src/renderer/hooks/symphony/useSymphonyContribution.ts
  • src/renderer/hooks/wizard/useWizardHandlers.ts
  • src/renderer/stores/settingsStore.ts
✅ Files skipped from review due to trivial changes (1)
  • src/renderer/hooks/symphony/useSymphonyContribution.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/tests/renderer/hooks/useWizardHandlers.test.ts
  • src/tests/renderer/hooks/useSymphonyContribution.test.ts
  • src/renderer/hooks/batch/usePlaybookManagement.ts
  • src/renderer/hooks/batch/batchUtils.ts
  • src/renderer/components/BatchRunnerModal.tsx

…verrides

Show a warning in the Auto Run settings tab when the custom prompt does
not reference Markdown tasks, matching the existing BatchRunnerModal
validation. Also trim the override value so whitespace-only strings
fall back to the built-in default instead of producing a blank prompt.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/renderer/components/Settings/SettingsModal.tsx (1)

211-252: Consolidate tab order into one source of truth

Line 212-252 duplicates tab ordering that is already represented in TAB_ITEMS (Line 385-403). Keeping both lists in sync is easy to miss on future tab additions/removals.

Refactor sketch
+ const TAB_ITEMS: Array<...> = [ ... ];
+ const tabOrder = TAB_ITEMS.map((tab) => tab.id);

 useEffect(() => {
   if (!isOpen) return;

   const handleTabNavigation = (e: KeyboardEvent) => {
-    const tabs: Array<...> = FEATURE_FLAGS.LLM_SETTINGS ? [...] : [...];
-    const currentIndex = tabs.indexOf(activeTab);
+    const currentIndex = tabOrder.indexOf(activeTab);

     if ((e.metaKey || e.ctrlKey) && e.shiftKey && e.key === '[') {
       e.preventDefault();
-      const prevIndex = currentIndex === 0 ? tabs.length - 1 : currentIndex - 1;
-      setActiveTab(tabs[prevIndex]);
+      const prevIndex = currentIndex === 0 ? tabOrder.length - 1 : currentIndex - 1;
+      setActiveTab(tabOrder[prevIndex]);
     } else if ((e.metaKey || e.ctrlKey) && e.shiftKey && e.key === ']') {
       e.preventDefault();
-      const nextIndex = (currentIndex + 1) % tabs.length;
-      setActiveTab(tabs[nextIndex]);
+      const nextIndex = (currentIndex + 1) % tabOrder.length;
+      setActiveTab(tabOrder[nextIndex]);
     }
   };

Also applies to: 385-403

🤖 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 211 - 252,
The duplicated tab list in handleTabNavigation should be replaced with a single
source of truth derived from TAB_ITEMS: compute the tabs array by mapping
TAB_ITEMS to their tab keys/ids (e.g., TAB_ITEMS.map(item => item.key) or
item.id depending on TAB_ITEMS shape) and then conditionally filter out 'llm'
when FEATURE_FLAGS.LLM_SETTINGS is false; update handleTabNavigation to use that
derived tabs array so order is maintained in one place and future tab changes
only require edits to TAB_ITEMS.
🤖 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/components/Settings/SettingsModal.tsx`:
- Around line 211-252: The duplicated tab list in handleTabNavigation should be
replaced with a single source of truth derived from TAB_ITEMS: compute the tabs
array by mapping TAB_ITEMS to their tab keys/ids (e.g., TAB_ITEMS.map(item =>
item.key) or item.id depending on TAB_ITEMS shape) and then conditionally filter
out 'llm' when FEATURE_FLAGS.LLM_SETTINGS is false; update handleTabNavigation
to use that derived tabs array so order is maintained in one place and future
tab changes only require edits to TAB_ITEMS.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e4d1ea2a-fe09-4daa-8083-8ba92d7f97ba

📥 Commits

Reviewing files that changed from the base of the PR and between 6d7bc41 and 8e3ec13.

📒 Files selected for processing (2)
  • src/renderer/components/Settings/SettingsModal.tsx
  • src/renderer/hooks/batch/batchUtils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/renderer/hooks/batch/batchUtils.ts

@pedramamini
Copy link
Copy Markdown
Collaborator

@scriptease this is neat; however, we have a pending PR that will make all of the prompts in Maestro editable, so that would supersede this one.

@pedramamini pedramamini self-assigned this Apr 11, 2026
@pedramamini
Copy link
Copy Markdown
Collaborator

Side note: if you change the auto-run prompt, it does persist for that agent. still changing the default is valuable of course.

@scriptease
Copy link
Copy Markdown
Author

This will return as part of: #197

@scriptease scriptease closed this Apr 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants