-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: Enable YOLO Mode for Insights Chat Agent #1395
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
base: develop
Are you sure you want to change the base?
feat: Enable YOLO Mode for Insights Chat Agent #1395
Conversation
…ument to insights_runner.py
… and configure security settings Configure Claude SDK security settings based on YOLO mode flag: - Add security settings configuration (YOLO vs standard mode) - Write settings to temporary file - Pass settings file to ClaudeSDKClient - Clean up temp file in finally block Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…ySkipPermissions parameter
…o InsightsService.sendMessage()
…uslySkipPermissions and pass to insightsService.sendMessage()
- Fixed critical bug: YOLO_MODE_FLAG was '--yolo' instead of '--dangerously-skip-permissions' - Verified complete chain: Settings → IPC Handler → Service → Executor → CLI → Python - All 6 steps in propagation chain now working correctly - Python CLI expects --dangerously-skip-permissions which now matches frontend executor
📝 WalkthroughWalkthroughA "YOLO mode" feature is implemented across the frontend and backend layers, allowing the insights system to bypass permission checks via a Changes
Sequence DiagramsequenceDiagram
actor User
participant IPC as IPC Handler
participant Service as InsightsService
participant Executor as InsightsExecutor
participant Python as Python Backend
User->>IPC: sendMessage request
IPC->>IPC: Read settings for dangerouslySkipPermissions
IPC->>Service: sendMessage(..., dangerouslySkipPermissions)
Service->>Executor: execute(..., dangerouslySkipPermissions)
Executor->>Executor: Check dangerouslySkipPermissions flag
alt YOLO Mode Enabled
Executor->>Executor: Append --dangerously-skip-permissions to args
else YOLO Mode Disabled
Executor->>Executor: Use default arguments
end
Executor->>Python: Spawn subprocess with arguments
Python->>Python: Create temporary security settings file
alt dangerously_skip_permissions = true
Python->>Python: Set bypassPermissions mode with allow ["*"]
else dangerously_skip_permissions = false
Python->>Python: Set restrictive sandbox (Read/Glob/Grep only)
end
Python->>Python: Pass settings file to ClaudeSDKClient
Python->>Python: Execute with SDK
Python->>Python: Cleanup temporary settings file
Python-->>Executor: Return results
Executor-->>Service: Return response
Service-->>IPC: Return response
IPC-->>User: Send response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Summary of ChangesHello @PatD42, 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 integrates a "YOLO Mode" into the Insights Chat Agent, providing an option to disable filesystem permission checks for the underlying Claude SDK client. This feature allows for more flexible interaction with the codebase by the AI agent, while also clearly marking the associated security implications. The changes span both the backend Python runner and the frontend TypeScript services to ensure end-to-end support for this new configuration. Highlights
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
|
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.
Code Review
This pull request successfully introduces a "YOLO Mode" for the Insights Chat Agent, which allows bypassing permission checks via a --dangerously-skip-permissions flag. The implementation is solid, spanning both the Python backend and the TypeScript frontend. I appreciate the clear and dangerous naming of the flag. The use of a temporary file for security settings is handled correctly, including cleanup in a finally block. I've included a couple of minor suggestions for the Python runner to improve robustness and clarity.
| "Read(./**)", | ||
| "Glob(./**)", | ||
| "Grep(./**)", |
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.
| except Exception: | ||
| # Ignore cleanup errors silently | ||
| pass |
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.
Catching the broad Exception class can mask unexpected issues. It's better to catch a more specific exception. For file cleanup operations like os.unlink, catching OSError is more appropriate as it specifically handles file-related errors (like FileNotFoundError) without suppressing other potential problems.
| except Exception: | |
| # Ignore cleanup errors silently | |
| pass | |
| except OSError: | |
| # Ignore cleanup errors silently | |
| pass |
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/frontend/src/main/ipc-handlers/insights-handlers.ts (1)
68-75: Error message should use i18n translation keys.Per coding guidelines, all user-facing text in the frontend must use i18n translation keys from
react-i18next. The error message on line 74 uses a hardcoded string.Suggested fix using i18n interpolation
- safeSendToRenderer( - getMainWindow, - IPC_CHANNELS.INSIGHTS_ERROR, - projectId, - `Failed to send message: ${errorMessage}` - ); + safeSendToRenderer( + getMainWindow, + IPC_CHANNELS.INSIGHTS_ERROR, + projectId, + { key: 'errors:insights.sendMessageFailed', params: { error: errorMessage } } + );Note: The exact implementation depends on how your error rendering handles i18n keys. You may need to adjust based on your existing error handling patterns. Based on coding guidelines.
apps/backend/runners/insights_runner.py (1)
216-238: Remove invalidsettingsparameter from ClaudeAgentOptions.The
settingsparameter is not supported by ClaudeAgentOptions. According to the Claude Agent SDK documentation, usesetting_sourcesinstead, which accepts a list of strings like["project"],["user"], or["local"]to load filesystem-based settings. The current code passes a file path to an unsupported parameter, which will cause a runtime error. Consider either:
- Removing this parameter and using
setting_sources=["project"]if loading CLAUDE.md configurations- Or, passing security settings through
permissionModeandallowDangerouslySkipPermissionsparameters if bypassing permissions is the goal
🤖 Fix all issues with AI agents
In `@apps/backend/runners/insights_runner.py`:
- Around line 320-328: The finally block that unlinks settings_file currently
swallows exceptions silently; update the except Exception handler in
insights_runner.py (the finally block that references settings_file and calls
debug("insights_runner", "Cleaned up security settings file",
path=settings_file)) to catch the exception as e and emit a debug-level log
including the exception details (e) and context (e.g., path=settings_file)
instead of a silent pass so cleanup failures are recorded for troubleshooting.
- Around line 189-197: Update the CLI help text for the
--dangerously-skip-permissions option to a stronger warning: locate where the
flag dangerously_skip_permissions and its help/description string are defined
(the --dangerously-skip-permissions option) and replace the current message
"Skip filesystem permission checks (DANGEROUS - use only in trusted
environments)" with a clearer, stronger message such as "DANGEROUS: Bypasses all
filesystem permission checks. Only use in fully trusted environments where you
control the code being executed." Ensure the new text is used wherever the
option's help/usage is rendered.
| if dangerously_skip_permissions: | ||
| # YOLO MODE: Bypass all permission prompts | ||
| debug("insights_runner", "YOLO mode enabled - bypassing all permissions") | ||
| security_settings = { | ||
| "permissions": { | ||
| "defaultMode": "bypassPermissions", | ||
| "allow": ["*"], | ||
| }, | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
# Search for dangerouslySkipPermissions usage in TypeScript/JavaScript files
rg "dangerouslySkipPermissions|dangerously_skip_permissions" --type=ts --type=tsx --type=js -C 3Repository: AndyMik90/Auto-Claude
Length of output: 92
🏁 Script executed:
# Also check the Python backend for how this flag is used
rg "dangerously_skip_permissions" apps/backend/ -C 2Repository: AndyMik90/Auto-Claude
Length of output: 2241
🏁 Script executed:
# Check for any UI configuration or settings files that expose this option
fd -e tsx -e ts -e json -e yaml | head -20 | xargs rg -l "permission|yolo|bypass" 2>/dev/null || trueRepository: AndyMik90/Auto-Claude
Length of output: 174
🏁 Script executed:
# Examine the insights_runner.py file more closely for the complete implementation
head -450 apps/backend/runners/insights_runner.py | tail -300Repository: AndyMik90/Auto-Claude
Length of output: 11353
YOLO mode implementation is acceptable but consider stronger warning messaging.
The dangerously_skip_permissions flag safely gates unrestricted permissions behind an explicit CLI opt-in (--dangerously-skip-permissions), and debug logging is properly implemented when activated. However, the help text warning could be stronger:
Current: "Skip filesystem permission checks (DANGEROUS - use only in trusted environments)"
Consider: "DANGEROUS: Bypasses all filesystem permission checks. Only use in fully trusted environments where you control the code being executed."
The implementation correctly handles the temporary settings file (cleanup in finally block, proper exception handling), and the bypass is appropriately restricted to the CLI interface without UI exposure.
🤖 Prompt for AI Agents
In `@apps/backend/runners/insights_runner.py` around lines 189 - 197, Update the
CLI help text for the --dangerously-skip-permissions option to a stronger
warning: locate where the flag dangerously_skip_permissions and its
help/description string are defined (the --dangerously-skip-permissions option)
and replace the current message "Skip filesystem permission checks (DANGEROUS -
use only in trusted environments)" with a clearer, stronger message such as
"DANGEROUS: Bypasses all filesystem permission checks. Only use in fully trusted
environments where you control the code being executed." Ensure the new text is
used wherever the option's help/usage is rendered.
| finally: | ||
| # Clean up temporary settings file | ||
| if settings_file: | ||
| try: | ||
| os.unlink(settings_file) | ||
| debug("insights_runner", "Cleaned up security settings file", path=settings_file) | ||
| except Exception: | ||
| # Ignore cleanup errors silently | ||
| pass |
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.
🧹 Nitpick | 🔵 Trivial
Consider logging cleanup failures at debug level instead of silent swallowing.
While silently ignoring cleanup errors is acceptable, logging at debug level would aid troubleshooting without impacting normal operation.
Suggested improvement
finally:
# Clean up temporary settings file
if settings_file:
try:
os.unlink(settings_file)
debug("insights_runner", "Cleaned up security settings file", path=settings_file)
- except Exception:
- # Ignore cleanup errors silently
- pass
+ except Exception as cleanup_err:
+ debug("insights_runner", "Failed to cleanup settings file", path=settings_file, error=str(cleanup_err))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| finally: | |
| # Clean up temporary settings file | |
| if settings_file: | |
| try: | |
| os.unlink(settings_file) | |
| debug("insights_runner", "Cleaned up security settings file", path=settings_file) | |
| except Exception: | |
| # Ignore cleanup errors silently | |
| pass | |
| finally: | |
| # Clean up temporary settings file | |
| if settings_file: | |
| try: | |
| os.unlink(settings_file) | |
| debug("insights_runner", "Cleaned up security settings file", path=settings_file) | |
| except Exception as cleanup_err: | |
| debug("insights_runner", "Failed to cleanup settings file", path=settings_file, error=str(cleanup_err)) |
🤖 Prompt for AI Agents
In `@apps/backend/runners/insights_runner.py` around lines 320 - 328, The finally
block that unlinks settings_file currently swallows exceptions silently; update
the except Exception handler in insights_runner.py (the finally block that
references settings_file and calls debug("insights_runner", "Cleaned up security
settings file", path=settings_file)) to catch the exception as e and emit a
debug-level log including the exception details (e) and context (e.g.,
path=settings_file) instead of a silent pass so cleanup failures are recorded
for troubleshooting.
67a743f to
e83e445
Compare
Insights chats should use the YOLO Mode configuration.
Changes
--dangerously-skip-permissionsCLI argument toinsights_runner.pyinsights-handlers.tsto readdangerouslySkipPermissionsfrom settings and pass to serviceInsightsService.sendMessage()to accept and forwarddangerouslySkipPermissionsparameterInsightsExecutor.execute()to add--dangerously-skip-permissionsflagImplementation Details
Backend (
insights_runner.py)--dangerously-skip-permissionsCLI argumentdefaultMode: 'bypassPermissions'with wildcard allowtempfilefor cross-platform temp file creationFrontend
insights-executor.ts: AddsYOLO_MODE_FLAGconstant and conditionally includes flag in spawn argsinsights-service.ts: AddsdangerouslySkipPermissionsparameter tosendMessage()insights-handlers.ts: ReadsdangerouslySkipPermissionsfrom settings asynchronouslyTesting
Related
Closes #1 (original PR)
Co-Authored-By: Claude Sonnet 4.5 noreply@anthropic.com
Summary by CodeRabbit
--dangerously-skip-permissionsCLI flag enabling unrestricted execution mode for insights operations✏️ Tip: You can customize this high-level summary in your review settings.