fix: enforce concurrency limit for manual feature execution#622
fix: enforce concurrency limit for manual feature execution#622ScotTFO wants to merge 2 commits intoAutoMaker-Org:mainfrom
Conversation
Add concurrency check at the start of executeFeature() so manual "Run" clicks are subject to the same maxConcurrency limit as auto-mode. This prevents users from exceeding the configured limit by clicking Run multiple times. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @ScotTFO, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue where manual feature executions could bypass the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughA concurrency control mechanism was added to the auto-mode service's executeFeature function to limit non-auto-mode runs to a maximum of 3 concurrent agents (configurable), while auto-mode runs remain unaffected. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly identifies and addresses a concurrency limit bypass for manual feature executions. The approach of adding a check at the beginning of executeFeature is sound. My review includes a suggestion to make this fix more robust by ensuring it respects the user's configured concurrency limit from settings, rather than falling back to a hardcoded value. This will make the behavior consistent and fully deliver on the goal of enforcing the configured limit.
| // Check global concurrency limit for manual executions | ||
| // (Auto-mode has its own check in the auto-loop, so skip for isAutoMode) | ||
| if (!isAutoMode) { | ||
| const maxConcurrency = this.config?.maxConcurrency || 3; |
There was a problem hiding this comment.
This is a good fix for enforcing a concurrency limit on manual runs. However, it currently falls back to a hardcoded limit of 3, as this.config is typically not set in that context. This might not reflect the user's actual configured maxConcurrency setting.
To ensure the user's configured limit is always respected, we should fetch the global settings as a fallback. Additionally, we can use the DEFAULT_GLOBAL_SETTINGS.maxConcurrency constant for the ultimate fallback value to avoid magic numbers.
You'll need to add DEFAULT_GLOBAL_SETTINGS to your imports from @automaker/types.
const globalSettings = await this.settingsService?.getGlobalSettings();
const maxConcurrency =
this.config?.maxConcurrency ??
globalSettings?.maxConcurrency ??
DEFAULT_GLOBAL_SETTINGS.maxConcurrency;Apply PR review feedback: use settingsService.getGlobalSettings() as fallback for maxConcurrency instead of hardcoded value. Uses DEFAULT_GLOBAL_SETTINGS.maxConcurrency as ultimate fallback to avoid magic numbers and ensure user's configured limit is always respected. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/server/src/services/auto-mode-service.ts`:
- Around line 500-506: When handling manual executions in executeFeature (where
isAutoMode is false) the code falls back to this.config which is only set by
startAutoLoop; instead fetch maxConcurrency from the injected SettingsService by
calling settingsService.getGlobalSettings() and reading its maxConcurrency when
this.config is undefined or isAutoMode is false, then use that value for the
runningFeatures.size check (preserve the existing error message and logic).
Update the branch around isAutoMode in executeFeature to prefer
this.config?.maxConcurrency when set, otherwise use
settingsService.getGlobalSettings().maxConcurrency, and ensure the existing
throw still triggers when runningFeatures.size >= resolved maxConcurrency.
| if (!isAutoMode) { | ||
| const maxConcurrency = this.config?.maxConcurrency || 3; | ||
| if (this.runningFeatures.size >= maxConcurrency) { | ||
| throw new Error( | ||
| `Cannot start: ${this.runningFeatures.size}/${maxConcurrency} agents already running` | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the file exists and examine its key sections
git ls-files | grep -i auto-mode-serviceRepository: AutoMaker-Org/automaker
Length of output: 314
🏁 Script executed:
# Search for maxConcurrency and config-related patterns
rg -n --type=ts 'maxConcurrency' -B 2 -A 2Repository: AutoMaker-Org/automaker
Length of output: 43754
🏁 Script executed:
# Search for this.config assignments and references
rg -n --type=ts 'this\.config\s*=' -B 1 -A 2Repository: AutoMaker-Org/automaker
Length of output: 1654
🏁 Script executed:
# Look for AutoModeService class definition and constructor
rg -n --type=ts 'class AutoModeService|constructor\s*\(' -A 5 apps/server/src/services/auto-mode-service.tsRepository: AutoMaker-Org/automaker
Length of output: 536
🏁 Script executed:
# Check the imports and config types
head -30 apps/server/src/services/auto-mode-service.tsRepository: AutoMaker-Org/automaker
Length of output: 902
🏁 Script executed:
# Search for executeFeature calls to understand manual execution paths
rg -n 'executeFeature' --type=ts -B 2 -A 1 apps/server/src/Repository: AutoMaker-Org/automaker
Length of output: 3679
🏁 Script executed:
# Check the SettingsService to see how maxConcurrency is accessed
rg -n 'settingsService' --type=ts apps/server/src/services/auto-mode-service.ts | head -20Repository: AutoMaker-Org/automaker
Length of output: 1491
🏁 Script executed:
# Get the full executeFeature method signature and context
sed -n '480,530p' apps/server/src/services/auto-mode-service.tsRepository: AutoMaker-Org/automaker
Length of output: 1663
🏁 Script executed:
# Check if this.config is initialized anywhere else besides startAutoLoop
rg -n 'this\.config\s*=' apps/server/src/services/auto-mode-service.tsRepository: AutoMaker-Org/automaker
Length of output: 90
🏁 Script executed:
# Verify SettingsService has getGlobalSettings method
rg -n 'getGlobalSettings' --type=ts apps/server/src/services/settings-service.ts | head -5Repository: AutoMaker-Org/automaker
Length of output: 341
🏁 Script executed:
# Check the full route file to confirm manual execution path
cat apps/server/src/routes/auto-mode/routes/run-feature.tsRepository: AutoMaker-Org/automaker
Length of output: 1611
Manual executions ignore user-configured maxConcurrency; use settings instead.
When executeFeature is called manually (before auto-mode starts or via the run-feature route), this.config is undefined because it's only initialized in startAutoLoop. This causes the check at line 500–506 to default to 3, bypassing any user-configured limit from GlobalSettings.
The SettingsService is injected into AutoModeService (constructor line 279–281) and has a getGlobalSettings() method that returns the configured maxConcurrency. Use it for manual executions: retrieve maxConcurrency from settingsService.getGlobalSettings() when isAutoMode is false.
🤖 Prompt for AI Agents
In `@apps/server/src/services/auto-mode-service.ts` around lines 500 - 506, When
handling manual executions in executeFeature (where isAutoMode is false) the
code falls back to this.config which is only set by startAutoLoop; instead fetch
maxConcurrency from the injected SettingsService by calling
settingsService.getGlobalSettings() and reading its maxConcurrency when
this.config is undefined or isAutoMode is false, then use that value for the
runningFeatures.size check (preserve the existing error message and logic).
Update the branch around isAutoMode in executeFeature to prefer
this.config?.maxConcurrency when set, otherwise use
settingsService.getGlobalSettings().maxConcurrency, and ensure the existing
throw still triggers when runningFeatures.size >= resolved maxConcurrency.
1 participant
Summary
executeFeature()methodmaxConcurrencylimit as auto-modeProblem
The
maxConcurrencycheck only ran in the auto-mode loop (auto-mode-service.ts:407), butrun-feature.tscallsexecuteFeature()directly with no capacity check. This allowed users to bypass the limit by manually clicking "Run" on features, potentially running 8+ agents when max was set to 5.Solution
Add concurrency check at the start of
executeFeature()for non-auto-mode executions:Auto-mode is skipped since it already has its own check in the auto-loop.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.