-
Notifications
You must be signed in to change notification settings - Fork 576
Fix memory leak #353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix memory leak #353
Changes from all commits
019d6dd
e32a82c
9552670
6d41c7d
d677910
afb0937
586aabe
22aa24a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| node_modules/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,34 @@ | ||
| npx lint-staged | ||
| #!/usr/bin/env sh | ||
|
|
||
| # Try to load nvm if available (optional - works without it too) | ||
| if [ -z "$NVM_DIR" ]; then | ||
| # Check for Herd's nvm first (macOS with Herd) | ||
| if [ -s "$HOME/Library/Application Support/Herd/config/nvm/nvm.sh" ]; then | ||
| export NVM_DIR="$HOME/Library/Application Support/Herd/config/nvm" | ||
| # Then check standard nvm location | ||
| elif [ -s "$HOME/.nvm/nvm.sh" ]; then | ||
| export NVM_DIR="$HOME/.nvm" | ||
| fi | ||
| fi | ||
|
|
||
| # Source nvm if found (silently skip if not available) | ||
| [ -n "$NVM_DIR" ] && [ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh" 2>/dev/null | ||
|
|
||
| # Load node version from .nvmrc if using nvm (silently skip if nvm not available) | ||
| [ -f .nvmrc ] && command -v nvm >/dev/null 2>&1 && nvm use >/dev/null 2>&1 | ||
|
|
||
| # Ensure common system paths are in PATH (for systems without nvm) | ||
| # This helps find node/npm installed via Homebrew, system packages, etc. | ||
| export PATH="$PATH:/usr/local/bin:/opt/homebrew/bin:/usr/bin" | ||
|
|
||
| # Run lint-staged - works with or without nvm | ||
| # Prefer npx, fallback to npm exec, both work with system-installed Node.js | ||
| if command -v npx >/dev/null 2>&1; then | ||
| npx lint-staged | ||
| elif command -v npm >/dev/null 2>&1; then | ||
| npm exec -- lint-staged | ||
| else | ||
| echo "Error: Neither npx nor npm found in PATH." | ||
| echo "Please ensure Node.js is installed (via nvm, Homebrew, system package manager, etc.)" | ||
| exit 1 | ||
| fi | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| # Bugs | ||
|
|
||
| - Setting the default model does not seem like it works. | ||
|
|
||
| # UX | ||
|
|
||
| - Consolidate all models to a single place in the settings instead of having AI profiles and all this other stuff | ||
| - Simplify the create feature modal. It should just be one page. I don't need nessa tabs and all these nested buttons. It's too complex. | ||
| - added to do's list checkbox directly into the card so as it's going through if there's any to do items we can see those update live | ||
| - When the feature is done, I want to see a summary of the LLM. That's the first thing I should see when I double click the card. | ||
| - I went away to mass edit all my features. For example, when I created a new project, it added auto testing on every single feature card. Now I have to manually go through one by one and change those. Have a way to mass edit those, the configuration of all them. | ||
| - Double check and debug if there's memory leaks. It seems like the memory of automaker grows like 3 gigabytes. It's 5gb right now and I'm running three different cursor cli features implementing at the same time. | ||
| - Typing in the text area of the plan mode was super laggy. | ||
| - When I have a bunch of features running at the same time, it seems like I cannot edit the features in the backlog. Like they don't persist their file changes and I think this is because of the secure FS file has an internal queue to prevent hitting that file open write limit. We may have to reconsider refactoring away from file system and do Postgres or SQLite or something. | ||
| - modals are not scrollable if height of the screen is small enough | ||
| - and the Agent Runner add an archival button for the new sessions. | ||
| - investigate a potential issue with the feature cards not refreshing. I see a lock icon on the feature card But it doesn't go away until I open the card and edit it and I turn the testing mode off. I think there's like a refresh sync issue. |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -252,10 +252,14 @@ export function getModelForUseCase( | |||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * Base options that apply to all SDK calls | ||||||||||||
| * | ||||||||||||
| * AUTONOMOUS MODE: Always bypass permissions and allow dangerous operations | ||||||||||||
| * for fully autonomous operation without user prompts. | ||||||||||||
| */ | ||||||||||||
| function getBaseOptions(): Partial<Options> { | ||||||||||||
| return { | ||||||||||||
| permissionMode: 'acceptEdits', | ||||||||||||
| permissionMode: 'bypassPermissions', | ||||||||||||
| allowDangerouslySkipPermissions: true, | ||||||||||||
| }; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
|
|
@@ -276,31 +280,27 @@ interface McpPermissionOptions { | |||||||||||
| * Centralizes the logic for determining permission modes and tool restrictions | ||||||||||||
| * when MCP servers are configured. | ||||||||||||
| * | ||||||||||||
| * AUTONOMOUS MODE: Always bypass permissions for fully autonomous operation. | ||||||||||||
| * Always allow unrestricted tools when MCP servers are configured. | ||||||||||||
| * | ||||||||||||
| * @param config - The SDK options config | ||||||||||||
| * @returns Object with MCP permission settings to spread into final options | ||||||||||||
| */ | ||||||||||||
| function buildMcpOptions(config: CreateSdkOptionsConfig): McpPermissionOptions { | ||||||||||||
| const hasMcpServers = config.mcpServers && Object.keys(config.mcpServers).length > 0; | ||||||||||||
| // Default to true for autonomous workflow. Security is enforced when adding servers | ||||||||||||
| // via the security warning dialog that explains the risks. | ||||||||||||
| const mcpAutoApprove = config.mcpAutoApproveTools ?? true; | ||||||||||||
| const mcpUnrestricted = config.mcpUnrestrictedTools ?? true; | ||||||||||||
|
|
||||||||||||
| // Determine if we should bypass permissions based on settings | ||||||||||||
| const shouldBypassPermissions = hasMcpServers && mcpAutoApprove; | ||||||||||||
| // Determine if we should restrict tools (only when no MCP or unrestricted is disabled) | ||||||||||||
| const shouldRestrictTools = !hasMcpServers || !mcpUnrestricted; | ||||||||||||
| // AUTONOMOUS MODE: Always bypass permissions and allow unrestricted tools | ||||||||||||
| // Only restrict tools when no MCP servers are configured | ||||||||||||
| const shouldRestrictTools = !hasMcpServers; | ||||||||||||
|
|
||||||||||||
| return { | ||||||||||||
| shouldRestrictTools, | ||||||||||||
| // Only include bypass options when MCP is configured and auto-approve is enabled | ||||||||||||
| bypassOptions: shouldBypassPermissions | ||||||||||||
| ? { | ||||||||||||
| permissionMode: 'bypassPermissions' as const, | ||||||||||||
| // Required flag when using bypassPermissions mode | ||||||||||||
| allowDangerouslySkipPermissions: true, | ||||||||||||
| } | ||||||||||||
| : {}, | ||||||||||||
| // AUTONOMOUS MODE: Always include bypass options (though base options already set this) | ||||||||||||
| bypassOptions: { | ||||||||||||
| permissionMode: 'bypassPermissions' as const, | ||||||||||||
| // Required flag when using bypassPermissions mode | ||||||||||||
| allowDangerouslySkipPermissions: true, | ||||||||||||
| }, | ||||||||||||
| // Include MCP servers if configured | ||||||||||||
| mcpServerOptions: config.mcpServers ? { mcpServers: config.mcpServers } : {}, | ||||||||||||
| }; | ||||||||||||
|
|
@@ -392,12 +392,6 @@ export interface CreateSdkOptionsConfig { | |||||||||||
|
|
||||||||||||
| /** MCP servers to make available to the agent */ | ||||||||||||
| mcpServers?: Record<string, McpServerConfig>; | ||||||||||||
|
|
||||||||||||
| /** Auto-approve MCP tool calls without permission prompts */ | ||||||||||||
| mcpAutoApproveTools?: boolean; | ||||||||||||
|
|
||||||||||||
| /** Allow unrestricted tools when MCP servers are enabled */ | ||||||||||||
| mcpUnrestrictedTools?: boolean; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // Re-export MCP types from @automaker/types for convenience | ||||||||||||
|
|
@@ -426,10 +420,7 @@ export function createSpecGenerationOptions(config: CreateSdkOptionsConfig): Opt | |||||||||||
|
|
||||||||||||
| return { | ||||||||||||
| ...getBaseOptions(), | ||||||||||||
| // Override permissionMode - spec generation only needs read-only tools | ||||||||||||
| // Using "acceptEdits" can cause Claude to write files to unexpected locations | ||||||||||||
| // See: https://github.com/AutoMaker-Org/automaker/issues/149 | ||||||||||||
| permissionMode: 'default', | ||||||||||||
| // AUTONOMOUS MODE: Base options already set bypassPermissions and allowDangerouslySkipPermissions | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By removing
Suggested change
|
||||||||||||
| model: getModelForUseCase('spec', config.model), | ||||||||||||
| maxTurns: MAX_TURNS.maximum, | ||||||||||||
| cwd: config.cwd, | ||||||||||||
|
|
@@ -458,8 +449,7 @@ export function createFeatureGenerationOptions(config: CreateSdkOptionsConfig): | |||||||||||
|
|
||||||||||||
| return { | ||||||||||||
| ...getBaseOptions(), | ||||||||||||
| // Override permissionMode - feature generation only needs read-only tools | ||||||||||||
| permissionMode: 'default', | ||||||||||||
| // AUTONOMOUS MODE: Base options already set bypassPermissions and allowDangerouslySkipPermissions | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to spec generation,
Suggested change
|
||||||||||||
| model: getModelForUseCase('features', config.model), | ||||||||||||
| maxTurns: MAX_TURNS.quick, | ||||||||||||
| cwd: config.cwd, | ||||||||||||
|
|
||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change appears unrelated to the PR's stated objective.
The PR title is "Fix memory leak" and the summary mentions removing MCP governance flags and UI memory-leak mitigation. However, this file introduces a complex nvm detection and Node.js environment setup wrapper for the pre-commit hook, which seems entirely unrelated to the core objective.
Consider moving this to a separate PR for better change isolation and traceability, or clarify how this relates to the memory leak fix.
🤖 Prompt for AI Agents