-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: use temp file for large system prompts to avoid errno 7 on Linux #1417
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?
Conversation
When CLAUDE.md exceeds ~128KB, passing it as a command-line argument causes "Argument list too long" (errno 7/E2BIG) on Linux due to the kernel's ARG_MAX limit. Changes: - Add _prepare_system_prompt() helper that detects large prompts - Write prompts >90KB to temp file using @filepath syntax - Add comprehensive tests for the new functionality Fixes #384
|
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. 📝 WalkthroughWalkthroughSystem-prompt size handling is implemented to prevent exceeding Linux ARG_MAX limits by writing oversized prompts to temporary files with Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ 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 @StillKnotKnown, 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 resolves a critical issue on Linux systems where large system prompts, particularly those incorporating extensive 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 effectively resolves the 'Argument list too long' error on Linux for large system prompts by using a temporary file. The approach is sound and the new tests are comprehensive. However, I've identified a critical resource leak where these temporary files are never deleted, which could lead to storage issues on the user's machine. I've provided a detailed comment on this issue. Additionally, I've included a suggestion to improve the file handling code to be more idiomatic and robust.
| print(" - CLAUDE.md: disabled by project settings") | ||
| # Build system prompt (handles large prompts via temp file to avoid errno 7) | ||
| # See: https://github.com/AndyMik90/Auto-Claude/issues/384 | ||
| system_prompt_value, _temp_prompt_file = _prepare_system_prompt(project_dir) |
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 line discards the path to the temporary file created for a large system prompt. Since _prepare_system_prompt creates the file with delete=False, it will never be cleaned up, leading to a resource leak on the user's machine. This is a critical issue that will cause temporary files to accumulate indefinitely.
The test test_large_prompt_returns_temp_file_reference correctly calls os.unlink(temp_file), which indicates awareness of this responsibility.
To fix this, you need to manage the lifecycle of the temporary file. Here are a couple of options:
- Modify
create_clientto return the temp file path: Change the return signature to-> tuple[ClaudeSDKClient, str | None]and update all call sites to handle the cleanup in afinallyblock. This is the most robust solution but requires broader changes. - Create a context manager wrapper:
create_clientcould return a wrapper object that implements__enter__and__exit__to handle cleanup. Callers would then need to usewith create_client(...) as client:. This also involves changing all call sites.
Please address this leak to prevent filling up the user's temporary directory.
apps/backend/core/client.py
Outdated
| # ruff: noqa: SIM115 | ||
| temp_file = tempfile.NamedTemporaryFile( | ||
| mode="w", | ||
| suffix=".txt", | ||
| delete=False, | ||
| encoding="utf-8", | ||
| ) | ||
| temp_file.write(base_prompt) | ||
| temp_file.close() | ||
|
|
||
| logger.info( | ||
| f"System prompt size ({prompt_size:,} bytes) exceeds threshold " | ||
| f"({_SYSTEM_PROMPT_SIZE_THRESHOLD:,} bytes). Using temp file: {temp_file.name}" | ||
| ) | ||
| print(f" - CLAUDE.md: large prompt ({prompt_size:,} bytes), using temp file") | ||
|
|
||
| return f"@{temp_file.name}", temp_file.name |
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.
Using a with statement to handle the temporary file is more idiomatic and ensures the file is closed correctly, even if errors occur. You can still access the file name outside the with block. This change also allows you to remove the ruff: noqa: SIM115 directive.
| # ruff: noqa: SIM115 | |
| temp_file = tempfile.NamedTemporaryFile( | |
| mode="w", | |
| suffix=".txt", | |
| delete=False, | |
| encoding="utf-8", | |
| ) | |
| temp_file.write(base_prompt) | |
| temp_file.close() | |
| logger.info( | |
| f"System prompt size ({prompt_size:,} bytes) exceeds threshold " | |
| f"({_SYSTEM_PROMPT_SIZE_THRESHOLD:,} bytes). Using temp file: {temp_file.name}" | |
| ) | |
| print(f" - CLAUDE.md: large prompt ({prompt_size:,} bytes), using temp file") | |
| return f"@{temp_file.name}", temp_file.name | |
| # Write to temp file and use @filepath syntax | |
| temp_file_path: str | |
| with tempfile.NamedTemporaryFile( | |
| mode="w", | |
| suffix=".txt", | |
| delete=False, | |
| encoding="utf-8", | |
| ) as temp_file: | |
| temp_file.write(base_prompt) | |
| temp_file_path = temp_file.name | |
| logger.info( | |
| f"System prompt size ({prompt_size:,} bytes) exceeds threshold " | |
| f"({_SYSTEM_PROMPT_SIZE_THRESHOLD:,} bytes). Using temp file: {temp_file_path}" | |
| ) | |
| print(f" - CLAUDE.md: large prompt ({prompt_size:,} bytes), using temp file") | |
| return f"@{temp_file_path}", temp_file_path |
apps/backend/core/client.py
Outdated
| temp_file = tempfile.NamedTemporaryFile( | ||
| mode="w", | ||
| suffix=".txt", | ||
| delete=False, | ||
| encoding="utf-8", | ||
| ) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
apps/backend/core/client.py
Outdated
| ) | ||
| print(f" - CLAUDE.md: large prompt ({prompt_size:,} bytes), using temp file") | ||
|
|
||
| return f"@{temp_file.name}", temp_file.name |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This addresses the resource leak issue identified in PR review where temporary files created for large system prompts were never cleaned up. Changes: - Add atexit cleanup handler for temp files - Track temp files in module-level list - Use with statement for idiomatic file handling - Remove ruff noqa comment (no longer needed) Fixes review comment from @gemini-code-assist
…ist-too-long-on-linux-when-claude-md-exceeds-128kb
|
|
||
|
|
||
| # Register cleanup handler to run on process exit | ||
| atexit.register(_cleanup_temp_files) |
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.
Bug: The Claude Agent SDK's system_prompt parameter does not support the "@{filepath}" syntax, causing large prompts passed as temp files to fail.
Severity: HIGH
Suggested Fix
Instead of passing a "@{temp_file_path}" string to the SDK, read the contents of the temporary file into a string variable and pass that variable directly to the system_prompt parameter. The temporary file can be deleted immediately after its contents are read.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: apps/backend/core/client.py#L471
Potential issue: The `_prepare_system_prompt` function creates a temporary file for
large system prompts and formats the prompt string as `"@{temp_file_path}"`. However,
the Claude Agent SDK's `system_prompt` parameter does not appear to support this
`@filepath` syntax for file references. The SDK will likely interpret
`"@{temp_file_path}"` as a literal string, passing a nonsensical prompt to the model
instead of the file's contents. This will cause the functionality for handling large
system prompts to fail, as the actual prompt content will not be loaded.
Did we get this right? 👍 / 👎 to inform future reviews.
Base Branch
developbranch (required for all feature/fix PRs)main(hotfix only - maintainers)Description
When CLAUDE.md exceeds ~128KB, passing it as a command-line argument causes "Argument list too long" (errno 7/E2BIG) on Linux due to the kernel's ARG_MAX limit. This fix detects large system prompts and writes them to a temporary file using the
@filepathsyntax supported by the Claude CLI.Related Issue
Closes #384
Refs: ACS-384
Type of Change
Area
Commit Message Format
Follow conventional commits:
<type>: <subject>Types: feat, fix, docs, style, refactor, test, chore
Example:
feat: add user authentication systemChecklist
developbranchencoding="utf-8"for text filesPlatform Testing Checklist
CRITICAL: This project supports Windows, macOS, and Linux. Platform-specific bugs are a common source of breakage.
platform/module instead of directprocess.platformchecksfindExecutable()or platform abstractions)If you only have access to one OS: CI now tests on all platforms. Ensure all checks pass before submitting.
CI/Testing Requirements
_prepare_system_prompt)Screenshots
Feature Toggle
use_feature_nameBreaking Changes
Breaking: No
Details:
This is a non-breaking fix. The temp file approach is transparent to users and only activates when the system prompt exceeds 90KB.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.